disambiguating bindings

10 files changed, 151 insertions, 1 deletions

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index fe6392bcb..badb21f66 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -72,11 +72,13 @@ import org.aspectj.weaver.patterns.ConcreteCflowPointcut;
 import org.aspectj.weaver.patterns.DeclareParents;
 import org.aspectj.weaver.patterns.FastMatchInfo;
 import org.aspectj.weaver.patterns.IfPointcut;
+import org.aspectj.weaver.patterns.KindedPointcut;
 import org.aspectj.weaver.patterns.NameBindingPointcut;
 import org.aspectj.weaver.patterns.NotPointcut;
 import org.aspectj.weaver.patterns.OrPointcut;
 import org.aspectj.weaver.patterns.Pointcut;
 import org.aspectj.weaver.patterns.PointcutRewriter;
+import org.aspectj.weaver.patterns.WithinPointcut;
 
 
 public class BcelWeaver implements IWeaver {
@@ -480,7 +482,7 @@ public class BcelWeaver implements IWeaver {
     	}
 		Set kindsInCommon = left.couldMatchKinds();
 		kindsInCommon.retainAll(right.couldMatchKinds());
-		if (!kindsInCommon.isEmpty()) {
+		if (!kindsInCommon.isEmpty() && couldEverMatchSameJoinPoints(left,right)) {
 			// we know that every branch binds every formal, so there is no ambiguity
 			// if each branch binds it in exactly the same way...
 			List ambiguousNames = new ArrayList();
@@ -561,6 +563,41 @@ public class BcelWeaver implements IWeaver {
     	}
     }
     
+    
+    // By returning false from this method, we are allowing binding of the same
+    // variable on either side of an or.
+    // Be conservative :- have to consider overriding, varargs, autoboxing,
+    // the effects of itds (on within for example), interfaces, the fact that
+    // join points can have multiple signatures and so on.
+    private boolean couldEverMatchSameJoinPoints(Pointcut left, Pointcut right) {
+    	if ((left instanceof OrPointcut) || (right instanceof OrPointcut)) return true;
+    	// look for withins
+    	WithinPointcut leftWithin = (WithinPointcut) findFirstPointcutIn(left,WithinPointcut.class);
+    	WithinPointcut rightWithin = (WithinPointcut) findFirstPointcutIn(right,WithinPointcut.class);
+    	if ((leftWithin != null) && (rightWithin != null)) {
+    		if (!leftWithin.couldEverMatchSameJoinPointsAs(rightWithin)) return false;
+    	}
+    	// look for kinded
+    	KindedPointcut leftKind = (KindedPointcut) findFirstPointcutIn(left,KindedPointcut.class);
+    	KindedPointcut rightKind = (KindedPointcut) findFirstPointcutIn(right,KindedPointcut.class);
+    	if ((leftKind != null) && (rightKind != null)) {
+    		if (!leftKind.couldEverMatchSameJoinPointsAs(rightKind)) return false;
+    	}
+    	return true;
+    }
+    
+    private Pointcut findFirstPointcutIn(Pointcut toSearch, Class toLookFor) {
+    	if (toSearch instanceof NotPointcut) return null;
+    	if (toLookFor.isInstance(toSearch)) return toSearch;
+    	if (toSearch instanceof AndPointcut) {
+    		AndPointcut apc = (AndPointcut) toSearch;
+    		Pointcut left = findFirstPointcutIn(apc.getLeft(),toLookFor);
+    		if (left != null) return left;
+    		return findFirstPointcutIn(apc.getRight(),toLookFor);
+    	}
+    	return null;
+    }
+    
     /**
 	 * @param userPointcut
 	 */
diff --git a/weaver/src/org/aspectj/weaver/patterns/AndTypePattern.java b/weaver/src/org/aspectj/weaver/patterns/AndTypePattern.java
index e3bb565b2..5258151fa 100644
--- a/weaver/src/org/aspectj/weaver/patterns/AndTypePattern.java
+++ b/weaver/src/org/aspectj/weaver/patterns/AndTypePattern.java
@@ -39,6 +39,9 @@ public class AndTypePattern extends TypePattern {
 		setLocation(left.getSourceContext(), left.getStart(), right.getEnd());
 	}
 
+	protected boolean couldEverMatchSameTypesAs(TypePattern other) {
+		return true; // don't dive into ands yet....
+	}
 	public FuzzyBoolean matchesInstanceof(ResolvedTypeX type) {
 		return left.matchesInstanceof(type).and(right.matchesInstanceof(type));
 	}
diff --git a/weaver/src/org/aspectj/weaver/patterns/ExactTypePattern.java b/weaver/src/org/aspectj/weaver/patterns/ExactTypePattern.java
index 4b2ca0c36..864a896da 100644
--- a/weaver/src/org/aspectj/weaver/patterns/ExactTypePattern.java
+++ b/weaver/src/org/aspectj/weaver/patterns/ExactTypePattern.java
@@ -68,6 +68,26 @@ public class ExactTypePattern extends TypePattern {
 		this.type = type;
 	}
 	
+	/* (non-Javadoc)
+	 * @see org.aspectj.weaver.patterns.TypePattern#couldEverMatchSameTypesAs(org.aspectj.weaver.patterns.TypePattern)
+	 */
+	protected boolean couldEverMatchSameTypesAs(TypePattern other) {
+		if (super.couldEverMatchSameTypesAs(other)) return true;
+		// false is necessary but not sufficient
+		TypeX otherType = other.getExactType();
+		if (otherType != ResolvedTypeX.MISSING) {
+			return type.equals(otherType);
+		} 
+		if (other instanceof WildTypePattern) {
+			WildTypePattern owtp = (WildTypePattern) other;
+			String yourSimpleNamePrefix = owtp.namePatterns[0].maybeGetSimpleName();
+			if (yourSimpleNamePrefix != null) {
+				return (type.getName().startsWith(yourSimpleNamePrefix));
+			}
+		}
+		return true;
+	}
+	
 	protected boolean matchesExactly(ResolvedTypeX matchType) {
 		return this.type.equals(matchType);
 	}
diff --git a/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java b/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java
index 8886e3209..611dbe4f6 100644
--- a/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java
@@ -67,6 +67,26 @@ public class KindedPointcut extends Pointcut {
 		return matchKinds;
 	}
 	
+	public boolean couldEverMatchSameJoinPointsAs(KindedPointcut other) {
+		if (this.kind != other.kind) return false;
+		String myName = signature.getName().maybeGetSimpleName();
+		String yourName = other.signature.getName().maybeGetSimpleName();
+		if (myName != null && yourName != null) {
+			if ( !myName.equals(yourName)) {
+				return false;
+			}
+		}
+		if (signature.getParameterTypes().ellipsisCount == 0) {
+			if (other.signature.getParameterTypes().ellipsisCount == 0) {
+				if (signature.getParameterTypes().getTypePatterns().length !=
+					other.signature.getParameterTypes().getTypePatterns().length) {
+					return false;
+				}
+			}
+		}
+		return true;
+	}
+	
     public FuzzyBoolean fastMatch(FastMatchInfo info) {
     	if (info.getKind() != null) {
 			if (info.getKind() != kind) return FuzzyBoolean.NO;
diff --git a/weaver/src/org/aspectj/weaver/patterns/NotTypePattern.java b/weaver/src/org/aspectj/weaver/patterns/NotTypePattern.java
index 8a0d2c619..2e6ab0a43 100644
--- a/weaver/src/org/aspectj/weaver/patterns/NotTypePattern.java
+++ b/weaver/src/org/aspectj/weaver/patterns/NotTypePattern.java
@@ -39,6 +39,13 @@ public class NotTypePattern extends TypePattern {
 		setLocation(pattern.getSourceContext(), pattern.getStart(), pattern.getEnd());
 	}
 
+	/* (non-Javadoc)
+	 * @see org.aspectj.weaver.patterns.TypePattern#couldEverMatchSameTypesAs(org.aspectj.weaver.patterns.TypePattern)
+	 */
+	protected boolean couldEverMatchSameTypesAs(TypePattern other) {
+		return true;
+	}
+	
 	public FuzzyBoolean matchesInstanceof(ResolvedTypeX type) {
 		return pattern.matchesInstanceof(type).not();
 	}
diff --git a/weaver/src/org/aspectj/weaver/patterns/OrPointcut.java b/weaver/src/org/aspectj/weaver/patterns/OrPointcut.java
index 2eca6de01..815b8a8a7 100644
--- a/weaver/src/org/aspectj/weaver/patterns/OrPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/OrPointcut.java
@@ -26,6 +26,7 @@ import org.aspectj.weaver.ISourceContext;
 import org.aspectj.weaver.IntMap;
 import org.aspectj.weaver.ResolvedTypeX;
 import org.aspectj.weaver.Shadow;
+import org.aspectj.weaver.ast.Literal;
 import org.aspectj.weaver.ast.Test;
 
 public class OrPointcut extends Pointcut {
diff --git a/weaver/src/org/aspectj/weaver/patterns/OrTypePattern.java b/weaver/src/org/aspectj/weaver/patterns/OrTypePattern.java
index d071c942d..4d8e6316c 100644
--- a/weaver/src/org/aspectj/weaver/patterns/OrTypePattern.java
+++ b/weaver/src/org/aspectj/weaver/patterns/OrTypePattern.java
@@ -39,6 +39,13 @@ public class OrTypePattern extends TypePattern {
 		setLocation(left.getSourceContext(), left.getStart(), right.getEnd());
 	}
 
+	/* (non-Javadoc)
+	 * @see org.aspectj.weaver.patterns.TypePattern#couldEverMatchSameTypesAs(org.aspectj.weaver.patterns.TypePattern)
+	 */
+	protected boolean couldEverMatchSameTypesAs(TypePattern other) {
+		return true; // don't dive at the moment...
+	}
+	
 	public FuzzyBoolean matchesInstanceof(ResolvedTypeX type) {
 		return left.matchesInstanceof(type).or(right.matchesInstanceof(type));
 	}
diff --git a/weaver/src/org/aspectj/weaver/patterns/TypePattern.java b/weaver/src/org/aspectj/weaver/patterns/TypePattern.java
index 1295be59d..a0c42e19e 100644
--- a/weaver/src/org/aspectj/weaver/patterns/TypePattern.java
+++ b/weaver/src/org/aspectj/weaver/patterns/TypePattern.java
@@ -67,6 +67,14 @@ public abstract class TypePattern extends PatternNode {
 		this.annotationPattern = annPatt;
 	}
 	
+	// answer conservatively...
+	protected boolean couldEverMatchSameTypesAs(TypePattern other) {
+		if (this.includeSubtypes || other.includeSubtypes) return true;
+		if (this.annotationPattern != AnnotationTypePattern.ANY) return true;
+		if (other.annotationPattern != AnnotationTypePattern.ANY) return true;
+		return false;
+	}
+	
 	//XXX non-final for Not, && and ||
 	public boolean matchesStatically(ResolvedTypeX type) {
 		if (includeSubtypes) {
@@ -284,6 +292,12 @@ class EllipsisTypePattern extends TypePattern {
 		super(false,false);
 	}
 
+	/* (non-Javadoc)
+	 * @see org.aspectj.weaver.patterns.TypePattern#couldEverMatchSameTypesAs(org.aspectj.weaver.patterns.TypePattern)
+	 */
+	protected boolean couldEverMatchSameTypesAs(TypePattern other) {
+		return true;
+	}
 	/**
 	 * @see org.aspectj.weaver.patterns.TypePattern#matchesExactly(IType)
 	 */
@@ -345,6 +359,12 @@ class AnyTypePattern extends TypePattern {
 		super(false,false);
 	}
 
+	/* (non-Javadoc)
+	 * @see org.aspectj.weaver.patterns.TypePattern#couldEverMatchSameTypesAs(org.aspectj.weaver.patterns.TypePattern)
+	 */
+	protected boolean couldEverMatchSameTypesAs(TypePattern other) {
+		return true;
+	}
 	/**
 	 * @see org.aspectj.weaver.patterns.TypePattern#matchesExactly(IType)
 	 */
@@ -415,6 +435,13 @@ class NoTypePattern extends TypePattern {
 		super(false,false);
 	}
 
+	
+	/* (non-Javadoc)
+	 * @see org.aspectj.weaver.patterns.TypePattern#couldEverMatchSameTypesAs(org.aspectj.weaver.patterns.TypePattern)
+	 */
+	protected boolean couldEverMatchSameTypesAs(TypePattern other) {
+		return false;
+	}
 	/**
 	 * @see org.aspectj.weaver.patterns.TypePattern#matchesExactly(IType)
 	 */
diff --git a/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java b/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java
index 02344c47e..105d8aaca 100644
--- a/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java
+++ b/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java
@@ -65,6 +65,30 @@ public class WildTypePattern extends TypePattern {
 		this.isVarArgs = isVarArg;
 	}
 
+	/* (non-Javadoc)
+	 * @see org.aspectj.weaver.patterns.TypePattern#couldEverMatchSameTypesAs(org.aspectj.weaver.patterns.TypePattern)
+	 */
+	protected boolean couldEverMatchSameTypesAs(TypePattern other) {
+		if (super.couldEverMatchSameTypesAs(other)) return true;
+		// false is necessary but not sufficient
+		TypeX otherType = other.getExactType();
+		if (otherType != ResolvedTypeX.MISSING) {
+			if (namePatterns.length > 0) {
+				if (!namePatterns[0].matches(otherType.getName())) return false;
+			}
+		} 
+		if (other instanceof WildTypePattern) {
+			WildTypePattern owtp = (WildTypePattern) other;
+			String mySimpleName = namePatterns[0].maybeGetSimpleName();
+			String yourSimpleName = owtp.namePatterns[0].maybeGetSimpleName();
+			if (mySimpleName != null && yourSimpleName != null) {
+				return (mySimpleName.startsWith(yourSimpleName) ||
+						yourSimpleName.startsWith(mySimpleName));
+			}
+		}
+		return true;
+	}
+	
 	//XXX inefficient implementation
 	public static char[][] splitNames(String s) {
 		List ret = new ArrayList();
diff --git a/weaver/src/org/aspectj/weaver/patterns/WithinPointcut.java b/weaver/src/org/aspectj/weaver/patterns/WithinPointcut.java
index edda70510..f622b8088 100644
--- a/weaver/src/org/aspectj/weaver/patterns/WithinPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/WithinPointcut.java
@@ -136,6 +136,10 @@ public class WithinPointcut extends Pointcut {
 		typePattern.postRead(enclosingType);
 	}
 
+	public boolean couldEverMatchSameJoinPointsAs(WithinPointcut other) {
+		return typePattern.couldEverMatchSameTypesAs(other.typePattern);
+	}
+	
 	public boolean equals(Object other) {
 		if (!(other instanceof WithinPointcut)) return false;
 		WithinPointcut o = (WithinPointcut)other;