aspects are reweavable by default, fixed issue in (AJC + LTW + Inlining + @AJ)

11 files changed, 89 insertions, 23 deletions

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAccessForInlineMunger.java b/weaver/src/org/aspectj/weaver/bcel/BcelAccessForInlineMunger.java
index c174ee3cd..a4727a460 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelAccessForInlineMunger.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelAccessForInlineMunger.java
@@ -48,6 +48,9 @@ import java.util.List;
  * Specific state and logic is kept in the munger ala ITD so that call/get/set pointcuts can still be matched
  * on the wrapped member thanks to the EffectiveSignature attribute.
  *
+ * FIXME AV - this whole one should be skept when -XnoInline is used
+ * (add breakpoint on lazyMethodGen.setCanInline to see where things happening)
+ *
  * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
  */
 public class BcelAccessForInlineMunger extends BcelTypeMunger {
@@ -219,6 +222,13 @@ public class BcelAccessForInlineMunger extends BcelTypeMunger {
 
             curr = next;
         }
+
+        // no reason for not inlining this advice
+        // since it is used for @AJ advice that cannot be inlined by defauilt
+        // make sure we set inline to true since we have done this analysis
+        if (!realizedCannotInline) {
+            aroundAdvice.setCanInline(true);
+        }
     }
 
     /**
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
index d28034fed..1fa87d1b0 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
@@ -21,6 +21,7 @@ import java.util.Collections;
 import org.aspectj.apache.bcel.generic.InstructionFactory;
 import org.aspectj.apache.bcel.generic.InstructionHandle;
 import org.aspectj.apache.bcel.generic.InstructionList;
+import org.aspectj.apache.bcel.generic.ReferenceType;
 import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.Message;
 import org.aspectj.weaver.Advice;
@@ -367,6 +368,13 @@ public class BcelAdvice extends Advice {
                     // ATAJ: for @AJ aspects, handle implicit binding of xxJoinPoint
 	                if (getKind() == AdviceKind.Around) {
 	                    il.append(closureInstantiation);
+                        //
+//                        if (canInline(shadow)) {
+//                            //FIXME ALEX? passin a boolean instead of checking there
+//                            il.append(fact.createCheckCast(
+//                                    (ReferenceType) BcelWorld.makeBcelType(TypeX.forName("org.aspectj.lang.ProceedingJoinPoint"))
+//                            ));
+//                        }
 	                    continue;
 	                } else if ("Lorg/aspectj/lang/JoinPoint$StaticPart;".equals(getSignature().getParameterTypes()[i].getSignature())) {
 	                    if ((getExtraParameterFlags() & ThisJoinPointStaticPart) != 0) {
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
index 305bdcf23..eab392a17 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
@@ -327,7 +327,7 @@ class BcelClassWeaver implements IClassWeaver {
        
 
         Set aspectsAffectingType = null;
-        if (inReweavableMode) aspectsAffectingType = new HashSet();
+        if (inReweavableMode || clazz.getType().isAspect()) aspectsAffectingType = new HashSet();
         
         boolean isChanged = false;
         
@@ -345,7 +345,7 @@ class BcelClassWeaver implements IClassWeaver {
         	boolean typeMungerAffectedType = munger.munge(this);
         	if (typeMungerAffectedType) {
         		isChanged = true;
-        		if (inReweavableMode) aspectsAffectingType.add(munger.getAspectType().getName());
+        		if (inReweavableMode || clazz.getType().isAspect()) aspectsAffectingType.add(munger.getAspectType().getName());
         	}
         }
 
@@ -378,7 +378,7 @@ class BcelClassWeaver implements IClassWeaver {
 			boolean shadowMungerMatched = match(mg);
 			if (shadowMungerMatched) {
 				// For matching mungers, add their declaring aspects to the list that affected this type
-				if (inReweavableMode) aspectsAffectingType.addAll(findAspectsForMungers(mg));
+				if (inReweavableMode || clazz.getType().isAspect()) aspectsAffectingType.addAll(findAspectsForMungers(mg));
               isChanged = true;
 			}
         }
@@ -415,7 +415,7 @@ class BcelClassWeaver implements IClassWeaver {
                     boolean typeMungerAffectedType = munger.munge(this);
                     if (typeMungerAffectedType) {
                         isChanged = true;
-                        if (inReweavableMode) aspectsAffectingType.add(munger.getAspectType().getName());
+                        if (inReweavableMode || clazz.getType().isAspect()) aspectsAffectingType.add(munger.getAspectType().getName());
                     }
                 }
             }
@@ -429,7 +429,7 @@ class BcelClassWeaver implements IClassWeaver {
 			weaveInAddedMethods(); // FIXME asc are these potentially affected by declare annotation?
         }
         
-        if (inReweavableMode) {
+        if (inReweavableMode || clazz.getType().isAspect()) {
         	WeaverStateInfo wsi = clazz.getOrCreateWeaverStateInfo();
         	wsi.addAspectsAffectingType(aspectsAffectingType);
         	wsi.setUnwovenClassFileData(ty.getJavaClass().getBytes());
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
index 8f80c7195..f41cce965 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
@@ -52,6 +52,7 @@ import org.aspectj.apache.bcel.generic.SWAP;
 import org.aspectj.apache.bcel.generic.StoreInstruction;
 import org.aspectj.apache.bcel.generic.TargetLostException;
 import org.aspectj.apache.bcel.generic.Type;
+import org.aspectj.apache.bcel.generic.ReferenceType;
 import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.ISourceLocation;
 import org.aspectj.bridge.Message;
@@ -1933,8 +1934,7 @@ public class BcelShadow extends Shadow {
         BcelObjectType ot = BcelWorld.getBcelObjectType(declaringType); 
         
 		LazyMethodGen adviceMethod = ot.getLazyClassGen().getLazyMethodGen(mungerSig);
-		if (!adviceMethod.getCanInline())
-		{
+		if (!adviceMethod.getCanInline()) {
 			weaveAroundClosure(munger, hasDynamicTest);
 			return;
 		}
@@ -2357,6 +2357,14 @@ public class BcelShadow extends Shadow {
                 ResolvedTypeX stateTypeX = BcelWorld.fromBcel(stateType).resolve(world);
                 if ("Lorg/aspectj/lang/JoinPoint;".equals(stateType.getSignature())) {
                     ret.append(new ALOAD(localJp));// from localAdvice signature
+//                } else if ("Lorg/aspectj/lang/ProceedingJoinPoint;".equals(stateType.getSignature())) {
+//                    //FIXME ALEX?
+//                        ret.append(new ALOAD(localJp));// from localAdvice signature
+////                        ret.append(fact.createCheckCast(
+////                                (ReferenceType) BcelWorld.makeBcelType(stateTypeX)
+////                    ));
+//                        // cast ?
+//
                 } else {
                     ret.append(InstructionFactory.createLoad(stateType, i));
                 }
@@ -2452,10 +2460,7 @@ public class BcelShadow extends Shadow {
 //        return ret;
     }
 
-    public void weaveAroundClosure(
-        BcelAdvice munger, 
-        boolean hasDynamicTest) 
-    {
+    public void weaveAroundClosure(BcelAdvice munger, boolean hasDynamicTest) {
     	InstructionFactory fact = getFactory();
 
 		enclosingMethod.setCanInline(false);
@@ -2938,6 +2943,8 @@ public class BcelShadow extends Shadow {
         // join point, it is unnecessary to accept (and pass) tjp.
         if (thisJoinPointVar != null) {
         	parameterTypes = addTypeToEnd(LazyClassGen.tjpType, parameterTypes);
+            //FIXME ALEX? which one            
+            //parameterTypes = addTypeToEnd(LazyClassGen.proceedingTjpType, parameterTypes);
         }
         
         TypeX returnType;
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index c41201142..0b67d4b55 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -162,6 +162,8 @@ public class BcelWeaver implements IWeaver {
 
     	//System.out.println("type: " + type + " for " + aspectName);
 		if (type.isAspect()) {
+            //TODO AV - happens to reach that a lot of time: for each type flagged reweavable X for each aspect in the weaverstate
+            //=> mainly for nothing for LTW - pbly for something in incremental build...
 			xcutSet.addOrReplaceAspect(type);
 		} else {
             // FIXME : Alex: better warning upon no such aspect from aop.xml
@@ -945,7 +947,19 @@ public class BcelWeaver implements IWeaver {
 		    BcelObjectType classType = getClassType(className);			            
 			processReweavableStateIfPresent(className, classType);
 		}
-								
+
+        // register all aspect that have been extracted from reweavable state for LTW
+        // note: when doing AJC, the missing aspect is already catch in the previous state thru Type.MISSING
+        if (alreadyConfirmedReweavableState != null) {
+            for (Iterator iterator = alreadyConfirmedReweavableState.iterator(); iterator.hasNext();) {
+                String aspectClassName = (String) iterator.next();
+                addLibraryAspect(aspectClassName);
+            }
+            // refresh all the stuff... perhaps too much here...
+            if (!alreadyConfirmedReweavableState.isEmpty())
+                prepareForWeave();
+        }
+
 		requestor.addingTypeMungers();
         
         // We process type mungers in two groups, first mungers that change the type
@@ -1062,7 +1076,8 @@ public class BcelWeaver implements IWeaver {
 			world.showMessage(IMessage.INFO,
 					WeaverMessages.format(WeaverMessages.REWEAVABLE_MODE),
 					null, null);
-    	 	    	
+
+        //TODO AV - can't we avoid this creation (LTW = happens for each class load!)
     	alreadyConfirmedReweavableState = new HashSet();
     }
     
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
index 9c5524b81..653da49a0 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
@@ -639,7 +639,7 @@ public final class LazyClassGen {
 	
 	public boolean isReweavable() {
 		if (myType.getWeaverState()==null) return false;
-		return myType.getWeaverState().isReweavable();
+        return myType.getWeaverState().isReweavable();
 	}
 	
 	public Set getAspectsAffectingType() {
@@ -700,7 +700,9 @@ public final class LazyClassGen {
     
     // reflective thisJoinPoint support
     Map/*BcelShadow, Field*/ tjpFields = new HashMap();
-    public static final ObjectType tjpType = 
+    public static final ObjectType proceedingTjpType =
+    	new ObjectType("org.aspectj.lang.ProceedingJoinPoint");
+    public static final ObjectType tjpType =
     	new ObjectType("org.aspectj.lang.JoinPoint");
     public static final ObjectType staticTjpType = 
     	new ObjectType("org.aspectj.lang.JoinPoint$StaticPart");
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
index 65db20769..344c4c2d8 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
@@ -100,7 +100,7 @@ public final class LazyMethodGen {
 
     private int             maxLocals; 
     
-    private boolean canInline = true;
+    private boolean canInline = true;//FIXME AV - ALEX? shouldn't that default to false or unknown?
     private boolean hasExceptionHandlers;
     
     private boolean isSynthetic = false;
@@ -140,6 +140,18 @@ public final class LazyMethodGen {
         this.attributes = new Attribute[0];
         this.enclosingClass = enclosingClass;
         assertGoodBody();
+
+        // @AJ advice are not inlined by default since requires further analysis
+        // and weaving ordering control
+        // TODO AV - improve - note: no room for improvement as long as aspects are reweavable
+        // since the inlined version with wrappers and an to be done annotation to keep
+        // inline state will be garbaged due to reweavable impl
+        if (memberView != null && isAdviceMethod()) {
+            if (enclosingClass.getType().isAnnotationStyleAspect()) {
+                //TODO we could check for @Around advice as well
+                this.canInline = false;
+            }
+        }
     }
        
     private int calculateMaxLocals() {
@@ -167,6 +179,18 @@ public final class LazyMethodGen {
         
 		this.accessFlags = m.getAccessFlags();
 		this.name = m.getName();
+
+        // @AJ advice are not inlined by default since requires further analysis
+        // and weaving ordering control
+        // TODO AV - improve - note: no room for improvement as long as aspects are reweavable
+        // since the inlined version with wrappers and an to be done annotation to keep
+        // inline state will be garbaged due to reweavable impl
+        if (memberView != null && isAdviceMethod()) {
+            if (enclosingClass.getType().isAnnotationStyleAspect()) {
+                //TODO we could check for @Around advice as well
+                this.canInline = false;
+            }
+        }
     }
     
     public boolean hasDeclaredLineNumberInfo() {
diff --git a/weaver/src/org/aspectj/weaver/patterns/PerCflow.java b/weaver/src/org/aspectj/weaver/patterns/PerCflow.java
index 76a2fc345..f9d97a936 100644
--- a/weaver/src/org/aspectj/weaver/patterns/PerCflow.java
+++ b/weaver/src/org/aspectj/weaver/patterns/PerCflow.java
@@ -105,9 +105,9 @@ public class PerCflow extends PerClause {
             );
         }
 
-        //ATAJ inline around advice support
+        //ATAJ inline around advice support - don't use a late munger to allow around inling for itself
         if (Ajc5MemberMaker.isAnnotationStyleAspect(inAspect)) {
-            inAspect.crosscuttingMembers.addLateTypeMunger(new BcelAccessForInlineMunger(inAspect));
+            inAspect.crosscuttingMembers.addTypeMunger(new BcelAccessForInlineMunger(inAspect));
         }
 
 		return ret;
diff --git a/weaver/src/org/aspectj/weaver/patterns/PerObject.java b/weaver/src/org/aspectj/weaver/patterns/PerObject.java
index aec9f7f4d..3ec3addeb 100644
--- a/weaver/src/org/aspectj/weaver/patterns/PerObject.java
+++ b/weaver/src/org/aspectj/weaver/patterns/PerObject.java
@@ -118,9 +118,9 @@ public class PerObject extends PerClause {
             );
         }
 
-        //ATAJ inline around advice support
+        //ATAJ inline around advice support - don't use a late munger to allow around inling for itself
         if (Ajc5MemberMaker.isAnnotationStyleAspect(inAspect)) {
-            inAspect.crosscuttingMembers.addLateTypeMunger(new BcelAccessForInlineMunger(inAspect));
+            inAspect.crosscuttingMembers.addTypeMunger(new BcelAccessForInlineMunger(inAspect));
         }
 
 		return ret;
diff --git a/weaver/src/org/aspectj/weaver/patterns/PerSingleton.java b/weaver/src/org/aspectj/weaver/patterns/PerSingleton.java
index ad780fc02..bf390fa6e 100644
--- a/weaver/src/org/aspectj/weaver/patterns/PerSingleton.java
+++ b/weaver/src/org/aspectj/weaver/patterns/PerSingleton.java
@@ -107,7 +107,7 @@ public class PerSingleton extends PerClause {
 
         //ATAJ inline around advice support
         if (Ajc5MemberMaker.isAnnotationStyleAspect(inAspect)) {
-            inAspect.crosscuttingMembers.addLateTypeMunger(new BcelAccessForInlineMunger(inAspect));
+            inAspect.crosscuttingMembers.addTypeMunger(new BcelAccessForInlineMunger(inAspect));
         }
 
         return ret;
diff --git a/weaver/src/org/aspectj/weaver/patterns/PerTypeWithin.java b/weaver/src/org/aspectj/weaver/patterns/PerTypeWithin.java
index 1971fa83b..59c4169b7 100644
--- a/weaver/src/org/aspectj/weaver/patterns/PerTypeWithin.java
+++ b/weaver/src/org/aspectj/weaver/patterns/PerTypeWithin.java
@@ -155,9 +155,9 @@ public class PerTypeWithin extends PerClause {
             );
         }
 
-        //ATAJ inline around advice support
+        //ATAJ inline around advice support - don't use a late munger to allow around inling for itself
         if (Ajc5MemberMaker.isAnnotationStyleAspect(inAspect)) {
-            inAspect.crosscuttingMembers.addLateTypeMunger(new BcelAccessForInlineMunger(inAspect));
+            inAspect.crosscuttingMembers.addTypeMunger(new BcelAccessForInlineMunger(inAspect));
         }
 
 		return ret;