Community
Participate
Working Groups
Args(..), and also after(..) returning(..), construction semantics do not follow a well defined pattern when generic types are applied. Please, take a look on the following thread: http://dev.eclipse.org/mhonarc/lists/aspectj-users/msg09949.html If the examples shown in that thread are the expected behavior for args(..) and after(..) returning (..) constructions. Please, provide a complete documentation explaining how it must work when generic types are applied. Note that the current documentation, "AspectJ 5 Developer's Notebook", only explains a very simple case. But, we need a more complete doc to understand what are the expected behaviors for all applicable cases. Thanks in advance, Fernando rubbo
fixed all the cases I am aware of. there are no doubt others.
> Note that the current documentation, "AspectJ 5 Developer's Notebook", only > explains a very simple case. But, we need a more complete doc to understand > what are the expected behaviors for all applicable cases. Before Java 5, the AJ used to be that args(A) matches if the concrete argument type of the object is a subtype of A. I think that's a very sensible and easy to understand property and I think that AJ 5 should use the same property with respect to generic types. (unless I am missing something)
Eric - are you saying the documentation is therefore wrong? or the implementation I've just 'fixed' is now wrong? or?
> Eric - are you saying the documentation is therefore wrong? or the > implementation I've just 'fixed' is now wrong? or? The documentation seems correct. My comment was just with respect to your sentence "there are no doubt others.". I think if you stick to this simple rule then there should be no strange corner cases. Sub-typing should be the only relationship that comes into play here. I cannot comment on the correctness of your fix because you did not provide any further information, but I guess that if the implementation is now based on checking for sub-typing then it should be correct.
My comment "there are no doubt others" is based on my past experience that if it isn't tested then it doesn't work. I've written no tests for exploring bounded wildcards or bounded type variables, I've not explored the use of + or *. I've done basically a few little tests to get things sort of behaving and I've made them pass but I don't have time right now to explore the space with all the tests it ought to have. So I'll leave this open to hopefully collect a few more samples and test cases
> [...] I've written no tests for exploring > bounded wildcards or bounded type variables, I've not explored the use of + or > *. Just my two cents. If it is gonna follow the Java subtyping I don't think we should allow + or *.
Just to not get forgotten.. Three things: 1) I did a simple test on this new aspectj development build and it is not working for runtime check. Take a look on the below example: public class C { void m1(List<Integer> e){} } aspect AC{ void around(): execution(* C.m1(..)) && args(List<Integer>){} //: Should match (it does) void around(): execution(* C.m1(..)) && args(ArrayList<Integer>){}//: Should runtime check (it does not match!) void around(): execution(* C.m1(..)) && args(List<Number>){}//: Should not match (it does not!) void around(): execution(* C.m1(..)) && args(ArrayList<Number>){}//: Not sure what is the expected behavior here (it does not match) void around(): execution(* C.m1(..)) && args(List<? extends Number>){}//: Should match (it does) void around(): execution(* C.m1(..)) && args(ArrayList<? extends Number>){}//: Should runtime check (it does not match!) void around(): execution(* C.m1(..)) && args(List){}//: Should match (it does) void around(): execution(* C.m1(..)) && args(ArrayList){}//: Should runtime check (it does not match!) void around(): execution(* C.m1(..)) && args(List<?>){}//: Should match (it does) void around(): execution(* C.m1(..)) && args(ArrayList<?>){}//: Should runtime check (it does not match!) void around(): execution(* C.m1(..)) && args(ArrayList<String>){}//: Should not match (it does not match!) } 2) I think we have a similar issue with the around's return. Look it out. public class B { List<Number> m(){ return null;} } aspect AB{ List<Integer> around() : execution(List<*> B.*(..)) { return proceed(); } } List<Integer> is not subtype of List<Number>. 3) Andy, I can help you with testing those functionality. But, to do this so, we need understand/agree what is the expected behavior. Will It be based on "casting conversion" in Java or Java "sub-typing"?
I'm simplifying the testing approach, rather than using fully baked programs I can test the compatibility at the type representation level. List list = new ArrayList(); List<String> listOfString = new ArrayList<String>(); List<?> listOfSomething = new ArrayList<Integer>(); List<? extends Number> listOfSomethingNumberish = new ArrayList<Integer>(); List<? super Double> listOfSomethingSuperDouble = new ArrayList<Number>(); // Depending on whether the assignment is allowed, I know if Assignable should return true list = listOfString; assertTrue(ajList.isAssignableFrom(ajListOfString)); // here the assignment requires an unchecked conversion, some assignability is not true but coerceability is true listOfString = list; // unchecked conversion to List<String> assertFalse(ajListOfString.isAssignableFrom(ajList)); assertTrue(ajListOfString.isCoerceableFrom(ajListOfSomething)); So, based on that, here is another failing case: // Should be an unchecked cast, but coerceability currently says no listOfSomethingNumberish = (List<? extends Number>) listOfSomethingSuperDouble; assertTrue(ajListOfSomethingNumberish.isCoerceableFrom(ajListOfSomethingSuperDouble));
From Fernando's forum post. import java.util.List; import java.util.ArrayList; public class C<E extends Number> { // void m1(List<Integer> e){} // void m2(List<? extends Number> e){} // void m3(List<Number> e){} // void m4(List<?> e){} // void m5(List<E> e){} // void m6(List<? extends E> e){} // void m7(List<? extends List<? extends E>> e){} // void m8(List e){} // void m9(E e){} } class A1{} class B1 extends A1{} class C1 extends B1{} class D1 extends C1{} class D2<E2 extends C1>{ void m5(List<E2> e){} } aspect AC{ //void around(): execution(* C.m1(..)) && args(List<Integer>){} //: Should match (it does) //void around(): execution(* C.m1(..)) && args(ArrayList<Integer>){}//: Should runtime check (it does!) //void around(): execution(* C.m1(..)) && args(List<Number>){}//: Should not match (it does not!) //void around(): execution(* C.m1(..)) && args(ArrayList<Number>){}//: Should not match (it does not) //void around(): execution(* C.m1(..)) && args(List<? extends Number>){}//: Should match (it does) //void around(): execution(* C.m1(..)) && args(ArrayList<? extends Number>){}//: Should runtime check (it does!) //void around(): execution(* C.m1(..)) && args(List){}//: Should match (it does) //void around(): execution(* C.m1(..)) && args(ArrayList){}//: Should runtime check (it does not match!)ERROR //void around(): execution(* C.m1(..)) && args(List<?>){}//: Should match (it does) //void around(): execution(* C.m1(..)) && args(ArrayList<?>){}//: Should runtime check (it does not match!) //void around(): execution(* C.m1(..)) && args(ArrayList<String>){}//: Should not match (it does not match!) //void around(): execution(* C.m2(..)) && args(List<Integer>){} //: Should not match (but it does) ERROR //void around(): execution(* C.m2(..)) && args(ArrayList<Integer>){}//: Should not match (but it does!) ERROR //void around(): execution(* C.m2(..)) && args(List<Number>){} //: Should not match (but it does) ERROR //void around(): execution(* C.m2(..)) && args(ArrayList<Number>){}//: Should not runtime check (but it does!) ERROR //void around(): execution(* C.m2(..)) && args(List<? extends Number>){}//: Should match (it does) //void around(): execution(* C.m2(..)) && args(ArrayList<? extends Number>){}//: Should runtime check (it does!) //void around(): execution(* C.m2(..)) && args(List){}//: Should match (it does) //void around(): execution(* C.m2(..)) && args(ArrayList){}//: Should runtime check (it does not match!) ERROR //void around(): execution(* C.m2(..)) && args(List<?>){}//: Should match (it does) //void around(): execution(* C.m2(..)) && args(ArrayList<?>){}//: Should runtime check (it does!) //void around(): execution(* C.m2(..)) && args(ArrayList<String>){}//: Should not match (it does not match!) // void around(): execution(* C.m3(..)) && args(List<Integer>){} //: Should not match (it does not) // void around(): execution(* C.m3(..)) && args(ArrayList<Integer>){}//: Should not match (it does not) // void around(): execution(* C.m3(..)) && args(List<Number>){}//: Should match (it does) // void around(): execution(* C.m3(..)) && args(ArrayList<Number>){}//: Should runtime match (it does) // void around(): execution(* C.m3(..)) && args(List<? extends Number>){}//: Should match (it does) // void around(): execution(* C.m3(..)) && args(ArrayList<? extends Number>){}//: Should runtime check (it does!) // void around(): execution(* C.m3(..)) && args(List){}//: Should match (it does) // void around(): execution(* C.m3(..)) && args(ArrayList){}//: Should runtime check (it does not match!) ERROR // void around(): execution(* C.m3(..)) && args(List<?>){}//: Should match (it does) // void around(): execution(* C.m3(..)) && args(ArrayList<?>){}//: Should runtime check (it does!) // void around(): execution(* C.m3(..)) && args(ArrayList<String>){}//: Should not match (it does not match!) // void around(): execution(* C.m4(..)) && args(List<Integer>){} //: Should not match (but it does) ERROR // void around(): execution(* C.m4(..)) && args(ArrayList<Integer>){}//: Should not match (but it does) ERROR // void around(): execution(* C.m4(..)) && args(List<Number>){}//: Should not match (but it does) ERROR // void around(): execution(* C.m4(..)) && args(ArrayList<Number>){}//: Should not match (but it does) ERROR // void around(): execution(* C.m4(..)) && args(List<? extends Number>){}//: Should not match (but it does) ERROR // void around(): execution(* C.m4(..)) && args(ArrayList<? extends Number>){}//: Should not match (but it does!) ERROR // void around(): execution(* C.m4(..)) && args(List){}//: Should match (it does) // void around(): execution(* C.m4(..)) && args(ArrayList){}//: Should runtime check (it does!) // void around(): execution(* C.m4(..)) && args(List<?>){}//: Should match (it does) // void around(): execution(* C.m4(..)) && args(ArrayList<?>){}//: Should runtime check (it does!) // void around(): execution(* C.m4(..)) && args(ArrayList<String>){}//: Should not match (it does not match!) // void around(): execution(* C.m5(..)) && args(List<Integer>){} //: Should not match (but it does) ERROR // void around(): execution(* C.m5(..)) && args(ArrayList<Integer>){}//: Should not match (but it does!) ERROR // void around(): execution(* C.m5(..)) && args(List<Number>){}//: Should not match (but it does!) ERROR // void around(): execution(* C.m5(..)) && args(ArrayList<Number>){}//: Should not match (it does) ERROR // void around(): execution(* C.m5(..)) && args(List<? extends Number>){}//: Should match (it does) // void around(): execution(* C.m5(..)) && args(ArrayList<? extends Number>){}//: Should runtime check (it does!) // void around(): execution(* C.m5(..)) && args(List){}//: Should match (it does) // void around(): execution(* C.m5(..)) && args(ArrayList){}//: Should runtime check (it does not match!) ERROR // void around(): execution(* C.m5(..)) && args(List<?>){}//: Should match (it does) // void around(): execution(* C.m5(..)) && args(ArrayList<?>){}//: Should runtime check (it does not match!) // void around(): execution(* C.m5(..)) && args(ArrayList<String>){}//: Should not match (it does not match!) // void around(): execution(* D2.m5(..)) && args(List<D1>){} //: Should not match (but it does) ERROR // void around(): execution(* D2.m5(..)) && args(ArrayList<D1>){}//: Should not match (but it does!) ERROR // void around(): execution(* D2.m5(..)) && args(List<C1>){}//: Should not match (but it does!) ERROR // void around(): execution(* D2.m5(..)) && args(ArrayList<C1>){}//: Should not match (it does) ERROR // void around(): execution(* D2.m5(..)) && args(List<? extends B1>){}//: Should match (it does) // void around(): execution(* D2.m5(..)) && args(ArrayList<? extends B1>){}//: Should runtime check (it does!) // void around(): execution(* D2.m5(..)) && args(List<? extends C1>){}//: Should match (it does) // void around(): execution(* D2.m5(..)) && args(ArrayList<? extends C1>){}//: Should runtime check (it does!) // void around(): execution(* D2.m5(..)) && args(List){}//: Should match (it does) // void around(): execution(* D2.m5(..)) && args(ArrayList){}//: Should runtime check (it does not match!) ERROR // void around(): execution(* D2.m5(..)) && args(List<?>){}//: Should match (it does) // void around(): execution(* D2.m5(..)) && args(ArrayList<?>){}//: Should runtime check (it does not match!) // void around(): execution(* D2.m5(..)) && args(ArrayList<String>){}//: Should not match (it does not match!) // void around(): execution(* C.m6(..)) && args(List<Integer>){} //: Should not match (but it does) ERROR // void around(): execution(* C.m6(..)) && args(ArrayList<Integer>){}//: Should not match (but it does!) ERROR // void around(): execution(* C.m6(..)) && args(List<Number>){}//: Should not match (but it does!) ERROR // void around(): execution(* C.m6(..)) && args(ArrayList<Number>){}//: Should not match (it does) ERROR // void around(): execution(* C.m6(..)) && args(List<? extends Number>){}//: Should match (it does) // void around(): execution(* C.m6(..)) && args(ArrayList<? extends Number>){}//: Should runtime check (it does!) // void around(): execution(* C.m6(..)) && args(List){}//: Should match (it does) // void around(): execution(* C.m6(..)) && args(ArrayList){}//: Should runtime check (it does not match!) // void around(): execution(* C.m6(..)) && args(List<?>){}//: Should match (it does) // void around(): execution(* C.m6(..)) && args(ArrayList<?>){}//: Should runtime check (it does not match!) // void around(): execution(* C.m6(..)) && args(ArrayList<String>){}//: Should not match (it does not match!) // void around(): execution(* C.m7(..)) && args(List<List<Integer>>){} //: Should not match (but it does) ERROR // void around(): execution(* C.m7(..)) && args(ArrayList<List<Integer>>){}//: Should not match (but it does!) ERROR // void around(): execution(* C.m7(..)) && args(List<List<Number>>){}//: Should not match (but it does!) ERROR // void around(): execution(* C.m7(..)) && args(ArrayList<List<Number>>){}//: Should not match (but it does) ERROR // void around(): execution(* C.m7(..)) && args(List<? extends List<Number>>){}//: Should not match (but it does) ERROR // void around(): execution(* C.m7(..)) && args(ArrayList< ? extends List<Number>>){}//: Should not match (but it does!) ERROR // void around(): execution(* C.m7(..)) && args(List< ? extends List<? extends Number>>){}//: Should match (it does!) // void around(): execution(* C.m7(..)) && args(ArrayList< ? extends List<? extends Number>>){}//: Should match (it does!) // void around(): execution(* C.m7(..)) && args(List){}//: Should match (it does) // void around(): execution(* C.m7(..)) && args(ArrayList){}//: Should runtime check (it does not match!) // void around(): execution(* C.m7(..)) && args(List<?>){}//: Should match (it does) // void around(): execution(* C.m7(..)) && args(ArrayList<?>){}//: Should runtime check (it does!) // void around(): execution(* C.m7(..)) && args(ArrayList<List<String>>){}//: Should not match (it does not match!) // void around(): execution(* C.m8(..)) && args(List<Integer>){} //: Should match with unchecked conversion (it does) // void around(): execution(* C.m8(..)) && args(ArrayList<Integer>){}//: Should runtime check with unchecked conversion (it does!) // void around(): execution(* C.m8(..)) && args(List<Number>){}//: Should match with unchecked conversion (it does!) // void around(): execution(* C.m8(..)) && args(ArrayList<Number>){}//: Should runtime check with unchecked conversion (it does) // void around(): execution(* C.m8(..)) && args(List<? extends Number>){}//: Should match with unchecked conversion (it does!) // void around(): execution(* C.m8(..)) && args(ArrayList<? extends Number>){}//: Should runtime check with unchecked conversion (it does) // void around(): execution(* C.m8(..)) && args(List){}//: Should match (it does) // void around(): execution(* C.m8(..)) && args(ArrayList){}//: Should runtime check (it does!) // void around(): execution(* C.m8(..)) && args(List<?>){}//: Should match (it does) // void around(): execution(* C.m8(..)) && args(ArrayList<?>){}//: Should runtime check (it does!) // void around(): execution(* C.m8(..)) && args(ArrayList<String>){}//: Should not match (it does not match!) // void around(): execution(* C.m9(..)) && args(List<Integer>){} //: Should not match (but it does) ERROR // void around(): execution(* C.m9(..)) && args(ArrayList<Integer>){}//: Should not match (it does not match!) // void around(): execution(* C.m9(..)) && args(Number){}//: Should match (it does!) // void around(): execution(* C.m9(..)) && args(Integer){}//: Should runtime check (it does) // void around(): execution(* C.m9(..)) && args(List<? extends Number>){}//: Should not match (but it does) ERROR // void around(): execution(* C.m9(..)) && args(ArrayList<? extends Number>){}//: Should not match (it does not match!) // void around(): execution(* C.m9(..)) && args(List){}//: Should not match (but it does) ERROR // void around(): execution(* C.m9(..)) && args(ArrayList){}//: Should not match (it does not match!) // void around(): execution(* C.m9(..)) && args(List<?>){}//: Should not match (but it does) ERROR // void around(): execution(* C.m9(..)) && args(ArrayList<?>){}//: Should not match (it does not match!) // void around(): execution(* C.m9(..)) && args(String){}//: Should not match (it does not match!) }
i really want to make that code into a test case but it is a complete nightmare to do so, hmmm.
right, i'm not 'in-the-zone' with thinking about all this at the moment, but can we clarify this case: void m2(List<? extends Number> e) {} //: Should not match (but it does) ERROR void around(): execution(* C.m2(..)) && args(List<Integer>){} I can write the code: void m2(List<? extends Number> e) { List<Integer> ln = (List<Integer>)e; } and get an appropriate warning rather than a compile time error: Blah.java uses unchecked or unsafe operations. so it is an unchecked cast, and so passing it to advice through args(List<Integer>) seems reasonable. AspectJ ought to produce a suitable unchecked warning though.
(In reply to comment #11) > right, i'm not 'in-the-zone' with thinking about all this at the moment, but > can we clarify this case: > > void m2(List<? extends Number> e) {} > > //: Should not match (but it does) ERROR > void around(): execution(* C.m2(..)) && args(List<Integer>){} > > I can write the code: > > void m2(List<? extends Number> e) { > List<Integer> ln = (List<Integer>)e; > } > > and get an appropriate warning rather than a compile time error: > Blah.java uses unchecked or unsafe operations. > > so it is an unchecked cast, and so passing it to advice through > args(List<Integer>) seems reasonable. AspectJ ought to produce a suitable > unchecked warning though. > That is correct. When I did those tests I focused on the Java invariant subtyping and I think I didn't manage correctly wildcards. So, may be there are other errors like this in that bunch of tests.
Ok, I've converted about 30% of your test program to an AspectJ test and found one issue I had to fix - that addressed lots of ERROR cases. I also changed the tests where appropriate based on my previous comment. I will commit this stuff and when I get a moment continue with the rest of the test program. thanks for the comprehensive set of scenarios.
unsetting the target field which is currently set for something already released
