Community
Participate
Working Groups
(This is an incomplete bug report -- sorry no time not to further isolate.) Running AJDT 1.1.6, I get dozens of these warnings: ---------- Warning does not match because declaring type is java.lang.Object, if match desired use target(st.ata.mc.exim.DatastoreReader) [Xlint:unmatchedSuperTypeInCall] Blah.aj project/src/package/dir line 22 ---------- for something like this code: ---------- import pack.Interface; ... pointcut dsrCall() : call(* Interface.*(..)); // WARNING HERE ... ---------- Each warning points to the pointcut (not the join point shadow). NPE running from the command-line with the latest tree (will attach).
Created attachment 9980 [details] Command and NPE trace NPE running from the command-line with the current tree.
Created attachment 9981 [details] initial attempt to reproduce .zip file with initial test case attempt that does not reproduce the bug but does reflect the basic structure of the code I'm using (and can't submit).
P2 for investigation as NPE
Great! I put this line in bug 59596: "I give up! I can't create a minimal library equivalent to rt.jar that contains an unresolvable member - so I can't create a testcase in CVS" It turns out that any kind of lint warning would do for showing up the NPE covered in bug 59596. In this bug report Wes has shown another way to demonstrate the same NPE, and I've played about a bit with his code to create a testcase. The NPE is due to the same thing as in 59596 - we attempt to make a source context when we don't have a compilation unit. The source context we are trying to make is the one that gives us information like: public static void main(Bananas[] args) { ^^^^^^^ which highlights the area of the source at fault. This means the fix for the 59596 NPE that I checked in yesterday covers this bug too. The important requirement for surfacing this bug is that you are attempting to create a source context for something that came in as binary. In the case here we are attempting to attach a lint warning. I'm going to attach the zip of my testcase setup. The key difference between what Wes attached and what I have attached is that I've moved the UnmatchedCallSupertype code so that it comes in as binary when we compile the aspect. (Incremental compilation is the other way it could come into the system in binary form, on the 2nd compile). Oh and I added the line: System.err.println(this.toString()); to UnmatchedCallSupertype - this is what will cause the lint warning to trigger. Because toString() is declared on java.lang.Object but inherited by interface ILib - this means a pointcut call(* ILib.*(..)) will warn you because toString() calls won't be matched even though you have said '*' for the method name. Am I making sense? I think thats whats happening. If you unzip the attached zip and run: ajc -d out src\*.java ajc -inpath out A.java You get (with 1.2rc1): java.lang.NullPointerException at org.aspectj.ajdt.internal.core.builder.EclipseAdapterUtils.makeLocationContext (EclipseAdapterUtils.java:50) at org.aspectj.ajdt.internal.core.builder.EclipseAdapterUtils.makeSourceLocation (EclipseAdapterUtils.java:120) at org.aspectj.ajdt.internal.core.builder.EclipseAdapterUtils.makeMessage (EclipseAdapterUtils.java:129) I'll attach the zip then follow up with more explanation ...
Created attachment 9996 [details] Testcase zip, shows NPE.
But ... that is not quite the end of the story. With the latest from HEAD which fixes the NPE, I can run that code from 60015.zip and get: C:\temp\wes\bugs2\unmatchedCallSupertype\A.java:6 warning does not match because declaring type is java.lang.Object, if match desired use target (lib.ILib) [Xlint:unmatchedSuperTypeInCall] (no source information available) see also: UnmatchedCallSupertype.java:11 see also: C:\temp\wes\bugs2 \unmatchedCallSupertype\out\UnmatchedCallSupertype.class 1 warning This is giving us all the information we need, but as Wes has noticed, AJDT is only paying attention to the first source location (the pointcut definition). The new AJDT will need to support the notion of multiple locations for messages. The real problem though is if I compile all the source together: ajc A.java src\*.java because that gives an entirely different message: C:\temp\wes\bugs2\unmatchedCallSupertype\src\UnmatchedCallSupertype.java:6 warning does not match because declaring type is java.lang.Object, if match desired use target(lib.ILib) [Xlint:unmatchedSuperTypeInCall] public static void main(String[] args) { ^^^^^^^^^^^^^^^^ see also: C:\temp\wes\bugs2 \unmatchedCallSupertype\src\UnmatchedCallSupertype.java:11 1 warning The first location is wrong. It points at line 6 of UnmatchedCallSupertype. What it should say is line 6 of A.java (to match the other variant of the message). And because that location is wrong, when it prints the source context out, it says: public static void main(String[] args) { ^^^^^^^^^^^^^^^^ which is nonsense, line 6 of UnmatchedCallSupertype is nothing to do with the pointcut or the shadow. This is messy. It is relatively straightforward to adjust the filename used in the 2nd message to be correct (A.java) but because the error arose whilst processing shadows in UnmatchedCallSupertype then when the logic goes looking through the compilation unit for the source context, it still looks at line 6 in UnmatchedCallSupertype which is wrong. We could swap the locations, so the first argument of this lint warning is the shadow that doesn't get matched. But then I'm not sure if the message still makes perfect sense. let me try it...
With the switch in locations, here is the output when doing an 'all source' build: C:\temp\wes\bugs2\unmatchedCallSupertype\src\UnmatchedCallSupertype.java:11 warning does not match because declaring type is java.lang.Object, if match desired use target(lib.ILib) [Xlint:unmatchedSuperTypeInCall] System.err.println(this.toString()); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ see also: C:\temp\wes\bugs2\unmatchedCallSupertype\A.java:6 (with a lovely correct source context, although maybe it could have just underlined toString()) And here is the output doing a 'binary input' build: UnmatchedCallSupertype.java:11 warning does not match because declaring type is java.lang.Object, if match desired use target(lib.ILib) [Xlint:unmatchedSuperTypeInCall] (no source information available) see also: C:\temp\wes\bugs2\unmatchedCallSupertype\A.java:6 see also: C:\temp\wes\bugs2 \unmatchedCallSupertype\out\UnmatchedCallSupertype.class Comments? I'll attach the patch to create this alternative behavior. At the time we create the lint warning we also have the actual shadow signature, in this case: method-call(java.lang.String java.lang.Object.toString()) I wonder if something can be done with that to improve the message because from all the information above (including the source context), it would still take some thinking to work out which shadow on that line wouldn't match. Does anyone think I write too much in these bug reports?
Created attachment 9997 [details] Patch for the weaver that reverses source locations.
(1) IMHO you don't write too much. It's *very* helpful to understand and encouraging to see things dealt with in their messy complexity, and it raises the bar for communications. (2) I am not sure I agree with the xlint message in this context. warning does not match because declaring type is java.lang.Object, if match desired use target(lib.ILib) It sounds like we're saying that using the declaring type is required, when AFAIK it is not. I assume that (i.e., I thought it used to be that) call(* Foo.*(..)) (where Foo is defined) would result in picking out any call where the compile-time type of the reference is a subtype of Foo, regardless of the declaring type of the method -- e.g., Foo foo = null; foo.toString(); // here, though toString declared by Object i.e., while the careful programmer can specify the declaring type, s/he might want to specify a subtype and limit the calls accordingly. (There is no requirement to use the declaring type. This was the suggested workaround before Java 1.4 compilers because earlier compilers were setting up the call site with the declaring type of the method rather than the compile-time type of the reference.) Which means that given class Bar { void bar(){}} class Foo extends Bar { void foo() {}} , the pointcut call(* Foo.bar()) will not match the shadows new Bar().bar() ((Bar)(new Foo()).bar() , though call(* Bar.bar()) would match the shadow new Foo().bar() . The problem with the type in call(* type.*()) is that people think it refers to the object being pointed at, not the reference doing the pointing. Which is why (I think?) we have an xlint warning for this: call(* Bar.foo()) call(* Bar.unmatched()) Where the member is not defined on the reference type, this will never match, even though it could match if you did this (for some subtype of Bar): target(Bar) && call(* foo()) So in the test case (2004-04-27 04:48), where lib.ILib is in fact defined on the classpath and properly imported by the aspect, I see no reason to issue this warning for any join point, and the before advice on call(* ILib.*(..)) in A.java should apply to this Client.java shadow: new Lib().run(); (And btw, I agree that the compile-time type of the reference is not the kind of thing that should matter to a programmer when writing aspects. A small change (e.g., a perfectly safe cast) could result in the pointcut not matching any more. Nonetheless, I believe that's what the semantics say, and coupled with cast join points it could be quite useful.) So back to when the warning (bug 41952) should be issued: it seems applicable when there is a literal method signature definitely matched and that method is defined by a supertype. Then either (a) at the specific method call join points where there is a reference with the compile-time type of a supertype causing the pointcut not to match, the warning would be issued that the advice/pointcut is not matching because of the specified type, and suggesting target instead; or (b) (once at the pointcut definition only) there should be a warning that it won't pick out references even to that type if they have a compile-time type of some supertype which is itself a subtype of the declaring type. I personally prefer (b).
What I've just checked solves the basic annoyance of producing the warning for something like toString() that is inherited from Object when you have clearly specified an interface. This program used to produce the warning on the indicated line: public void run(); } aspect P implements I { public void run() { } public static void main(String[]argv) { aspectOf().run(); aspectOf().toString(); // XLINT WARNING HERE } before(): call(* I.*(..)) {} } But with the fix, it doesnt. AJDTs ability to cope with messages that have multiple source locations (discussed in this bug) is being dealt with under bug 71059 and will be in AJDT 1.1.12. That makes me feel more comfortable about the fact that we are still producing messages with locations that could be considered to be *the wrong way round*.
Hi Wes, Are you ok if we close this bug now - I've done as much as I currently plan to do. The original unhelpful warning no longer comes out. You can download the most recent development driver to test it out. thanks, Andy.
Silence is consent!
Fix released as part of AspectJ 1.2.1
