fixes for pr113447 - dont do bindings check at each shadow.

8 files changed, 33 insertions, 85 deletions

diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index c2049533b..8d730932c 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -437,6 +437,9 @@ public class BcelWeaver implements IWeaver {
 
 		shadowMungerList = xcutSet.getShadowMungers();
 		rewritePointcuts(shadowMungerList);
+		// Sometimes an error occurs during rewriting pointcuts (for example, if ambiguous bindings
+		// are detected) - we ought to fail the prepare when this happens because continuing with
+		// inconsistent pointcuts could lead to problems
 		typeMungerList = xcutSet.getTypeMungers();
         lateTypeMungerList = xcutSet.getLateTypeMungers();
 		declareParentsList = xcutSet.getDeclareParents();
diff --git a/weaver/src/org/aspectj/weaver/patterns/AnnotationPointcut.java b/weaver/src/org/aspectj/weaver/patterns/AnnotationPointcut.java
index 3cf61459a..b6ee8ee08 100644
--- a/weaver/src/org/aspectj/weaver/patterns/AnnotationPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/AnnotationPointcut.java
@@ -197,18 +197,7 @@ public class AnnotationPointcut extends NameBindingPointcut {
 			if (var == null) throw new BCException("Impossible! annotation=["+annotationType+
 					                               "]  shadow=["+shadow+" at "+shadow.getSourceLocation()+
 												   "]    pointcut is at ["+getSourceLocation()+"]");//return Literal.FALSE;
-			// Check if we have already bound something to this formal
-			if ((state.get(btp.getFormalIndex())!=null) &&(lastMatchedShadowId == shadow.shadowId)) {
-//				ISourceLocation pcdSloc = getSourceLocation(); 
-//				ISourceLocation shadowSloc = shadow.getSourceLocation();
-//				Message errorMessage = new Message(
-//					"Cannot use @pointcut to match at this location and bind a formal to type '"+var.getType()+
-//					"' - the formal is already bound to type '"+state.get(btp.getFormalIndex()).getType()+"'"+
-//					".  The secondary source location points to the problematic binding.",
-//					shadowSloc,true,new ISourceLocation[]{pcdSloc}); 
-//				shadow.getIWorld().getMessageHandler().handleMessage(errorMessage);
-				state.setErroneousVar(btp.getFormalIndex());
-			}
+
 			state.set(btp.getFormalIndex(),var);
 		}
 		if (matchInternal(shadow).alwaysTrue()) 
diff --git a/weaver/src/org/aspectj/weaver/patterns/ArgsPointcut.java b/weaver/src/org/aspectj/weaver/patterns/ArgsPointcut.java
index c8bdee759..6ddb09e7d 100644
--- a/weaver/src/org/aspectj/weaver/patterns/ArgsPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/ArgsPointcut.java
@@ -212,19 +212,6 @@ public class ArgsPointcut extends NameBindingPointcut {
 				if (type.matchesInstanceof(argRTX).alwaysTrue()) {
 					continue;
 				}
-			} else {
-			  BindingTypePattern btp = (BindingTypePattern)type;
-			  // Check if we have already bound something to this formal
-			  if ((state.get(btp.getFormalIndex())!=null) &&(lastMatchedShadowId != shadow.shadowId)) {
-//			  	ISourceLocation isl = getSourceLocation();
-//				Message errorMessage = new Message(
-//                    "Ambiguous binding of type "+type.getExactType().toString()+
-//                    " using args(..) at this line - formal is already bound"+
-//                    ".  See secondary source location for location of args(..)",
-//					shadow.getSourceLocation(),true,new ISourceLocation[]{getSourceLocation()});
-//				shadow.getIWorld().getMessageHandler().handleMessage(errorMessage);
-				state.setErroneousVar(btp.getFormalIndex());
-			  }
 			}
 
 			World world = shadow.getIWorld();
diff --git a/weaver/src/org/aspectj/weaver/patterns/ExposedState.java b/weaver/src/org/aspectj/weaver/patterns/ExposedState.java
index 03dd40e15..230abdb69 100644
--- a/weaver/src/org/aspectj/weaver/patterns/ExposedState.java
+++ b/weaver/src/org/aspectj/weaver/patterns/ExposedState.java
@@ -16,6 +16,8 @@ package org.aspectj.weaver.patterns;
 import java.util.Arrays;
 
 import org.aspectj.weaver.Member;
+import org.aspectj.weaver.ResolvedType;
+import org.aspectj.weaver.UnresolvedType;
 import org.aspectj.weaver.ast.Expr;
 import org.aspectj.weaver.ast.Var;
 
@@ -23,6 +25,7 @@ public class ExposedState {
 	public Var[] vars;
 	private boolean[] erroneousVars;
 	private Expr aspectInstance;
+	private UnresolvedType[] expectedVarTypes; // enables us to check that binding is occurring with the *right* types
 
 	public ExposedState(int size) {
 		super();
@@ -33,6 +36,13 @@ public class ExposedState {
 	public ExposedState(Member signature) {
 		// XXX there maybe something about target for non-static sigs
 		this(signature.getParameterTypes().length);
+		expectedVarTypes = new UnresolvedType[signature.getParameterTypes().length];
+		if (expectedVarTypes.length>0) {
+			for (int i = 0; i < signature.getParameterTypes().length; i++) {
+				expectedVarTypes[i] = signature.getParameterTypes()[i];
+			}
+		}
+		
 	}
 	
 	public boolean isFullySetUp() {
@@ -43,12 +53,18 @@ public class ExposedState {
 	}
 
 	public void set(int i, Var var) {
-		//XXX add sanity checks
-		// Some checks added in ArgsPointcut and ThisOrTargetPointcut
-//		if (vars[i]!=null) {
-//			if (!var.getType().equals(vars[i].getType()))
-//			  throw new RuntimeException("Shouldn't allow a slot to change type! Currently="+var.getType()+"   New="+vars[i].getType());
-//		}
+		// check the type is OK if we can... these are the same rules as in matchesInstanceOf() processing
+		if (expectedVarTypes!=null) {
+			ResolvedType expected = expectedVarTypes[i].resolve(var.getType().getWorld());
+			if (!expected.equals(ResolvedType.OBJECT)) {
+				if (!expected.isAssignableFrom(var.getType())) {
+					if (!var.getType().isCoerceableFrom(expected)) {
+//						throw new BCException("Expected type "+expectedVarTypes[i]+" in slot "+i+" but attempt to put "+var.getType()+" into it");
+						return;
+					}
+				}
+			}
+		}
 		vars[i] = var;
 	}
     public Var get(int i) {
diff --git a/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetAnnotationPointcut.java b/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetAnnotationPointcut.java
index 72c262c64..54f26a01d 100644
--- a/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetAnnotationPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetAnnotationPointcut.java
@@ -192,18 +192,7 @@ public class ThisOrTargetAnnotationPointcut extends NameBindingPointcut {
 				shadow.getTargetAnnotationVar(annotationType);
 			if (annVar == null)
 				throw new RuntimeException("Impossible!");
-			// Check if we have already bound something to this formal
-			if ((state.get(btp.getFormalIndex())!=null) &&(lastMatchedShadowId == shadow.shadowId)) {
-//				ISourceLocation pcdSloc = getSourceLocation(); 
-//				ISourceLocation shadowSloc = shadow.getSourceLocation();
-//				Message errorMessage = new Message(
-//					"Cannot use @pointcut to match at this location and bind a formal to type '"+annVar.getType()+
-//					"' - the formal is already bound to type '"+state.get(btp.getFormalIndex()).getType()+"'"+
-//					".  The secondary source location points to the problematic binding.",
-//					shadowSloc,true,new ISourceLocation[]{pcdSloc}); 
-//				shadow.getIWorld().getMessageHandler().handleMessage(errorMessage);
-				state.setErroneousVar(btp.getFormalIndex());
-			}
+
 			state.set(btp.getFormalIndex(),annVar);
 		}
 
diff --git a/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetPointcut.java b/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetPointcut.java
index da23506ea..916fff935 100644
--- a/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetPointcut.java
@@ -169,31 +169,19 @@ public class ThisOrTargetPointcut extends NameBindingPointcut {
 		return (isThis ? "this(" : "target(") + type + ")";
 	}
 
+	/** 
+	 * Residue is the remainder of the pointcut match that couldn't be
+	 * performed with the purely static information at compile time and
+	 * this method returns the residue of a pointcut at a particular shadow.
+	 */
 	protected Test findResidueInternal(Shadow shadow, ExposedState state) {
 		if (!couldMatch(shadow)) return Literal.FALSE;
 		
+		// if no preference is specified, just say TRUE which means no residue
 		if (type == TypePattern.ANY) return Literal.TRUE;
 		
 		Var var = isThis ? shadow.getThisVar() : shadow.getTargetVar();	
 
-		if (type instanceof BindingTypePattern) {
-		  BindingTypePattern btp = (BindingTypePattern)type;
-		  // Check if we have already bound something to this formal
-		  Var existingVarInThisSlot = state.get(btp.getFormalIndex());
-		  
-		  if (existingVarInThisSlot != null ) {
-			  
-			// Is it already bound to exactly the same thing?
-			if (existingVarInThisSlot.equals(var)) return Literal.TRUE;
-			
-			// If state.get() returned non-null then someone has already bound the variable in that slot at
-			// the shadow 'shadow.shadowId'.  If our 'lastMatchedShadowId' is not the same as 'shadow.shadowId'
-			// then this pointcut wasn't involved in matching and so shouldn't contribute to binding - so just
-			// return Literal.TRUE meaning 'no residue'
-		    if (lastMatchedShadowId != shadow.shadowId) return Literal.TRUE;
-			state.setErroneousVar(btp.getFormalIndex());
-		  }
-		}
 		return exposeStateForVar(var, type, state, shadow.getIWorld());
 	}
 
diff --git a/weaver/src/org/aspectj/weaver/patterns/WithinAnnotationPointcut.java b/weaver/src/org/aspectj/weaver/patterns/WithinAnnotationPointcut.java
index fc87efde1..c15d35787 100644
--- a/weaver/src/org/aspectj/weaver/patterns/WithinAnnotationPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/WithinAnnotationPointcut.java
@@ -137,18 +137,6 @@ public class WithinAnnotationPointcut extends NameBindingPointcut {
                         "]  shadow=["+shadow+" at "+shadow.getSourceLocation()+
 						   "]    pointcut is at ["+getSourceLocation()+"]");
 			
-			// Check if we have already bound something to this formal
-			if ((state.get(btp.getFormalIndex())!=null)  &&(lastMatchedShadowId == shadow.shadowId)) {
-//				ISourceLocation pcdSloc = getSourceLocation(); 
-//				ISourceLocation shadowSloc = shadow.getSourceLocation();
-//				Message errorMessage = new Message(
-//					"Cannot use @pointcut to match at this location and bind a formal to type '"+var.getType()+
-//					"' - the formal is already bound to type '"+state.get(btp.getFormalIndex()).getType()+"'"+
-//					".  The secondary source location points to the problematic binding.",
-//					shadowSloc,true,new ISourceLocation[]{pcdSloc}); 
-//				shadow.getIWorld().getMessageHandler().handleMessage(errorMessage);
-				state.setErroneousVar(btp.getFormalIndex());
-			}
 			state.set(btp.getFormalIndex(),var);
 		} 
 		return Literal.TRUE;
diff --git a/weaver/src/org/aspectj/weaver/patterns/WithinCodeAnnotationPointcut.java b/weaver/src/org/aspectj/weaver/patterns/WithinCodeAnnotationPointcut.java
index 7a4bc988f..0a67b285e 100644
--- a/weaver/src/org/aspectj/weaver/patterns/WithinCodeAnnotationPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/WithinCodeAnnotationPointcut.java
@@ -150,18 +150,6 @@ public class WithinCodeAnnotationPointcut extends NameBindingPointcut {
                         "]  shadow=["+shadow+" at "+shadow.getSourceLocation()+
 						   "]    pointcut is at ["+getSourceLocation()+"]");
 				
-			// Check if we have already bound something to this formal
-			if ((state.get(btp.getFormalIndex())!=null)  &&(lastMatchedShadowId == shadow.shadowId)) {
-//				ISourceLocation pcdSloc = getSourceLocation(); 
-//				ISourceLocation shadowSloc = shadow.getSourceLocation();
-//				Message errorMessage = new Message(
-//					"Cannot use @pointcut to match at this location and bind a formal to type '"+var.getType()+
-//					"' - the formal is already bound to type '"+state.get(btp.getFormalIndex()).getType()+"'"+
-//					".  The secondary source location points to the problematic binding.",
-//					shadowSloc,true,new ISourceLocation[]{pcdSloc}); 
-//				shadow.getIWorld().getMessageHandler().handleMessage(errorMessage);
-				state.setErroneousVar(btp.getFormalIndex());
-			}
 			state.set(btp.getFormalIndex(),var);
 		} 
 		return Literal.TRUE;