Community
Participate
Working Groups
First aim is to get the binary form on the disk correct, so that it captures all the necessary information that was originally in the source code. This will enable the weaver to do the correct thing. The kind of information that is important relates to type variables and bounds for ITDs. Problem1. A type variable reference type was attempting to serialize the declaring element for the type variable when an attempt was made to serialize it. Unfortunately what goes on to disk when a typemunger is written into the weaver state info object is the collapsed form (collapsed to bounds) of the type variables. So for this "<T> C.m(T t)" we would write out the type variable "Ljava/lang/Object;-1" where -1 means we didn't happen to know the declaring element. When UnresolvedType is used to deserialize this, it sees Ljava/lang/Object; and just reads in a regular type. it leaves -1 on the stream for the next poor bit of logic to crash into. The symptoms of this are a StreamCorruptedException and NegativeArraySizeException. I'm not sure why the declaring element needs to be written out as it should be easy to determine for any given type variable - it is either a type variable for the member, a type variable for the enclosing type or a type variable in the target type (for an ITD).
Problem2. Why on earth do ResolvedMembers capture their typeVariables are 'UnresolvedType[] typeVariables' - that suggests it is an array of UnresolvedTypeVariableReferenceTypes or TypeVariableReferenceTypes. The reference types are suitable for return type or parametertypes but the set recorded directly against the resolvedmember *must* be the real type variables. So I'm changing those to be 'TypeVariable[] typeVariables'. This also means changing (improving!) the serialization logic for typevariables - TypeVariable now has proper write/read methods that record the name and bounds. On deserializing a resolvedMember we now recover typevariables directly rather than TypeVariableReferenceTypes. Also the resolve(world) method on resolvedmember has been joined by a resolve(world,typevariablescope). This method delegates to the resolve method everywhere that type variables *arent* involved. Where type variables *are* involved the typevariablescope can be used to look up a type variable that we only know by name and connect it to the one on the declaring member or type. So, for example, on deserializing '<T> void m(T t)' we would have a typevariable called T stored against the resolvedmember and the parameter would be an unresolvedtypevariablereferencetype for a typevariable T - at this stage the T's aren't the same T. When resolve() is called, we lookup the parameter type variable T on the declaring element and discover the real T, we then return a TypeVariableReferenceType from resolution that embeds a reference to the real T. This means post resolution: member.getTypeVariables()[0]==((TypeVariableReference)member.getParameters[0])).getTypeVariable() i.e. identity equality works now to get the tests passing with these enhancements...
Ok, the new tests are passing but there are still a few situations where equality isn't correct - bcelmethods(fields) that unpack signatures currently won't resolve correctly to all use the same type variable instance, and itds that share type vars won't work either. Hence the resolve code in allows for this. Also, I changed from having resolve(world,declaringelement) back to the old resolve(world) method *but* I added the ability to set the current lookupscope for type variables to the world. So when resolving parts of a resolvedmember we set the lookupscope and with a finally clause we unset it - this works so far!
damn slow progress .... but this mornings problem was interesting, I hit this signature on a method: (LGenericAspect<TT;>.Simple<TN;>;)V and the signature parser crashed and burned - it didnt before because we never unpacked this thing. It failed because after we hit the '.' we didn't set the flag 'couldSeePrimitive' to false and since it was true, the tokenized stream looked like this: '.' 'S' 'imple' because 'S' represents the primitive type String. I changed the parsing of '.' to indicate it couldn't be followed by a primitive and the tokenstream became: '.' 'Simple' which works ;) A fluke that the inner type started with an 'S'
Ok ... all tests passing again, what am I changing this time: 1. An intertypedeclaration knows about the intertype scope it sticks in the scope hierarchy. 2. When an intertypescope is asked for an aliased type variable, e.g. for the 'P' in the parameter to this method: void C<P>.m(List<P> ns) {} where the target type is interface C<N extends Number> {} then the intertypescope returns 'N extends Number' *and* records that it did so, this is so that later on when building the entries to go into the class files representing the ITD we can go back from what we have in the binding after resolution (N extends Number) to the original P. It is important for separate compilation that P is recorded in the file rather than the letter it happened to match with. 3. Modified the makeTypeVariableBinding() code in eclipse factory to work directly with TypeVariables rather than TypeVariableReference objects. 4. Modified makeMethodBinding() to allow for aliases that may be specified for a generic ITD. 5. Modified the ctor for InterTypeMethodBinding to take a munger rather than just the signature for the munger so that it can access the type variable aliases. 6. Fixed a bug in GenericSignatureParser when using inner types within a generic type that will be included in a generic signature attribute. 7. AjcMemberMaker - copies any type variables across when creating the helper methods for ITDs. Not yet complete - only done ITDMs so far. 8. toDebugString() added to the member hierarchy, gives out all the juicy information it can. 9. MemberImpl - changed typesToSignature() to fully respect the 'useRawTypes' flag - and if you specify that it will not only collapse parameterized types to their erasure, it will collapse type variable references to their erasure. 10. NewMethodTypeMunger - serializes the aliases 11. ResolvedMemberImpl - remembers generic information about return type and parameters ! Bit more tolerant in parameterizedWith 12. ResolvedTypeMunger - remembers alias list. 13. TypeVariable - has a getFirstBound() which might be the upper bound or the first interface bound if the upperbound is object 14. Improved the code that looks for the ITD member that will hold any annotations actually attached to an ITD (BcelTypeMunger) still need to work on fields and constructors and ensure that each method created for an ITD collapses its type variables in the right way for binary weaving.
Slight change of tack now, I wanted to use that infrastructure to get something working. So I looked at one of the cases in bug 110307 to do with providing default implementations of ITDs when generics are involved. Under that bug number there are 6 cases checked into the test suite. I now have one working: interface I<T> { } class A { } aspect X { List<T> I<T>.foo() { return null; } // should be ok... declare parents: A implements I<String>; } this did not work at all before because the particular application of the ITD to the class A which implements I was not parameterized according to the usage of I (I<String>) in A's hierarchy. this now works - it builds on the support that persists type variable aliases and ensures resolvedmembers record generic info. Main changes are: Through the ConcreteTypeMunger and ResolvedTypeMunger hierarchy I added 'parameterizeFor()' which can take any ITD that targets a generic type and fills in the type variables as appropriate. For example for "List<T> I<T>.m() {return null;}" where the target interface is used as I<String> in some type, the return value of m() will become List<String> after parameterizeFor() is called. Changed 'addInterTypeMunger' in ResolvedType to do its type system consistency checking based on the parameterized form of an ITD rather than the original form.
To get the final tests for 110307 to pass I had to modify the logic in BcelTypeMunger.enforceDecpRule1_abstractMethodsImplemented() - this didn't take account of ITDs that needed parameterization in order to be considered as overriding inherited abstract methods. Only one case for 110307 still fails - thats to do with using the same named type variable as the target type, i.e. if targetting type 'interface I<T> {}' with an ITD 'public void I<T>.m(List<T> lts) {}' we can get into trouble because of how the eclipse compiler finds the wrong T. We need to change getType() in the scope hierarchy so that Intertypescope can get in to the loop.
We had a recent bug where Adrian had to comment out some tests - they are now back in because of the changes i've made to make the system more generics aware. I've also modified ITD ctors so that in serialized form they remember any type variable aliases used - this is exactly the same as what I did for methods. I still need to finish it for fields tho. A new case I'm worried about is this: interface I<N extends Number> {} aspect X { public Base<Z>.new(Z z) {} } since we still appear to be creating members (ajc$members) in X based on the matched type variable (so we get a method that takes a parameter of 'Number' - the upper bound) when we should have created a method with type Object - the fact that it was Number is irrelevant since that isn't expressed in the aspect, its expressed in the target type and may be changed later. I'm not sure what to do about this yet!
Ok, final bit of work for now is to do fields since methods/ctors are already covered. basically to ensure when sharing type variables with a target generic type, the form of the ITD in the class file refers to the 'alias' and is not tied to the letter in the actual target class. Usual changes for this: NewFieldTypeMungers remember their aliases when (de)serialized makeFieldBinding in eclipseFactory takes account of aliases other changes: changes isAssignable() test when comparing two type variable references. ResolvedTypeMunger - when parameterized it now stashes the 'original' form of the munger. this isn't used yet but I think we'll need it for bridge method creation when an ITD targets an interface and is 'picked up' by implementors. parameterizedFor() on all the type mungers changed. Now cheats a little - parameterizedFor is used to change the aliases to the real type variables for some target type.
Now I'm going through the GenericsTests JUnit tests and trying to include the tests that are commented out. First failing case was to do with the ParentChildRelationship aspect and why the ordering of a set of 3 types passed to the compiler made a difference as to whether the program works. The types are: Blob - simple class ParentChildRelationship<Parent,Child> - abstract generic aspect BlobContainment extends ParentChildRelationship<Blob,Blob> - concrete subaspect If the compiler processes BlobContainment before ParentChildRelationship then things just don't work ... it shouldnt be possible for this to happen but there was a bug in AjLookupEnvironment.collectAllITDsAndDeclares() which didn't allow for the supertype being a parameterizedtypebinding - in the case above the supertype of BlobContainment is a parameterizedtypebinding. All the testcases we must have written so far work because we declare the abstract aspect ahead of the concrete aspect (thats reasonable...) - however you really get into trouble when the concrete and the abstract are in separate files since the compiler is much more likely to see them in a different order. Anyway, I made the logic cope with PTBs and now it all works, do Ctrl-Shift-R and open TheBigOne.java - yes, that passes!!! The aspect is suffering from cast-overload at the moment ... and that needs fixing. Oh, fixing this also uncovered a hopeless piece of logic I put into ResolvedType.discoverActualOccurrenceOfTypeInHierarchy() which I've now fixed.
Ok, next problem was generictestK which attempted to violate bounds, we weren't getting an error message when we should have - this is now fixed.
Amazing !! I was looking at GenericAspectsV which uses the parent child relationship abstract aspect. However, it is full of casts. I wanted to remove the casts. I've done it - finally - and this is the finished working aspect: abstract aspect ParentChildRelationship<Parent,Child> { // The interfaces interface ParentHasChildren<C extends ChildHasParent>{ List<C> getChildren(); void addChild(C child); void removeChild(C child); } interface ChildHasParent<P extends ParentHasChildren>{ P getParent(); void setParent(P parent); } // Put the interfaces on the relevant people declare parents: Parent implements ParentHasChildren<Child>; declare parents: Child implements ChildHasParent<Parent>; public E ChildHasParent<E>.getParent() { return parent; } public List<A> ParentHasChildren<A>.children = new ArrayList<A>(); public B ChildHasParent<B>.parent; public List<D> ParentHasChildren<D>.getChildren() { return Collections.unmodifiableList(children); } public void ChildHasParent<F>.setParent(F parent) { this.parent = parent; } public void ParentHasChildren<G>.addChild(G child) { if (child.getParent() != null) { child.getParent().removeChild(child); } children.add(child); } public void ParentHasChildren<H>.removeChild(H child) { if (children.remove(child)) { child.setParent(null); } } } but its going to take me a little while to get it into CVS. I had to modify some declare parents hierarchy checking and also add the bridge method support to method and field munging...
er .... well integrating some of that took longer than anticipated. Let's talk implementation: 1. Tailored mungers We now tailor mungers for paramaterized types (if they were sharing type variables with the generic type). For example, an 'interface I<N>' targeted with an ITD 'public N I<N>.field' then implemented with 'implements I<String>' will have a munger 'String field' attached to the implementor - inside the munger is a declaredSignature that enables you to get back to the original declaration (required for creating the bridge methods). 2. Bridge methods I'm doing some work on this under another bug too but here I was really interested in the case where you ITD onto a generic interface. For example: interface I<N extends Number> { } public class FieldQ implements I<Double> { } aspect X { Z I<Z>.list; } Now, FieldQ will get a field 'list' of type 'Double' - and for manipulating the list we will have created two ajc$ methods, the getter and the setter: public java.lang.Double ajc$interFieldGet$X$I$list(); Code: Stack=1, Locals=1, Args_size=1 0: aload_0 1: getfield #28; //Field ajc$interField$X$I$list:Ljava/lang/Double; 4: areturn public void ajc$interFieldSet$X$I$list(java.lang.Double); Code: Stack=2, Locals=2, Args_size=2 0: aload_0 1: aload_1 2: putfield #28; //Field ajc$interField$X$I$list:Ljava/lang/Double; 5: return // Notice that these talk in terms of 'Double' - the actual parameterization of I in FieldQ. The problem is that at the interface level we create: public java.lang.Number ajc$interFieldGet$X$I$list(); public void ajc$interFieldSet$X$I$list(java.lang.Number); and so we need bridge methods in FieldQ, that look like this: public void ajc$interFieldSet$X$I$list(java.lang.Number); Code: Stack=2, Locals=2, Args_size=2 0: aload_0 1: aload_1 2: checkcast #34; //class java/lang/Double 5: invokevirtual #38; //Method ajc$interFieldSet$X$I$list:(Ljava/lang/Dou ble;)V 8: return public java.lang.Number ajc$interFieldGet$X$I$list(); Code: Stack=1, Locals=1, Args_size=1 0: aload_0 1: invokevirtual #32; //Method ajc$interFieldGet$X$I$list:()Ljava/lang/Do uble; 4: areturn they delegate to the local 'Double' versions. all this works (shock horror) 3. Type parameter checking There is one place I wimped out and its a reason why the aspect I appended in the previous comment won't work right now. The problem is these two lines declare parents: Parent implements ParentHasChildren<Child>; declare parents: Child implements ChildHasParent<Parent>; Now, WildTypePattern does some validation of type parameters to check they meet the bounds. In the case above it checked that Child meets the bounds for the first type variable in 'ParentHasChildren' and that Parent meets the bounds for the first type variable in 'ChildHasParent'. The two interfaces are declared as: interface ParentHasChildren<C extends ChildHasParent> interface ChildHasParent<P extends ParentHasChildren> Now... because of those declare parents, the type parameters *do* meet the type variable bounds for the interfaces but only after they have both been applied to the types Child and Parent. The problem is that WildTypePattern resolveBindings() which verifies type parameters actually runs way before the delcare parents have been applied. So we get an error for both declare parents statements - even though after they have been applied everything is ok. Moving the type parameter checking is a real pain so I'll look at that after this stuff has gone in. Right now, because the testcases are such uber-testcases for generics/bridge methods/etc I have made the failing check in WildTypePattern configurable and the testcases turn it off (yes, yes, start throwing rocks at me now...). I need to think some more about this but I really want to get everything I've done so far into CVS. phew. I'm about to check it all in. Besides the type param checking above, I'll investigate why the generic aspect Matthew Webster wrote a little while ago still fails. I suspect it's because of unusual bounds - we still have very few tests for the "whacky bounds"(tm) that are allowed with Java5.
matthews aspect is now in. It was incorrect but the error messages were hopeless - so they've been improved.
I modified the type parameter checking to allow the complex ParentChild aspect through. This has damaged another test, genericaspectK but that covers something that is rather less likely to occur in the wild, so that is now commented out. The problem is merely that an error message doesnt come out right for 'K' so its not the end of the world.
Created attachment 29048 [details] Patch of some stuff I was trying out... This creates the 'potentialproblem' support which allows potentialproblems to be recorded against a world and verified later. Might help with the type parameter checking which is currently done too early.
I'm not going to do much more with this bug. I've just added GenericAspectX test and that confirms that the generic aspect from the AJDK compiles fine and is basically usable. However, it doesn't check that the pointcuts in the generic aspect behave as expected. I also added GenericAspectY to check the pointcuts were OK and it fails. I suspect the reason is something similar to the problem I had when verifying declare parents were OK before they had been applied. see the end of comment #13.
Finally... I am closing this. We have generic tests Y and Z written - Y does some basic pointcut stuff, Z does the more advanced pointcut stuff. and I've updated the docs to agree with the testcases.
