From: aclement Date: Tue, 6 Jan 2009 01:55:58 +0000 (+0000) Subject: 259649: bit of refactoring and aspect comparison logic fixed X-Git-Tag: pre268419~251 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=84dbc011f6b45048fd4e57a5147702284d18e059;p=aspectj.git 259649: bit of refactoring and aspect comparison logic fixed --- diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembers.java b/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembers.java index 9cdb78a64..a71512f13 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembers.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/CrosscuttingMembers.java @@ -31,12 +31,11 @@ import org.aspectj.weaver.patterns.Pointcut; import org.aspectj.weaver.patterns.PointcutRewriter; /** - * This holds on to all members that have an invasive effect outside of there - * own compilation unit. These members need to be all gathered up and in a world - * before any weaving can take place. + * This holds on to all members that have an invasive effect outside of there own compilation unit. These members need to be all + * gathered up and in a world before any weaving can take place. * - * They are also important in the compilation process and need to be gathered up - * before the inter-type declaration weaving stage (unsurprisingly). + * They are also important in the compilation process and need to be gathered up before the inter-type declaration weaving stage + * (unsurprisingly). * * All members are concrete. * @@ -52,15 +51,15 @@ public class CrosscuttingMembers { private List typeMungers = new ArrayList(4); private List lateTypeMungers = new ArrayList(0); - private List declareParents = new ArrayList(4); + private Set declareParents = new HashSet(); private List declareSofts = new ArrayList(0); private List declareDominates = new ArrayList(4); // These are like declare parents type mungers - private List declareAnnotationsOnType = new ArrayList(); - private List declareAnnotationsOnField = new ArrayList(); - private List declareAnnotationsOnMethods = new ArrayList(); // includes - // ctors + private Set declareAnnotationsOnType = new HashSet(); + private Set declareAnnotationsOnField = new HashSet(); + private Set declareAnnotationsOnMethods = new HashSet(); + // declareAnnotationsOnMethods includes constructors too private boolean shouldConcretizeIfNeeded = true; @@ -99,9 +98,9 @@ public class CrosscuttingMembers { } public void addTypeMunger(ConcreteTypeMunger m) { - if (m == null) + if (m == null) { throw new Error("FIXME AV - should not happen or what ?");// return; - // //??? + } typeMungers.add(m); } @@ -220,20 +219,15 @@ public class CrosscuttingMembers { } /** - * Updates the records if something has changed. This is called at most - * twice, firstly whilst collecting ITDs and declares. At this point the - * CrosscuttingMembers we're comparing ourselves with doesn't know about - * shadowmungers. Therefore a straight comparison with the existing list of - * shadowmungers would return that something has changed even though it - * might not have, so in this first round we ignore the shadowMungers. The - * second time this is called is whilst we're preparing to weave. At this - * point we know everything in the system and so we're able to compare the - * shadowMunger list. (see bug 129163) + * Updates the records if something has changed. This is called at most twice, firstly whilst collecting ITDs and declares. At + * this point the CrosscuttingMembers we're comparing ourselves with doesn't know about shadowmungers. Therefore a straight + * comparison with the existing list of shadowmungers would return that something has changed even though it might not have, so + * in this first round we ignore the shadowMungers. The second time this is called is whilst we're preparing to weave. At this + * point we know everything in the system and so we're able to compare the shadowMunger list. (see bug 129163) * * @param other * @param careAboutShadowMungers - * @return true if something has changed since the last time this method was - * called, false otherwise + * @return true if something has changed since the last time this method was called, false otherwise */ public boolean replaceWith(CrosscuttingMembers other, boolean careAboutShadowMungers) { boolean changed = false; @@ -352,44 +346,49 @@ public class CrosscuttingMembers { // initial go at equivalence logic rather than set compare (see // pr133532) - // if (theseTypeMungers.size()!=otherTypeMungers.size()) { - // changed = true; - // typeMungers = other.typeMungers; - // } else { - // boolean foundInequality=false; - // for (Iterator iter = theseTypeMungers.iterator(); iter.hasNext() && - // !foundInequality;) { - // Object thisOne = (Object) iter.next(); - // boolean foundInOtherSet = false; - // for (Iterator iterator = otherTypeMungers.iterator(); - // iterator.hasNext();) { - // Object otherOne = (Object) iterator.next(); - // if (thisOne instanceof ConcreteTypeMunger && otherOne instanceof - // ConcreteTypeMunger) { - // if (((ConcreteTypeMunger)thisOne).equivalentTo(otherOne)) { - // foundInOtherSet=true; - // } else if (thisOne.equals(otherOne)) { - // foundInOtherSet=true; - // } - // } else { - // if (thisOne.equals(otherOne)) { - // foundInOtherSet=true; - // } - // } - // } - // if (!foundInOtherSet) foundInequality=true; - // } - // if (foundInequality) { - // changed = true; - // typeMungers = other.typeMungers; - // // } else { - // // typeMungers = other.typeMungers; - // } - // } - if (!theseTypeMungers.equals(otherTypeMungers)) { + if (theseTypeMungers.size() != otherTypeMungers.size()) { changed = true; typeMungers = other.typeMungers; + } else { + boolean shouldOverwriteThis = false; + boolean foundInequality = false; + for (Iterator iter = theseTypeMungers.iterator(); iter.hasNext() && !foundInequality;) { + Object thisOne = (Object) iter.next(); + boolean foundInOtherSet = false; + for (Iterator iterator = otherTypeMungers.iterator(); iterator.hasNext();) { + Object otherOne = (Object) iterator.next(); + if (thisOne instanceof ConcreteTypeMunger) { + if (((ConcreteTypeMunger) thisOne).shouldOverwrite()) { + shouldOverwriteThis = true; + } + } + if (thisOne instanceof ConcreteTypeMunger && otherOne instanceof ConcreteTypeMunger) { + if (((ConcreteTypeMunger) thisOne).equivalentTo(otherOne)) { + foundInOtherSet = true; + } else if (thisOne.equals(otherOne)) { + foundInOtherSet = true; + } + } else { + if (thisOne.equals(otherOne)) { + foundInOtherSet = true; + } + } + } + if (!foundInOtherSet) + foundInequality = true; + } + if (foundInequality) { + // System.out.println("type munger change"); + changed = true; + } + if (shouldOverwriteThis) { + typeMungers = other.typeMungers; + } } + // if (!theseTypeMungers.equals(otherTypeMungers)) { + // changed = true; + // typeMungers = other.typeMungers; + // } if (!lateTypeMungers.equals(other.lateTypeMungers)) { changed = true; @@ -460,10 +459,8 @@ public class CrosscuttingMembers { PointcutRewriter pr = new PointcutRewriter(); Pointcut p = munger.getPointcut(); Pointcut newP = pr.rewrite(p); - if (p.m_ignoreUnboundBindingForNames.length != 0) {// *sigh* dirty fix - // for dirty hacky - // implementation - // pr149305 + if (p.m_ignoreUnboundBindingForNames.length != 0) { + // *sigh* dirty fix for dirty hacky implementation pr149305 newP.m_ignoreUnboundBindingForNames = p.m_ignoreUnboundBindingForNames; } munger.setPointcut(newP); @@ -482,7 +479,7 @@ public class CrosscuttingMembers { return declareDominates; } - public List getDeclareParents() { + public Collection getDeclareParents() { return declareParents; } @@ -502,18 +499,18 @@ public class CrosscuttingMembers { return lateTypeMungers; } - public List getDeclareAnnotationOnTypes() { + public Collection getDeclareAnnotationOnTypes() { return declareAnnotationsOnType; } - public List getDeclareAnnotationOnFields() { + public Collection getDeclareAnnotationOnFields() { return declareAnnotationsOnField; } /** * includes declare @method and @constructor */ - public List getDeclareAnnotationOnMethods() { + public Collection getDeclareAnnotationOnMethods() { return declareAnnotationsOnMethods; } diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/DeclareParents.java b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/DeclareParents.java index 3d97bcad7..e21401222 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/patterns/DeclareParents.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/patterns/DeclareParents.java @@ -148,6 +148,7 @@ public class DeclareParents extends Declare { public void resolve(IScope scope) { // ScopeWithTypeVariables resolutionScope = new ScopeWithTypeVariables(typeVariablesInScope,scope); child = child.resolveBindings(scope, Bindings.NONE, false, false); + isWildChild = (child instanceof WildTypePattern); parents = parents.resolveBindings(scope, Bindings.NONE, false, true); // Could assert this ... @@ -175,6 +176,8 @@ public class DeclareParents extends Declare { private ResolvedType maybeGetNewParent(ResolvedType targetType, TypePattern typePattern, World world,boolean reportErrors) { if (typePattern == TypePattern.NO) return null; // already had an error here + +// isWildChild = (child instanceof WildTypePattern); UnresolvedType iType = typePattern.getExactType(); ResolvedType parentType = iType.resolve(world);