LTW modules + AntSpec to test LTW in the sandbox + code cleanup in my FIXME + fix in error handling when no debug info can be read from @Aj aspect (many files modified since refactored IMessageHanlder for LTW dontIgnore method)

7 files changed, 94 insertions, 43 deletions

diff --git a/weaver/src/org/aspectj/weaver/Shadow.java b/weaver/src/org/aspectj/weaver/Shadow.java
index 93c49f0f0..d0936c214 100644
--- a/weaver/src/org/aspectj/weaver/Shadow.java
+++ b/weaver/src/org/aspectj/weaver/Shadow.java
@@ -458,17 +458,16 @@ public abstract class Shadow {
 		else if (ak.getKey()==AdviceKind.Around.getKey())
 				return IRelationship.Kind.ADVICE_AROUND;
 		else if (ak.getKey()==AdviceKind.CflowEntry.getKey() ||
-		ak.getKey()==AdviceKind.CflowBelowEntry.getKey() ||
-		ak.getKey()==AdviceKind.InterInitializer.getKey() ||
-		ak.getKey()==AdviceKind.PerCflowEntry.getKey() ||
-		ak.getKey()==AdviceKind.PerCflowBelowEntry.getKey() ||
-		ak.getKey()==AdviceKind.PerThisEntry.getKey() ||
-		ak.getKey()==AdviceKind.PerTargetEntry.getKey() ||
-		ak.getKey()==AdviceKind.Softener.getKey() ||
-        ak.getKey()==AdviceKind.PerTypeWithinEntry.getKey()) {
-        //FIXME: Alex: why this System.err was there ?? It prints nasty thing in my LTW.
-          System.err.println("Dont want a message about this: "+ak);
-			  return null;
+                ak.getKey()==AdviceKind.CflowBelowEntry.getKey() ||
+                ak.getKey()==AdviceKind.InterInitializer.getKey() ||
+                ak.getKey()==AdviceKind.PerCflowEntry.getKey() ||
+                ak.getKey()==AdviceKind.PerCflowBelowEntry.getKey() ||
+                ak.getKey()==AdviceKind.PerThisEntry.getKey() ||
+                ak.getKey()==AdviceKind.PerTargetEntry.getKey() ||
+                ak.getKey()==AdviceKind.Softener.getKey() ||
+                ak.getKey()==AdviceKind.PerTypeWithinEntry.getKey()) {
+            //System.err.println("Dont want a message about this: "+ak);
+            return null;
 		}
 		throw new RuntimeException("Shadow.determineRelKind: What the hell is it? "+ak);
 	}
diff --git a/weaver/src/org/aspectj/weaver/ataspectj/Aj5Attributes.java b/weaver/src/org/aspectj/weaver/ataspectj/Aj5Attributes.java
index 290e32f61..a9502994a 100644
--- a/weaver/src/org/aspectj/weaver/ataspectj/Aj5Attributes.java
+++ b/weaver/src/org/aspectj/weaver/ataspectj/Aj5Attributes.java
@@ -425,7 +425,12 @@ public class Aj5Attributes {
             ElementNameValuePair beforeAdvice = getAnnotationElement(before, "value");
             if (beforeAdvice != null) {
                 // this/target/args binding
-                FormalBinding[] bindings = extractBindings(struct);
+                FormalBinding[] bindings = new org.aspectj.weaver.patterns.FormalBinding[0];
+                try {
+                    bindings = extractBindings(struct);
+                } catch (UnreadableDebugInfo unreadableDebugInfo) {
+                    return false;
+                }
                 IScope binding = new BindingScope(
                         struct.enclosingType,
                         bindings
@@ -470,7 +475,12 @@ public class Aj5Attributes {
             ElementNameValuePair afterAdvice = getAnnotationElement(after, "value");
             if (afterAdvice != null) {
                 // this/target/args binding
-                FormalBinding[] bindings = extractBindings(struct);
+                FormalBinding[] bindings = new org.aspectj.weaver.patterns.FormalBinding[0];
+                try {
+                    bindings = extractBindings(struct);
+                } catch (UnreadableDebugInfo unreadableDebugInfo) {
+                    return false;
+                }
                 IScope binding = new BindingScope(
                         struct.enclosingType,
                         bindings
@@ -538,7 +548,12 @@ public class Aj5Attributes {
 
             // this/target/args binding
             // exclude the return binding from the pointcut binding since it is an extraArg binding
-            FormalBinding[] bindings = (returned==null?extractBindings(struct):extractBindings(struct, returned));
+            FormalBinding[] bindings = new org.aspectj.weaver.patterns.FormalBinding[0];
+            try {
+                bindings = (returned==null?extractBindings(struct):extractBindings(struct, returned));
+            } catch (UnreadableDebugInfo unreadableDebugInfo) {
+                return false;
+            }
             IScope binding = new BindingScope(
                     struct.enclosingType,
                     bindings
@@ -611,7 +626,12 @@ public class Aj5Attributes {
 
             // this/target/args binding
             // exclude the throwned binding from the pointcut binding since it is an extraArg binding
-            FormalBinding[] bindings = (throwned==null?extractBindings(struct):extractBindings(struct, throwned));
+            FormalBinding[] bindings = new org.aspectj.weaver.patterns.FormalBinding[0];
+            try {
+                bindings = (throwned==null?extractBindings(struct):extractBindings(struct, throwned));
+            } catch (UnreadableDebugInfo unreadableDebugInfo) {
+                return false;
+            }
             IScope binding = new BindingScope(
                     struct.enclosingType,
                     bindings
@@ -660,7 +680,12 @@ public class Aj5Attributes {
             ElementNameValuePair aroundAdvice = getAnnotationElement(around, "value");
             if (aroundAdvice != null) {
                 // this/target/args binding
-                FormalBinding[] bindings = extractBindings(struct);
+                FormalBinding[] bindings = new org.aspectj.weaver.patterns.FormalBinding[0];
+                try {
+                    bindings = extractBindings(struct);
+                } catch (UnreadableDebugInfo unreadableDebugInfo) {
+                    return false;
+                }
                 IScope binding = new BindingScope(
                         struct.enclosingType,
                         bindings
@@ -732,10 +757,15 @@ public class Aj5Attributes {
                 }
 
                 // this/target/args binding
-                IScope binding = new BindingScope(
-                        struct.enclosingType,
-                        extractBindings(struct)
-                );
+                final IScope binding;
+                try {
+                    binding = new BindingScope(
+                            struct.enclosingType,
+                            extractBindings(struct)
+                    );
+                } catch(UnreadableDebugInfo e) {
+                    return;
+                }
 
                 TypeX[] argumentTypes = new TypeX[struct.method.getArgumentTypes().length];
                 for (int i = 0; i < argumentTypes.length; i++) {
@@ -788,15 +818,25 @@ public class Aj5Attributes {
      * Build the bindings for a given method (pointcut / advice)
      *
      * @param struct
-     * @return
+     * @return null if no debug info is available
      */
-    private static FormalBinding[] extractBindings(AjAttributeMethodStruct struct) {
+    private static FormalBinding[] extractBindings(AjAttributeMethodStruct struct)
+    throws UnreadableDebugInfo {
         Method method = struct.method;
         String[] argumentNames = struct.getArgumentNames();
 
         // assert debug info was here
         if (argumentNames.length != method.getArgumentTypes().length) {
-            throw new RuntimeException("cannot access debug info " + method);
+            struct.handler.handleMessage(
+                    new Message(
+                            "Cannot read debug info for @Aspect '" + struct.enclosingType.getName() + "'"
+                            + " (please compile with 'javac -g' or '<javac debug='true'.../>' in Ant)",
+                            IMessage.FAIL,
+                            null,
+                            struct.enclosingType.getSourceLocation()
+                    )
+            );
+            throw new UnreadableDebugInfo();
         }
 
         List bindings = new ArrayList();
@@ -823,7 +863,8 @@ public class Aj5Attributes {
     }
 
     //FIXME alex deal with exclude index
-    private static FormalBinding[] extractBindings(AjAttributeMethodStruct struct, String excludeFormal) {
+    private static FormalBinding[] extractBindings(AjAttributeMethodStruct struct, String excludeFormal)
+    throws UnreadableDebugInfo {
         FormalBinding[] bindings = extractBindings(struct);
         int excludeIndex = -1;
         for (int i = 0; i < bindings.length; i++) {
@@ -1056,4 +1097,10 @@ public class Aj5Attributes {
         }
         pointcut.m_ignoreUnboundBindingForNames = (String[])ignores.toArray(new String[ignores.size()]);
     }
+
+    /**
+     * A check exception when we cannot read debug info (needed for formal binding)
+     */
+    private static class UnreadableDebugInfo extends Exception {
+    }
 }
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
index 813cf8232..e25beb343 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
@@ -150,11 +150,17 @@ public class BcelAdvice extends Advice {
         } else if (getKind() == AdviceKind.After) {   
             shadow.weaveAfter(this);
         } else if (getKind() == AdviceKind.Around) {
+            // Note: under regular LTW the aspect is usually loaded after the first use of any class affecteted by it
+            // This means that as long as the aspect has not been thru the LTW, it's woven state is unknown
+            // and thus canInline(s) will return false.
+            // To force inlining (test), ones can do Class aspect = FQNAspect.class in the clinit of the target class
+            // FIXME AV : for AJC compiled @AJ aspect (or any code style aspect), the woven state can never be known
+            // if the aspect belongs to a parent classloader. In that case the aspect will never be inlined.
+            // It might be dangerous to change that especially for @AJ aspect non compiled with AJC since if those
+            // are not weaved (f.e. use of some limiteed LTW etc) then they cannot be prepared for inlining.
+            // One solution would be to flag @AJ aspect with an annotation as "prepared" and query that one.
         	if (!canInline(s)) {
         		shadow.weaveAroundClosure(this, hasDynamicTests());
-                //FIXME : check Inlining and LTW
-                //ALEX : uncomment to force inlining for LTW - else inlining does not seems to happen.
-                //shadow.weaveAroundInline(this, hasDynamicTests());
         	} else {
             	shadow.weaveAroundInline(this, hasDynamicTests());
         	}
@@ -231,7 +237,7 @@ public class BcelAdvice extends Advice {
      */
     public boolean mustCheckExceptions() {
         if (getConcreteAspect() == null) {
-            //FIXME Alex: not sure this is good to default to that.
+            //FIXME AV: not sure this is good to default to that.
             // dig when do we reach that ie not yet concretized
             return true;
         }
@@ -357,7 +363,7 @@ public class BcelAdvice extends Advice {
 	                        fact,
 	                        getExtraParameterType().resolve(world));
 	                } else {
-                        //FIXME this code  will throw an error if ProceedingJP is used in a before advice f.e. ok ??
+                        //FIXME AV test for that - this code  will throw an error if ProceedingJP is used in a before advice f.e. ok ??
                         throw new Error("Should not happen - unbound advice argument at index " + i + " in [" +
                                 toString() + "]");
 	                }
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
index 0442d8c68..762bc52ae 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
@@ -1942,14 +1942,10 @@ public class BcelShadow extends Shadow {
 
         // specific test for @AJ proceedInInners
         if (munger.getConcreteAspect().isAnnotationStyleAspect()) {
-            //FIXME
             // if we can't find one proceed()
             // we suspect that the call is happening in an inner class
             // so we don't inline it.
             // Note: for code style, this is done at Aspect compilation time.
-            // Limitation: if there is two calls and one is done in an inner class, inlining will happen
-            // since we will see the non proceed in inner one as if it was the sole one.
-            // The one made in the inner class will never be madeXXX. Should be documented.
             boolean canSeeProceedPassedToOther = false;
             InstructionHandle curr = adviceMethod.getBody().getStart();
             InstructionHandle end = adviceMethod.getBody().getEnd();
@@ -1957,11 +1953,7 @@ public class BcelShadow extends Shadow {
             while (curr != end) {
                 InstructionHandle next = curr.getNext();
                 Instruction inst = curr.getInstruction();
-                /*if ((inst instanceof INVOKEINTERFACE)
-                    && "proceed".equals(((INVOKEINTERFACE) inst).getMethodName(cpg))) {
-                    canSeeProceed = true;
-                    //continue since we may have a proceed in inner or pjp as arg
-                } else*/ if ((inst instanceof InvokeInstruction)
+                if ((inst instanceof InvokeInstruction)
                     && ((InvokeInstruction)inst).getSignature(cpg).indexOf("Lorg/aspectj/lang/ProceedingJoinPoint;") > 0) {
                     // we may want to refine to exclude stuff returning jp ?
                     // does code style skip inline if i write dump(thisJoinPoint) ?
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
index 5a62d9fc7..65db20769 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
@@ -88,7 +88,7 @@ public final class LazyMethodGen {
     private  Attribute[]     attributes;
    // private AnnotationGen[]  annotations;
     /* private */ final LazyClassGen    enclosingClass;   
-    private final BcelMethod      memberView;
+    private /*final*/ BcelMethod      memberView;//FIXME AV LTW
     int highestLineNumber = 0;
 
 	/** This is nonnull if this method is the result of an "inlining".  We currently
@@ -194,8 +194,10 @@ public final class LazyMethodGen {
 	public boolean hasAnnotation(TypeX annotationTypeX) {
 		initialize();
 		if (memberView==null) {
-			System.err.println("REPORT THIS! 02: Can't determine if "+this+" has annotation "+annotationTypeX);
-			return false;
+            memberView = new BcelMethod(getEnclosingClass().getBcelObjectType(), getMethod());//FIXME AV LTW
+			//System.err.println("REPORT THIS! 02: Can't determine if "+getEnclosingClass().getName() + "." + this.getName() + this.getSignature()+" has annotation "+annotationTypeX);
+			//return false
+            return memberView.hasAnnotation(annotationTypeX);
 		}
 		return memberView.hasAnnotation(annotationTypeX);
 	}
diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
index a24008f7c..a1bf961cd 100644
--- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
+++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java
@@ -174,6 +174,7 @@ public class WeavingAdaptor {
 	 */
 	public byte[] weaveClass (String name, byte[] bytes) throws IOException {
 		if (shouldWeave(name)) {
+            System.out.println("WeavingAdaptor.weaveClass " + name);
 			info("weaving '" + name + "'");
 			bytes = getWovenBytes(name, bytes);
 		}
@@ -192,7 +193,11 @@ public class WeavingAdaptor {
     }
 
 	private boolean shouldWeaveName (String name) {
-		return !((name.startsWith("org.apache.bcel.") || name.startsWith("org.aspectj.") || name.startsWith("java.") || name.startsWith("javax.")));
+		return !((name.startsWith("org.apache.bcel.")//FIXME AV why ? bcel is wrapped in org.aspectj.
+                || name.startsWith("org.aspectj.")
+                || name.startsWith("java.")
+                || name.startsWith("javax."))
+                || name.startsWith("$Proxy"));//JDK proxies
 	}
 
     /**
@@ -213,7 +218,6 @@ public class WeavingAdaptor {
 	 * @param bytes the bytes that define the class
 	 * @return byte[] the woven bytes for the class
 	 * @throws IOException
-	 * @throws FileNotFoundException
 	 */
 	private byte[] getWovenBytes(String name, byte[] bytes) throws IOException {
 		WeavingClassFileProvider wcp = new WeavingClassFileProvider(name,bytes);
diff --git a/weaver/testsrc/org/aspectj/weaver/patterns/AnnotationPatternMatchingTestCase.java b/weaver/testsrc/org/aspectj/weaver/patterns/AnnotationPatternMatchingTestCase.java
index e9173723d..35c191bbb 100644
--- a/weaver/testsrc/org/aspectj/weaver/patterns/AnnotationPatternMatchingTestCase.java
+++ b/weaver/testsrc/org/aspectj/weaver/patterns/AnnotationPatternMatchingTestCase.java
@@ -113,6 +113,7 @@ public class AnnotationPatternMatchingTestCase extends TestCase {
 			return false;
 		}
 		public boolean isIgnoring(Kind kind) {return false;}
+        public void dontIgnore(IMessage.Kind kind) {;}
 	}
 	
 	public void testReferenceToNonAnnotationType() {