Community
Participate
Working Groups
When two methods are introduced and one is in a sublcass and should override the other but has a different access modifier (i.e. public vs. package), dispatch breaks. The following program illustrates: public class Trouble { static abstract class A {} static class B extends A {} public static void main(String[] args) { System.out.println(new B()); } static aspect Bang { abstract String A.getName(); public String B.getName() { return "B"; } public String A.toString() { return getName(); } } } Unexpectedly, it throws the following exception: java.lang.AbstractMethodError: trouble.Trouble$A.ajc$interMethodDispatch2$trouble$getName()Ljava/lang/String; at trouble.Trouble$Bang.ajc$interMethodDispatch1 $trouble_Trouble$Bang$trouble_Trouble$A$getName(Trouble.java) at trouble.Trouble$Bang.ajc$interMethod$trouble_Trouble$Bang$trouble_Trouble$A$toS tring(Trouble.java:40) at trouble.Trouble$A.toString(Trouble.java) at java.lang.String.valueOf(String.java:2131) at java.io.PrintStream.print(PrintStream.java:462) at java.io.PrintStream.println(PrintStream.java:599) at trouble.Trouble.main(Trouble.java:34) Exception in thread "main"
scheduling for investigation in aj5 m3...
There are multiple bugs that discuss the ITD visibility topic, perhaps the most relevant to this bug is bug 70794 which was to do with forcing users to declare ITDs on interfaces as being public. Here we have a similar situation to that. Because the ITD on the supertype is non-public, the member added to the type has a mangled name. Because the ITD on the subtype is public, it gets its 'real' name - effectively there is no overriding relationship between them. Basically, this call to getName(): public String A.toString() { return getName(); } has become a call to the mangled member in A. B doesn't override the mangled member so we go bang. There are two options ... 1. Police that abstract ITDs have to be declared public. We currently say only those targetting interfaces have to be public, perhaps we could extend the rule to all abstract ITDs 2. We have to generate a dispatcher in all subtypes that were hit by the ITD. This dispatcher looks something like: public String ajc$interMethodDispatch2$trouble$getName() { getName(); } It is possible that we don't have access to all subtypes making the second solution unpleasant. And if some user at a later time wants to subtype the abstract class, they'd be expected to implement this horrible mangled name. hmmm
I'm including Wes' useful mailing list post here: > I'm proposing to make it an error, depending on any feedback I get > here, so that you can't 'specify' default visibility for abstract > ITDs. See also "info" bug 50195 https://bugs.eclipse.org/bugs/show_bug.cgi?id=50195 Only publically introduced members can be overriden by standard Java code. This is a corollary of the rule that package-private ITD's are visible only in the same AspectJ compilation. I'm not happy with that rule, but it is a clear one that explains a number of things without resorting to mangling discussions. In 1.2.1, overriding works if the package-private ITD's and their package-private implementations are declared in the same compile process and the implementations are ITD's. It does not work if the implementation is in a class, or in another compile process. If the implementing ITD increases the visibility to public, it can access the supertype implementation using super, and clients can invoke it from pure-Java code, even in a later javac compile. (72834 is about accessing subtype implementations via the supertype reference.) You're saying that we shouldn't do the dispatch method because it wouldn't be effective for other subtypes and later subtypes, but we've already punted on things outside the current compilation. By making abstract package-private ITD's illegal, you remove the cases where people use package-private implementations in the same compile process. I personally prefer to minimize visibility to reduce complexity, so I'd like to retain that ability. I can imagine some pattern implementations using aspects where you have an abstract method (like traverse()) and aspects corresponding to different subtypes handling traversal. If you don't want other classes to see and use the traverse() method, then you'd be out of luck with the new error message. One of the benefits of package-private pattern implementations would be using the same method names (like "traverse()") without fear of method name collision with other pattern implementations targetting the same types. Nor do I think it helps to single out abstract methods. The issue is with any package-private ITD, no? (It's perhaps more likely that the implementor is outside the compilation process with abstract ITD's.) Would it be enough to have a lint message for any ITD with package visibility, since its scope is effectively limited to that compile process (and to aspects for the purpose of overriding)? Then users could decide whether they are info/warning/error messages, depending on whether they understand the issue. btw, I would distinguish the case of interfaces; since Java forces interface declarations to be public, we haven't taken anything away from users. Is the implementation of package-private ITD overriding inhibiting some other necessary compiler change? Would you want to avoid the dispatcher by doing it on the call side? (Would that break incremental?) Wes P.S. - I should fold the known limitations (i.e., the "info" bugs) into the documentation. ================================ that was in response to me seeing if people actually use this feature. over the weekend I was thinking about just providing more warnings/messages in the situation where your ITDs aren't going to behave as you expect...
putting on my TODO list, testcase already added to CVS.
I'm not going to tackle this for 1.5.0 ... punting it *again* ;)
*cough* punt
