From 12b90faa3eb1ad3496d24605fcb0187e7a551083 Mon Sep 17 00:00:00 2001 From: aclement Date: Fri, 11 Nov 2005 11:24:10 +0000 Subject: [PATCH] fixes for pr113447 - dont do bindings check at each shadow. --- .../org/aspectj/weaver/bcel/BcelWeaver.java | 3 ++ .../weaver/patterns/AnnotationPointcut.java | 13 +-------- .../aspectj/weaver/patterns/ArgsPointcut.java | 13 --------- .../aspectj/weaver/patterns/ExposedState.java | 28 +++++++++++++++---- .../ThisOrTargetAnnotationPointcut.java | 13 +-------- .../weaver/patterns/ThisOrTargetPointcut.java | 24 ++++------------ .../patterns/WithinAnnotationPointcut.java | 12 -------- .../WithinCodeAnnotationPointcut.java | 12 -------- 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; -- 2.39.5