pr93253: lazytjp the default

org.aspectj.ajdt.core/src/org/aspectj/ajdt/ajc/BuildArgParser.java

@@ -383,6 +383,7 @@ public class BuildArgParser extends Main {
         public void parseOption(String arg, LinkedList args) { // XXX use ListIterator.remove()
 			int nextArgIndex = args.indexOf(arg)+1; // XXX assumes unique
             // trim arg?
+			buildConfig.setXlazyTjp(true); // now default - MINOR could be pushed down and made default at a lower level
             if (LangUtil.isEmpty(arg)) {
                 showWarning("empty arg found");
             } else if (arg.equals("-inpath")) {;
@@ -523,7 +524,8 @@ public class BuildArgParser extends Main {
 			} else if (arg.equals("-XserializableAspects")) {
 				buildConfig.setXserializableAspects(true);
 			} else if (arg.equals("-XlazyTjp")) {
-				buildConfig.setXlazyTjp(true);
+				// do nothing as this is now on by default
+				showWarning("-XlazyTjp should no longer be used, build tjps lazily is now the default");
             } else if (arg.startsWith("-Xreweavable")) {
             	showWarning("-Xreweavable is on by default");
             	if (arg.endsWith(":compress")) {

org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/PerformanceTestCase.java

@@ -40,7 +40,8 @@ public class PerformanceTestCase extends CommandTestCase {
 		//   joinpoint method-execution(int LazyTjp.doit3(int)) because around advice is used [Xlint:canNotImplementLazyTjp]'
 		// into an error so that we can use checkCompiles() ability to check errors occur.
 		// Pass -proceedOnError to ensure even though we get that message, we still get the class file on disk
-		checkCompile("src1/LazyTjp.aj", new String[] {"-XlazyTjp","-Xlint:error","-proceedOnError"}, new int[] {96});
+		checkCompile("src1/LazyTjp.aj", new String[] {"-Xlint:error","-proceedOnError"}, new int[] {96});
 		TestUtil.runMain("out", "LazyTjp");
 	}
+	
 }

tests/bugs/lazyTjpXLintWarning/LazyTjpTest4.java

@@ -13,6 +13,10 @@ public class LazyTjpTest4 {
     before() : execution(public void LazyTjpTest4.test1()) {
       System.out.println(thisJoinPoint);
     }
+
+    before() : execution(public void LazyTjpTest4.test1()) {
+      System.out.println(thisJoinPoint);
+    }
   }
 
 }

tests/bugs/lazyTjpXLintWarning/LazyTjpTest5.java

@@ -0,0 +1,22 @@
+public class LazyTjpTest5 {
+
+  public void test1 () { }
+  
+  private static aspect Aspect1 {
+
+    private static boolean enabled = true;
+    
+    after () : if(enabled) && execution(public void LazyTjpTest5.test1()) {
+      System.out.println(thisJoinPoint);
+    }
+
+    before() : execution(public void LazyTjpTest5.test1()) {
+      System.out.println(thisJoinPoint);
+    }
+
+    void around() : execution(public void LazyTjpTest5.test1()) {
+      System.out.println(thisJoinPoint);
+    }
+  }
+
+}

tests/bugs/lazyTjpXLintWarning/Scenario3.aj

@@ -7,7 +7,7 @@ public aspect Scenario3 {
     pointcut toBeTraced() : execution(* main(..));
 
     Object around() : toBeTraced() {
-	//	Object[] args = thisJoinPoint.getArgs();
+	//Object[] args = thisJoinPoint.getArgs();
         return proceed();
     }
 	

tests/src/org/aspectj/systemtest/xlint/XLintTests.java

@@ -12,6 +12,7 @@ package org.aspectj.systemtest.xlint;
 import java.io.File;
 import junit.framework.Test;
 import org.aspectj.testing.XMLBasedAjcTestCase;
+import org.aspectj.weaver.bcel.BcelShadow;
 
 public class XLintTests extends org.aspectj.testing.XMLBasedAjcTestCase {
 
@@ -100,8 +101,64 @@ public class XLintTests extends org.aspectj.testing.XMLBasedAjcTestCase {
     runTest("XLint warning for call PCD's using subtype of defining type (-1.3 -Xlint:ignore)");
   }
   
+
+  // the following five tests check various scenarios around the lazyTjp XLint message
+  public void test020(){
+     runTest("no XLint warning: thisJoinPoint potentially lazy and nothing stopping it");
+     assertTrue("Something prevented the lazytjp optimization from working??",BcelShadow.appliedLazyTjpOptimization);
+  }
+  
+  public void test021(){
+     runTest("XLint warning: thisJoinPoint potentially lazy but stopped by around advice which doesn't use tjp");
+     assertFalse("lazytjp optimization should have failed to be applied because of around advice at the jp",
+    		 BcelShadow.appliedLazyTjpOptimization);
+  }
+  
+  public void test022(){
+    runTest("no XLint warning: thisJoinPoint not lazy (no if PCD) but would have been stopped anyway by around advice");
+    assertFalse("lazytjp optimization should have failed to be applied because of around advice *and* before advice has no if() at the jp",
+   		 BcelShadow.appliedLazyTjpOptimization);
+  }
+  
+  public void test023(){
+    runTest("no XLint warning: thisJoinPoint cannot be built lazily");
+    assertFalse("lazytjp optimization should have failed to be applied because before advice has no if() at the jp",
+      		 BcelShadow.appliedLazyTjpOptimization);
+  }  
+   
+  public void test024(){
+    runTest("XLint warning: thisJoinPoint potentially lazy but stopped by around advice which uses tjp");
+    assertFalse("lazytjp optimization should have failed to be applied because around advice uses tjp",
+     		 BcelShadow.appliedLazyTjpOptimization);
+  }
+    
+  public void test025(){
+	runTest("check for xlazytjp warning if actually supplied");
+    assertTrue("Something prevented the lazytjp optimization from working??",BcelShadow.appliedLazyTjpOptimization);
+  }
+  
+  public void test026(){
+	runTest("lazytjp: warning when around advice uses tjp");
+  }
+  
+  public void test027() {
+		runTest("lazytjp: warning when if missing on before advice");
+  }
+  
+  public void test028() {
+		runTest("lazytjp: warning when if missing on after advice");
+  }
+  
+  public void test029() {
+		runTest("lazytjp: multiple clashing advice preventing lazytjp");
+  }
+  
+  public void test030() {
+		runTest("lazytjp: interfering before and around");
+  }
+  
   // FIXME asc put this back in !
-//  public void test020() {
+//  public void test031() {
 //	 if (is15VMOrGreater)
 //       runTest("7 lint warnings");
 //  }

tests/src/org/aspectj/systemtest/xlint/xlint-tests.xml

@@ -2,6 +2,84 @@
 
    <!-- .................................... -Xlint tests -->
     <!-- ............... positive -Xlint tests -->
+    
+    <ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="no XLint warning: thisJoinPoint potentially lazy and nothing stopping it">
+		<compile options="-Xlint:warning" files="Scenario1.aj"/>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="XLint warning: thisJoinPoint potentially lazy but stopped by around advice which doesn't use tjp">
+		<compile options="-Xlint:warning" files="Scenario2.aj">
+			<message kind="warning" line="21" text="can not implement lazyTjp on this joinpoint method-execution(void Test.main(java.lang.String[])) because around advice is used [Xlint:canNotImplementLazyTjp]"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="no XLint warning: thisJoinPoint not lazy (no if PCD) but would have been stopped anyway by around advice">
+		<compile options="-Xlint:warning" files="Scenario3.aj">
+		  <message kind="warning" line="14" text="can not build thisJoinPoint lazily for this advice since it has no suitable guard.  The advice applies at method-execution(void Test.main(java.lang.String[])) [Xlint:noGuardForLazyTjp]"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning" title="no XLint warning: thisJoinPoint cannot be built lazily">
+		<compile options="-Xlint:warning" files="Scenario4.aj">
+			<message kind="warning" line="9" text="can not build thisJoinPoint lazily for this advice since it has no suitable guard.  The advice applies at method-execution(void Test.main(java.lang.String[]))"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="XLint warning: thisJoinPoint potentially lazy but stopped by around advice which uses tjp">
+		<compile options="-Xlint:warning" files="Scenario5.aj">
+			<message kind="warning" line="22" text="can not implement lazyTjp on this joinpoint method-execution(void Test.main(java.lang.String[])) because around advice is used [Xlint:canNotImplementLazyTjp]"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="check for xlazytjp warning if actually supplied">
+		<compile options="-XlazyTjp -Xlint:warning" files="Scenario1.aj">
+		  <message kind="warning" text="-XlazyTjp should no longer be used, build tjps lazily is now the default"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="lazytjp: warning when around advice uses tjp">
+		<compile options="-Xlint:warning" files="LazyTjpTest1.java">
+			<message kind="warning" line="4" text="can not implement lazyTjp on this joinpoint method-execution(void LazyTjpTest1.test2()) because around advice is used [Xlint:canNotImplementLazyTjp]"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="lazytjp: warning when if missing on before advice">
+		<compile options="-Xlint:warning" files="LazyTjpTest2.java">
+		  <message kind="warning" line="16" text="can not build thisJoinPoint lazily for this advice since it has no suitable guard.  The advice applies at method-execution(void LazyTjpTest2.test2())"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="lazytjp: warning when if missing on after advice">
+		<compile options="-Xlint:warning" files="LazyTjpTest3.java">
+		  <message kind="warning" line="17" text="can not build thisJoinPoint lazily for this advice since it has no suitable guard.  The advice applies at method-execution(void LazyTjpTest3.test2())"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="lazytjp: multiple clashing advice preventing lazytjp">
+		<compile options="-Xlint:warning" files="LazyTjpTest4.java">
+		  <message kind="warning" line="13" text="can not build thisJoinPoint lazily for this advice since it has no suitable guard.  The advice applies at method-execution(void LazyTjpTest4.test1())"/>
+		  <message kind="warning" line="3"  text="can not implement lazyTjp at joinpoint method-execution(void LazyTjpTest4.test1()) because of advice conflicts, see secondary locations to find conflicting advice"/>
+		  <message kind="warning" line="17" text="can not build thisJoinPoint lazily for this advice since it has no suitable guard.  The advice applies at method-execution(void LazyTjpTest4.test1())"/>
+		</compile>
+	</ajc-test>
+	
+	<ajc-test dir="bugs/lazyTjpXLintWarning"
+		title="lazytjp: interfering before and around">
+		<compile options="-Xlint:warning" files="LazyTjpTest5.java">
+		  <message kind="warning" line="3" text="can not implement lazyTjp at joinpoint method-execution(void LazyTjpTest5.test1()) because of advice conflicts, see secondary locations to find conflicting advice"/>
+		  <message kind="warning" line="13" text="can not build thisJoinPoint lazily for this advice since it has no suitable guard.  The advice applies at method-execution(void LazyTjpTest5.test1())"/>
+		</compile>
+	</ajc-test>
+    
     <ajc-test dir="options" 
       title="options -Xlint args()" 
       keywords="lint">

weaver/src/org/aspectj/weaver/Lint.java

@@ -54,6 +54,9 @@ public class Lint {
 	public final Kind canNotImplementLazyTjp = 
 		new Kind("canNotImplementLazyTjp", "can not implement lazyTjp on this joinpoint {0} because around advice is used");
 
+	public final Kind multipleAdviceStoppingLazyTjp = 
+		new Kind("multipleAdviceStoppingLazyTjp", "can not implement lazyTjp at joinpoint {0} because of advice conflicts, see secondary locations to find conflicting advice");
+
 	public final Kind needsSerialVersionUIDField = 
 		new Kind("needsSerialVersionUIDField", "serialVersionUID of type {0} needs to be set because of {1}");
 
@@ -92,7 +95,10 @@ public class Lint {
 	
 	public final Kind uncheckedAdviceConversion =
 		new Kind("uncheckedAdviceConversion","unchecked conversion when advice applied at shadow {0}, expected {1} but advice uses {2}");
-	
+ 
+	public final Kind noGuardForLazyTjp =
+		new Kind("noGuardForLazyTjp","can not build thisJoinPoint lazily for this advice since it has no suitable guard.  The advice applies at {0}");
+
     public Lint(World world) {
 		this.world = world;
 	}

weaver/src/org/aspectj/weaver/XlintDefault.properties

@@ -9,7 +9,9 @@ shadowNotInStructure = ignore
 
 unmatchedSuperTypeInCall = warning
 
-canNotImplementLazyTjp = warning
+canNotImplementLazyTjp = ignore
+multipleAdviceStoppingLazyTjp=ignore
+noGuardForLazyTjp=ignore
 
 uncheckedAdviceConversion = warning
 

weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java

@@ -19,11 +19,12 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
 
+import org.aspectj.apache.bcel.generic.InstructionConstants;
 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.InstructionConstants;
 import org.aspectj.bridge.IMessage;
+import org.aspectj.bridge.ISourceLocation;
 import org.aspectj.bridge.Message;
 import org.aspectj.weaver.Advice;
 import org.aspectj.weaver.AdviceKind;
@@ -141,8 +142,22 @@ public class BcelAdvice extends Advice {
         }
 
         if ((getExtraParameterFlags() & ThisJoinPoint) != 0) {
-			((BcelShadow)shadow).requireThisJoinPoint(pointcutTest != Literal.TRUE && getKind() != AdviceKind.Around);
+    		boolean hasGuardTest = pointcutTest != Literal.TRUE && getKind() != AdviceKind.Around;
+    		boolean isAround = getKind() == AdviceKind.Around;
+			((BcelShadow)shadow).requireThisJoinPoint(hasGuardTest,isAround);
 			((BcelShadow)shadow).getEnclosingClass().warnOnAddedStaticInitializer(shadow,getSourceLocation());
+			if (!hasGuardTest && world.getLint().multipleAdviceStoppingLazyTjp.isEnabled()) {
+				// collect up the problematic advice
+				((BcelShadow)shadow).addAdvicePreventingLazyTjp(this);
+			}
+			if (!isAround && !hasGuardTest && world.getLint().noGuardForLazyTjp.isEnabled()) {
+				// can't build tjp lazily, no suitable test...
+				world.getLint().noGuardForLazyTjp.signal(
+					    new String[] {shadow.toString()},
+					    getSourceLocation(),
+					    new ISourceLocation[] { ((BcelShadow)shadow).getSourceLocation() }
+					);				
+			}
         }
         
         if ((getExtraParameterFlags() & ThisEnclosingJoinPointStaticPart) != 0) {

weaver/src/org/aspectj/weaver/bcel/BcelShadow.java

@@ -134,6 +134,9 @@ public class BcelShadow extends Shadow {
     private final LazyMethodGen enclosingMethod;
 	private boolean fallsThrough;  //XXX not used anymore
 	
+	// SECRETAPI - for testing, this will tell us if the optimization succeeded *on the last shadow processed*
+	public static boolean appliedLazyTjpOptimization;
+	
 	// Some instructions have a target type that will vary 
     // from the signature (pr109728) (1.4 declaring type issue)
 	private String actualInstructionTargetType; 
@@ -241,6 +244,14 @@ public class BcelShadow extends Shadow {
 		}
 	}
 	
+	// records advice that is stopping us doing the lazyTjp optimization
+	private List badAdvice = null;
+	
+	public void addAdvicePreventingLazyTjp(BcelAdvice advice) {
+		if (badAdvice == null) badAdvice = new ArrayList();
+		badAdvice.add(advice);
+	}
+	
  	protected void prepareForMungers() {
 		// if we're a constructor call, we need to remove the new:dup or the new:dup_x1:swap, 
 		// and store all our
@@ -289,15 +300,43 @@ public class BcelShadow extends Shadow {
 		}
 
 		// now we ask each munger to request our state
-		isThisJoinPointLazy = world.isXlazyTjp();
-		
+		isThisJoinPointLazy = true;//world.isXlazyTjp(); // lazy is default now
+
+		badAdvice = null;
 		for (Iterator iter = mungers.iterator(); iter.hasNext();) {
 			ShadowMunger munger = (ShadowMunger) iter.next();
 			munger.specializeOn(this);
 		}
 		
+		
 		initializeThisJoinPoint();
 
+		if (thisJoinPointVar!=null && !isThisJoinPointLazy && badAdvice!=null &&  badAdvice.size()>1) {
+			// something stopped us making it a lazy tjp
+			// can't build tjp lazily, no suitable test...
+			int valid = 0;
+			for (Iterator iter = badAdvice.iterator(); iter.hasNext();) {
+				BcelAdvice element = (BcelAdvice) iter.next();
+				ISourceLocation sLoc = element.getSourceLocation();
+				if (sLoc!=null && sLoc.getLine()>0) valid++;
+			}
+			if (valid!=0) { 
+				ISourceLocation[] badLocs = new ISourceLocation[valid];
+				int i = 0;
+				for (Iterator iter = badAdvice.iterator(); iter.hasNext();) {
+					BcelAdvice element = (BcelAdvice) iter.next();
+					ISourceLocation sLoc = element.getSourceLocation();
+					if (sLoc!=null) badLocs[i++]=sLoc;
+				}
+				world.getLint().multipleAdviceStoppingLazyTjp.signal(
+					    new String[] {this.toString()},
+					    getSourceLocation(),badLocs
+					);
+			}
+		}
+		badAdvice=null;
+		
+		
 	    // If we are an expression kind, we require our target/arguments on the stack
 	    // before we do our actual thing.  However, they may have been removed
 	    // from the stack as the shadowMungers have requested state.  
@@ -972,13 +1011,20 @@ public class BcelShadow extends Shadow {
 	public final Var getThisEnclosingJoinPointStaticPartVar() {
 		return getThisEnclosingJoinPointStaticPartBcelVar();
 	}
-    
-    public void requireThisJoinPoint(boolean hasGuardTest) {
-    	if (!hasGuardTest) {
-    		isThisJoinPointLazy = false;
-    	} else {
-    		lazyTjpConsumers++;
-    	}
+	
+    public void requireThisJoinPoint(boolean hasGuardTest, boolean isAround) { 
+		if (!isAround){
+			if (!hasGuardTest) {
+				isThisJoinPointLazy = false;
+			} else {
+				lazyTjpConsumers++;
+			}
+		}
+//    	if (!hasGuardTest) {
+//    		isThisJoinPointLazy = false;
+//    	} else {
+//    		lazyTjpConsumers++;
+//    	}
     	if (thisJoinPointVar == null) {
 			thisJoinPointVar = genTempVar(UnresolvedType.forName("org.aspectj.lang.JoinPoint"));
     	}
@@ -986,7 +1032,7 @@ public class BcelShadow extends Shadow {
     
     
     public Var getThisJoinPointVar() {
-    	requireThisJoinPoint(false);
+    	requireThisJoinPoint(false,false);
     	return thisJoinPointVar;
     }
     
@@ -998,6 +1044,7 @@ public class BcelShadow extends Shadow {
     	}
     		
 		if (isThisJoinPointLazy) {
+			appliedLazyTjpOptimization = true;
 			createThisJoinPoint(); // make sure any state needed is initialized, but throw the instructions out
 			
 			if (lazyTjpConsumers == 1) return; // special case only one lazyTjpUser
@@ -1008,6 +1055,7 @@ public class BcelShadow extends Shadow {
 			il.append(thisJoinPointVar.createStore(fact));
 			range.insert(il, Range.OutsideBefore);
 		} else {
+			appliedLazyTjpOptimization = false;
 			InstructionFactory fact = getFactory();
 			InstructionList il = createThisJoinPoint();
 			il.append(thisJoinPointVar.createStore(fact));
@@ -1021,11 +1069,13 @@ public class BcelShadow extends Shadow {
 			ShadowMunger munger = (ShadowMunger) i.next();
 			if (munger instanceof Advice) {
 				if ( ((Advice)munger).getKind() == AdviceKind.Around) {
-					world.getLint().canNotImplementLazyTjp.signal(
-					    new String[] {toString()},
-					    getSourceLocation(),
-					    new ISourceLocation[] { munger.getSourceLocation() }
-					);
+					if (munger.getSourceLocation()!=null) { // do we know enough to bother reporting?
+						world.getLint().canNotImplementLazyTjp.signal(
+						    new String[] {toString()},
+						    getSourceLocation(),
+						    new ISourceLocation[] { munger.getSourceLocation() }
+						);
+					}
 					return false;
 				}
 			}
@@ -1039,8 +1089,10 @@ public class BcelShadow extends Shadow {
 		InstructionList il = new InstructionList();
 
     	if (isThisJoinPointLazy) {
+    		// If we're lazy, build the join point right here.
     		il.append(createThisJoinPoint());
     		
+    		// Does someone else need it? If so, store it for later retrieval
     		if (lazyTjpConsumers > 1) {
 				il.append(thisJoinPointVar.createStore(fact));
 				
@@ -1050,6 +1102,7 @@ public class BcelShadow extends Shadow {
 				il.insert(thisJoinPointVar.createLoad(fact));
     		}
     	} else {			
+    		// If not lazy, its already been built and stored, just retrieve it
 			thisJoinPointVar.appendLoad(il, fact);
     	}