Fix and tests for PR94086 (multiple if PCDs in a pointcut causes massive slow download in matching)

4 files changed, 118 insertions, 16 deletions

diff --git a/tests/bugs150/PR94086.aj b/tests/bugs150/PR94086.aj
new file mode 100644
index 000000000..6991c734e
--- /dev/null
+++ b/tests/bugs150/PR94086.aj
@@ -0,0 +1,73 @@
+public aspect PR94086 {
+    private static final SimpleLogger sl
+        = new SimpleLogger();
+
+    pointcut PC() :
+        (execution(* Test.a(..)) && if (sl.isEnabled()))
+        || (execution(* Test.b(..)) && if (sl.isEnabled()))
+        || (execution(* Test.c(..)) && if (sl.isEnabled()))
+        || (execution(* Test.d(..)) && if (sl.isEnabled()))
+        || (execution(* Test.e(..)) && if (sl.isEnabled()))
+        || (execution(* Test.f(..)) && if (sl.isEnabled()))
+        || (execution(* Test.g(..)) && if (sl.isEnabled()))
+        || (execution(* Test.h(..)) && if (sl.isEnabled()))
+        || (execution(* Test.i(..)) && if (sl.isEnabled()))
+        || (execution(* Test.j(..)) && if (sl.isEnabled()))
+        ;
+
+    before() : PC() {
+        sl.log("Before");
+    }
+
+    after() : PC() {
+        sl.log("After");
+    }
+}
+
+class Test {
+        public void a() {}
+        public void b() {}
+        public void c() {}
+        public void d() {}
+        public void e() {}
+        public void f() {}
+        public void g() {}
+        public void h() {}
+        public void i() {}
+        public void j() {}
+        public void k() {}
+        public void l() {}
+        public void m() {}
+        public void n() {}
+        public void o() {}
+        public void p() {}
+}
+
+
+class SimpleLogger {
+    private boolean enabled;
+
+    public SimpleLogger() {
+        enabled = false;
+    }
+
+    public void disable() {
+        enabled = false;
+    }
+
+    public void enable() {
+        enabled = true;
+    }
+
+    public boolean isEnabled() {
+        return enabled;
+    }
+
+    public void log(String str) {
+        if (enabled) {
+            System.out.println("> Log: " + str);
+        }
+
+    }
+
+}
diff --git a/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
index 4bae9cf4c..919621e80 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
@@ -165,6 +165,14 @@ public class Ajc150Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   public void testInternalCompilerError_pr86832() {
 	  runTest("Internal compiler error");
   }
+
+  /**
+   * IfPointcut.findResidueInternal() was modified to make this test complete in a short amount
+   * of time - if you see it hanging, someone has messed with the optimization.
+   */
+  public void testIfEvaluationExplosiion_PR94086() {
+	  runTest("Exploding compile time with if() statements in pointcut");
+  }
   
   // helper methods.....
   
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
index 2bb4efb95..7f114ea56 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
@@ -2196,4 +2196,8 @@
    <ajc-test dir="bugs150" title="Internal compiler error">
      <compile files="PR86832.aj" options="-1.5"/>
    </ajc-test>
+   
+   <ajc-test dir="bugs150" title="Exploding compile time with if() statements in pointcut">
+     <compile files="PR94086.aj" options="-1.5"/>
+   </ajc-test>
 </suite>
\ No newline at end of file
diff --git a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
index c7b3e02b0..39d2fdb8c 100644
--- a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
@@ -134,29 +134,43 @@ public class IfPointcut extends Pointcut {
 
 	//??? The implementation of name binding and type checking in if PCDs is very convoluted
 	//    There has to be a better way...
-	private boolean findingResidue = false;
+
+    private boolean findingResidue = false;
+	
+	// Similar to lastMatchedShadowId - but only for if PCDs.
+	private int ifLastMatchedShadowId;
+	private Test ifLastMatchedShadowResidue;
+	
+	/**
+	 * At each shadow that matched, the residue can be different.
+	 */
 	protected Test findResidueInternal(Shadow shadow, ExposedState state) {
 		if (findingResidue) return Literal.TRUE;
 		findingResidue = true;
 		try {
-			ExposedState myState = new ExposedState(baseArgsCount);
-			//System.out.println(residueSource);
-			//??? we throw out the test that comes from this walk.  All we want here
-			//    is bindings for the arguments
-			residueSource.findResidue(shadow, myState);
-			
-			//System.out.println(myState);
+
+			// Have we already been asked this question?
+			if (shadow.shadowId == ifLastMatchedShadowId) return ifLastMatchedShadowResidue;
 			
 			Test ret = Literal.TRUE;
-			
 			List args = new ArrayList();
-	        for (int i=0; i < baseArgsCount; i++) {
-	        	Var v = myState.get(i);
-	        	args.add(v);
-	        	ret = Test.makeAnd(ret, 
-	        		Test.makeInstanceof(v, 
-	        			testMethod.getParameterTypes()[i].resolve(shadow.getIWorld())));
-	        }
+			
+			// If there are no args to sort out, don't bother with the recursive call
+			if (baseArgsCount > 0) {
+				ExposedState myState = new ExposedState(baseArgsCount);
+				//??? we throw out the test that comes from this walk.  All we want here
+				//    is bindings for the arguments
+				residueSource.findResidue(shadow, myState);
+				
+				
+		        for (int i=0; i < baseArgsCount; i++) {
+		        	Var v = myState.get(i);
+		        	args.add(v);
+		        	ret = Test.makeAnd(ret, 
+		        		Test.makeInstanceof(v, 
+		        			testMethod.getParameterTypes()[i].resolve(shadow.getIWorld())));
+		        }
+			}
 	
 	        // handle thisJoinPoint parameters
 	        if ((extraParameterFlags & Advice.ThisJoinPoint) != 0) {
@@ -173,6 +187,9 @@ public class IfPointcut extends Pointcut {
 	        
 	        ret = Test.makeAnd(ret, Test.makeCall(testMethod, (Expr[])args.toArray(new Expr[args.size()])));
 
+			// Remember...
+			ifLastMatchedShadowId = shadow.shadowId;
+			ifLastMatchedShadowResidue = ret;
 			return ret; 
 			
 		} finally {