Community
Participate
Working Groups
When weaving into source, ITDs for constructors and methods override private members of the same signature: it is as if the private member never existed. This is illustrated by the example in Dups.java. The same example, when the classes are compiled separately and using bytecode weaving, gives a ClassFormatError upon execution. The situation for fields is different: the compiler flags an error when an ITD tries to introduce a public x where a private x already existed. The error is however flagged only when x is used in the class. Without any uses, such a clashing field is passed by the compiler, but when the program is run, java throws a ClassFormatError. This behaviour is illustrated in DupField.java The above seems to be a bug: the behaviour should be consistent in all situations. Request: make it always legal to introduce a new member by ITD that has the same name/signature as a private member. Mangle the private member and all its uses. /* --------------------------------------------------------------- Dups.java When weaving into source, ITDs for constructors and methods override private members of the same name. Using bytecode weaving (and separate class files for each of the classes) this example gives a ClassFormatError upon execution. */ aspect Aspect { public A.new() { super(); System.out.println("ITD A()"); } public void A.bar() { System.out.println("ITD bar"); } } class A { void foo() { A a = new A(); bar(); } private A() { super(); System.out.println("private A()"); } private void bar() { System.out.println("private bar"); } } public class Dups { static public void main(String[] args) { new A().foo(); } } /* ------------------------------------------------------------------ DupField.java The compiler flags an error when an ITD tries to introduce a public x where a private x already existed. The error is however flagged only when x is used in the class. Without any uses, such a clashing field is passed by the compiler, but when the program is run, java throws a ClassFormatError. This behaviour is illustrated in DupField.java: to see the ClassFormatError, comment out the assignment "x=0" in foo(). */ aspect Aspect { public int A.x; } class A { private int x; void foo() { // when the line below is commented, we get a runtime error x=0; // error: The field x is ambiguous } } public class DupField { static public void main(String[] args) { new A(); } }
marked as target 1.2.1
The resolution of this bug will be to give an error message when attempting to define an ITD with the same signature as an existing member of the target type. (The JLS does not include visibility as part of the signature). If we did otherwise, consider the situation where a public ITD is made over-the-top of a protected member - eurgh!
(unless the target type is an interface of course)
ok.... we already do the right thing with protected. The bug report raises an interesting semantic question that the semantics appendix doesn't fully address. There is a case to be made that private member declarations should not pollute the namespace for external aspects wishing to make ITDs on that type (Oege's proposal). This would be equivalent to the behaviour seen when declaring a sub-type of the target type. The case for ruling that this should be disallowed is that ITDs do not have the semantics of a sub-type, but rather the semantics of open class extension (as evidenced by that fact that members in the target class are refered to via 'this' within the body of an ITDM rather than 'super'). In particular, if the target class defines a private method m(), and an aspect makes a public ITDM Target.m(), then a reference to this.m() inside the Target class is ambiguous and there is no way for the programmer to differentiate between the two possibilities. There's also the more obvious point that Java doesn't allow you to define more than one member with the same signature but different visibility in the same type - and we have said that ITDs have 'same type' semantics rather than 'sub-type' semantics. All that said, I wasn't party to the original design discussion on how ITDs should work in this case.... so ***Erik***, please jump in if you believe that we *should* do the mangling thing here....
would help if I actually cc'd Erik when asking him a question!
With apologies to anyone who is getting fed-up watching me argue with myself.... There's one big disadvantage to the "error message" solution which is this: if I write an aspect that makes a public ITD on a target class, that aspect is now required to know the internal, private, details of the target class (to make a choice of distinct name). Worse, subsequent maintenance (adding or renaming a member) of the private members of the target class could break existing aspects. That's more tightly coupled than I'd ideally like things to be. If we *did* mangle, the rule would have to be that from inside the target type, any private member defined in that type hides any ITDs with the same signature. However, as Andy points out, if we do mangle, it's not just a case of renaming the private member and its references, we would also have to ensure that all the original (unmangled) join points still exist. Since the current implementation is broken, we know that no-one is actually doing this. I propose to make this an error in 1.2.1, and if we decide that the mangling really is the right thing to do, the 1.2.1 behaviour will be described as an 'implementation limitation' (as opposed to AspectJ's semantics) and we can look to do the harder thing in 1.3.
Fix now available for download in latest aspectj development build. Marking bug as resolved-LATER pending discussion of whether to do mangling in 1.3 or not.
This is definitely the consistent with the original AspectJ design: We consciously decided to make most open-class collisions conflicts, as per the Java conflict rules (declaration for methods, reference for fields). While Oege is right that it may be a _useful_ mechanism to be able to have original private members "shadow" inter-type public methods, I'd vote to have the AspectJ successor language have that feature and leave AspectJ to not do that shadowing.
Fix released as part of AspectJ 1.2.1
LATER/REMIND bugs are being automatically reopened as P5 because the LATER and REMIND resolutions are deprecated.
