test update and fix for 138223: funky annotation matching

4 files changed, 63 insertions, 29 deletions

diff --git a/tests/bugs152/pr138223/DoubleAnnotationMatching.aj b/tests/bugs152/pr138223/DoubleAnnotationMatching.aj
index 415309421..ad163d007 100644
--- a/tests/bugs152/pr138223/DoubleAnnotationMatching.aj
+++ b/tests/bugs152/pr138223/DoubleAnnotationMatching.aj
@@ -1,33 +1,48 @@
 import java.lang.annotation.*;
 
-@Retention(RetentionPolicy.RUNTIME)
-@interface Tx {
-  boolean readOnly() default false;
-}
+@Retention(RetentionPolicy.RUNTIME) @interface Tx {boolean value() default false;}
 
 public aspect DoubleAnnotationMatching {
-
-
- pointcut methodInTxType(Tx tx) : 
-   execution(* *(..)) && @this(tx) && if(tx.readOnly());
+  pointcut methodInTxType(Tx tx) : 
+    execution(* *(..)) && @this(tx) && if(tx.value());
    
- pointcut txMethod(Tx tx) :
-   execution(* *(..)) && @annotation(tx) && if(tx.readOnly());
+  pointcut txMethod(Tx tx) :
+    execution(* *(..)) && @annotation(tx) && if(tx.value());
    
- pointcut transactionalOperation() :
-   methodInTxType(Tx) || txMethod(Tx);
+  pointcut transactionalOperation() :
+    methodInTxType(Tx) || txMethod(Tx);
    
- before() : transactionalOperation() {
-   // do something
- }
-
+  before() : transactionalOperation() {
+    System.err.println("advice running at "+thisJoinPoint);
+  }
+
+  public static void main(String [] argv) {
+	  new Foo().a();
+	  new Foo().b();
+	  new Foo().c();
+	  new TxTrueFoo().a();
+	  new TxTrueFoo().b();
+	  new TxTrueFoo().c();
+	  new TxFalseFoo().a();
+	  new TxFalseFoo().b();
+	  new TxFalseFoo().c();
+  }
 }
 
-@Tx class Foo {
-
-  public void foo() {}
-  
-  @Tx public void bar() {}
+@Tx(true) class TxTrueFoo {
+  @Tx(true) public void a() {}
+  @Tx(false) public void b() {}
+  public void c() {}
+}
 
+@Tx(false) class TxFalseFoo {
+  @Tx(true) public void a() {}
+  @Tx(false) public void b() {}
+  public void c() {}
+}
 
+class Foo {
+  @Tx(true) public void a() {}
+  @Tx(false) public void b() {}
+  public void c() {}
 }
\ No newline at end of file
diff --git a/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java b/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java
index 178d54058..645a7f0a5 100644
--- a/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc152/Ajc152Tests.java
@@ -51,7 +51,7 @@ public class Ajc152Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
   public void testReferencePCutInDeclareWarning_pr138215() { runTest("Reference pointcut fails inside @DeclareWarning");}
   public void testReferencePCutInPerClause_pr138219() { runTest("Can't use a FQ Reference pointcut in any pointcut expression referenced by a per-clause");}
   public void testReferencePCutInPerClause_pr130722() { runTest("FQ Reference pointcut from perclause ref pc"); }
-//  public void testDoubleAnnotationMatching_pr138223() { runTest("Double at annotation matching (no binding)");}
+  public void testDoubleAnnotationMatching_pr138223() { runTest("Double at annotation matching (no binding)");}
   public void testSuperCallsInAtAspectJAdvice_pr139749() { runTest("Super calls in @AspectJ advice");}
 
   public void testNoClassCastExceptionWithPerThis_pr138286() { runTest("No ClassCastException with perThis");}
diff --git a/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml b/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml
index 6407865fb..122af5fdd 100644
--- a/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml
+++ b/tests/src/org/aspectj/systemtest/ajc152/ajc152.xml
@@ -250,6 +250,15 @@
     <ajc-test dir="bugs152/pr138223" pr="138223" title="Double at annotation matching (no binding)">
       <compile files="DoubleAnnotationMatching.aj" options="-1.5">
       </compile>
+      <run class="DoubleAnnotationMatching">
+        <stderr>
+          <line text="advice running at execution(void Foo.a())"/>
+          <line text="advice running at execution(void TxTrueFoo.a())"/>
+          <line text="advice running at execution(void TxTrueFoo.b())"/>
+          <line text="advice running at execution(void TxTrueFoo.c())"/>
+          <line text="advice running at execution(void TxFalseFoo.a())"/>
+        </stderr>
+      </run>
     </ajc-test>
 
     <ajc-test dir="bugs152/pr138286" pr="138286" title="No ClassCastException with perThis">
diff --git a/weaver/src/org/aspectj/weaver/patterns/AnnotationPointcut.java b/weaver/src/org/aspectj/weaver/patterns/AnnotationPointcut.java
index 883f93fb6..ede464f11 100644
--- a/weaver/src/org/aspectj/weaver/patterns/AnnotationPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/AnnotationPointcut.java
@@ -22,7 +22,6 @@ import org.aspectj.bridge.MessageUtil;
 import org.aspectj.util.FuzzyBoolean;
 import org.aspectj.weaver.AjcMemberMaker;
 import org.aspectj.weaver.AnnotatedElement;
-import org.aspectj.weaver.BCException;
 import org.aspectj.weaver.ISourceContext;
 import org.aspectj.weaver.IntMap;
 import org.aspectj.weaver.Member;
@@ -194,12 +193,23 @@ public class AnnotationPointcut extends NameBindingPointcut {
 			UnresolvedType annotationType = btp.getAnnotationType();
 			Var var = shadow.getKindedAnnotationVar(annotationType);
 			
-			// This should not happen, we shouldn't have gotten this far 
-			// if we weren't going to find the annotation
-			if (var == null) throw new BCException("Impossible! annotation=["+annotationType+
-					                               "]  shadow=["+shadow+" at "+shadow.getSourceLocation()+
-												   "]    pointcut is at ["+getSourceLocation()+"]");//return Literal.FALSE;
-
+			// At this point, var *could* be null.  The only reason this could happen (if we aren't failing...)
+			// is if another binding annotation designator elsewhere in the pointcut is going to expose the annotation
+			// eg.  (execution(* a*(..)) && @annotation(foo)) || (execution(* b*(..)) && @this(foo))
+			// where sometimes @annotation will be providing the value, and sometimes
+			// @this will be providing the value (see pr138223)
+			
+			// If we are here for other indecipherable reasons (it's not the case above...) then
+			// you might want to uncomment this next bit of code to collect the diagnostics
+//			if (var == null) throw new BCException("Impossible! annotation=["+annotationType+
+//					                               "]  shadow=["+shadow+" at "+shadow.getSourceLocation()+
+//												 "]    pointcut is at ["+getSourceLocation()+"]");
+			if (var==null) {
+				if (matchInternal(shadow).alwaysTrue()) 
+					return Literal.TRUE;
+				else 
+					return Literal.FALSE;
+			}
 			state.set(btp.getFormalIndex(),var);
 		}
 		if (matchInternal(shadow).alwaysTrue())