Bugzilla Bug 33635

   Negation of if pointcut does not work

diff --git a/tests/bugs/NotIf.java b/tests/bugs/NotIf.java
new file mode 100644 (file)
index 0000000..3220104
--- /dev/null
+++ b/tests/bugs/NotIf.java
@@ -0,0 +1,49 @@
+// for Bug#:  33635  
+import org.aspectj.testing.Tester;
+
+
+public class NotIf {
+    public static void main(String[] args) {
+               Tester.checkEqual(Aspect1.ranNone, 0, "shouldn't run");
+               Tester.checkEqual(Aspect1.ranTwo, 2, "should run");
+               Tester.checkEqual(Aspect2.ran, 1, "should run with values");
+    }
+}
+
+aspect Aspect1 {
+       static int ranNone = 0;
+       static int ranTwo = 0;
+       
+       static boolean testTrue() { return true; }
+       
+       static boolean testFalse() { return false; }
+       
+       before(): execution(void main(..)) && !if(testTrue()) {
+               ranNone += 1;
+       }
+       
+       before(): execution(void main(..)) && if(!testTrue()) {
+               ranNone += 1;
+       }
+       before(): execution(void main(..)) && !if(testFalse()) {
+               ranTwo += 1;
+       }
+       
+       before(): execution(void main(..)) && if(!testFalse()) {
+               ranTwo += 1;
+       }
+}
+
+aspect Aspect2 {
+       static int ran = 0;
+       
+       static boolean testValues(int i, String s, Object o) {
+               return false;
+       }
+       
+       before(String[] a): execution(void main(String[])) && 
+                               !if(testValues(a.length, a.toString(), a)) && args(a)
+       {
+               ran += 1;
+       }
+}

diff --git a/tests/jimTests.xml b/tests/jimTests.xml
index e7e9b8b977819bb303aa5b176cf0e8f7a5c9bb56..6306ec843a79d56a676d3b942f82100799aa7da3 100644 (file)
--- a/tests/jimTests.xml
+++ b/tests/jimTests.xml
@@ -1,6 +1,13 @@
 <!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd">
 <suite>
 
+    <ajc-test dir="bugs" pr="33635" 
+               title="Negation of if pointcut does not work">
+        <compile files="NotIf.java"/>
+        <run class="NotIf"/>
+    </ajc-test>
+    
+    
     <!--
     
 

diff --git a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
index 4c4beb1ab748984974b22b5114804c167db294da..267e5e00cf35e85439955662eebf74ce6671220e 100644 (file)
--- a/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/IfPointcut.java
@@ -29,6 +29,7 @@ import org.aspectj.weaver.ResolvedPointcutDefinition;
 import org.aspectj.weaver.ResolvedTypeX;
 import org.aspectj.weaver.Shadow;
 import org.aspectj.weaver.ShadowMunger;
+import org.aspectj.weaver.ast.*;
 import org.aspectj.weaver.ast.Expr;
 import org.aspectj.weaver.ast.Literal;
 import org.aspectj.weaver.ast.Test;
@@ -84,6 +85,9 @@ public class IfPointcut extends Pointcut {
                return "if(" + testMethod + ")";
        }
 
+
+       //??? 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;
        public Test findResidue(Shadow shadow, ExposedState state) {
                if (findingResidue) return Literal.TRUE;
@@ -91,15 +95,21 @@ public class IfPointcut extends Pointcut {
                try {
                        ExposedState myState = new ExposedState(baseArgsCount);
                        //System.out.println(residueSource);
-                       //??? some of these tests are preconditions for the if to run correctly
-                       //    this implementation will duplicate those tests, we should be more careful
-                       Test preTest = residueSource.findResidue(shadow, myState); // might need this
+                       //??? 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);
                        
+                       Test ret = Literal.TRUE;
+                       
                        List args = new ArrayList();
                for (int i=0; i < baseArgsCount; i++) {
-                       args.add(myState.get(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
@@ -114,8 +124,10 @@ public class IfPointcut extends Pointcut {
                if ((extraParameterFlags & Advice.ThisEnclosingJoinPointStaticPart) != 0) {
                        args.add(shadow.getThisEnclosingJoinPointStaticPartVar());
                }
-                       Test myTest = Test.makeCall(testMethod, (Expr[])args.toArray(new Expr[args.size()]));
-                       return Test.makeAnd(preTest, myTest);
+               
+               ret = Test.makeAnd(ret, Test.makeCall(testMethod, (Expr[])args.toArray(new Expr[args.size()])));
+
+                       return ret; 
                        
                } finally {
                        findingResidue = false;