Comment 1 Wes Isberg 2004-04-22 13:03:41 EDT

True of initialization and pre-initialization, but not of ITD
constructor-execution or method-execution
----
aspect A {  
    HW.new(String s) {}  
    declare warning : initialization(HW.new(String)) : "";
}
class HW {}
----

I can write advice on one of these join points, and the program runs correctly.
Saying !within({aspecttype}) does not work to avoid the bug, but it does work to
say initialization((!HW).new(String)).  So the documented bug would be

  Compiler exception when declaring a warning or error on 
  initialization or preinitialization when the constructor
  is declared in an aspect on another type. To avoid this 
  error, exclude the join point using the target type pattern:

     initialization((!Target).new(..)

If this is a regression from 1.1.1, then it's worth investigating.
Otherwise, since it hasn't come up before it's a low-traffic case.

As a side note (and different bug?), no warning issue for field-set join point
during during ITD initialization:

----
  private int HW.i = 1;  // expected warning not issued
  declare warning : set(int HW.i): "";
----

Comment 2 Andrew Clement 2004-04-23 12:58:24 EDT

Thanks for investigating Wes.  I had to adapt your small program slightly to 
show the NPE.  It is the initialization join point as you discovered, but 
HW.new(String) isn't the signature of the new ctor so I didn't get an NPE or 
my decw come out? For a while I could only get it to fail if I made 
it "initialization(HW.new(..))".  Then through debugging I saw that the ctor 
that is actually added looks like this "HW.new(String,A)".  So this program 
shows the fault ... but looks bizarre!

aspect A {  
    HW.new(String s) {}  
    declare warning : initialization(HW.new(String,A)) : "";
}
class HW {}

It seems to be broken in AspectJ 1.1 as well, so we haven't regressed - so I'm 
not sure whether we should fix it or not...

Here is a patch for BcelShadow.java that fixes the IllegalArgumentException.
There is a place in that code where, if we haven't a clue about the line 
number for the shadow within a type we decide to use the line number for the 
actual type declaration (ie.  class XXX {).  At the moment it says defer to 
using the type declaration line number if the only line number we can come up 
with is 0.  I have augmented that test to check if only line number we can 
come up with is -1.  This is just an update to describe how far I have gotten, 
perhaps a better version of the fix might be to come up with the line number 
where the ITD was declared (even though it was in a different type) - but I'm 
not sure if it is possible to navigate back from the shadow we are matching on 
to the originating ITD statement (that sounds like a bunch of engineering!)

==================================
Index: BcelShadow.java
===================================================================
RCS 
file: /home/technology/org.aspectj/modules/weaver/src/org/aspectj/weaver/bcel/B
celShadow.java,v
retrieving revision 1.32
diff -u -r1.32 BcelShadow.java
--- BcelShadow.java	7 Apr 2004 12:58:47 -0000	1.32
+++ BcelShadow.java	23 Apr 2004 16:45:32 -0000
@@ -2212,7 +2212,7 @@
     
 	public ISourceLocation getSourceLocation() {
 		int sourceLine = getSourceLine();
-		if (sourceLine == 0) {
+		if (sourceLine == 0 || sourceLine == -1) {
 //			Thread.currentThread().dumpStack();
 //			System.err.println(this + ": " + range);
 			return getEnclosingClass().getType().getSourceLocation
();

=====================================

On your other scenario (the set(int HW.i)) - I think that may warrant a new 
bug - but I suspect it will get marked as a compiler limitation.

Comment 3 Andrew Clement 2004-04-26 06:11:13 EDT

I have checked in the fix I added as a patch previously - this is to prevent 
the nasty exception but there is a bigger issue here that needs looking into 
later.  initialization(HW.new(String,A)) shouldn't match - the fact that the 
ITD of a ctor has led to the creation of a ctor taking a 2nd parameter (aspect 
instance) should not be something the user is aware of.  Ideally they should 
be able to write initialization(HW.new(String)) and that would match - but I 
think a change like that is rather fundamental.

Comment 4 Adrian Colyer 2004-05-13 05:15:00 EDT

Immediate issue dealt with - underlying issue to be looked at as enhancement.

Comment 5 Adrian Colyer 2005-03-22 08:54:49 EST

The constructor we add should be marked aj-synthetic and not match jps? Marked
for consideration in AJ5 M3.

Comment 6 Andrew Clement 2005-06-21 06:01:11 EDT

not generics - moving to M4

Comment 7 Adrian Colyer 2005-09-01 08:40:50 EDT

finally fixed for both KindedPointcut and ArgsPointcut - the synthetic arguments
are no longer taken into account when matching.