Fix and tests for PR94086 (multiple if PCDs in a pointcut causes massive slow downloa...

diff --git a/tests/bugs150/PR94086.aj b/tests/bugs150/PR94086.aj
new file mode 100644 (file)
index 0000000..6991c73
--- /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 4bae9cf4c4cad6660c0c9348599d5026f6650c40..919621e8004c6ebbad7f0ee23cea2c9522c9fc74 100644 (file)
--- 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 2bb4efb9564e8f6cb217a636b91f609db511a870..7f114ea5696860c71c6aca03465758267695d043 100644 (file)
--- 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 c7b3e02b0c8d112e0d833d16176a64717cc86f40..39d2fdb8cd8d1bd5bce217b2af0bd81ea35e73e9 100644 (file)
--- 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 {