From 2f85beef220b0b67fefb587b9045d80fd1356da1 Mon Sep 17 00:00:00 2001 From: acolyer Date: Thu, 16 Dec 2004 13:07:38 +0000 Subject: [PATCH] disambiguating bindings --- .../org/aspectj/weaver/bcel/BcelWeaver.java | 39 ++++++++++++++++++- .../weaver/patterns/AndTypePattern.java | 3 ++ .../weaver/patterns/ExactTypePattern.java | 20 ++++++++++ .../weaver/patterns/KindedPointcut.java | 20 ++++++++++ .../weaver/patterns/NotTypePattern.java | 7 ++++ .../aspectj/weaver/patterns/OrPointcut.java | 1 + .../weaver/patterns/OrTypePattern.java | 7 ++++ .../aspectj/weaver/patterns/TypePattern.java | 27 +++++++++++++ .../weaver/patterns/WildTypePattern.java | 24 ++++++++++++ .../weaver/patterns/WithinPointcut.java | 4 ++ 10 files changed, 151 insertions(+), 1 deletion(-) 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; -- 2.39.5