From 7808f3c29e34fe462c2f248d00727bbbc25fde2c Mon Sep 17 00:00:00 2001 From: aclement Date: Tue, 7 Mar 2006 11:06:30 +0000 Subject: [PATCH] 129163: more bits... recording whether crosscutting members has changed 'since the last reset' --- .../compiler/lookup/AjLookupEnvironment.java | 6 +-- .../tools/MultiProjectIncrementalTests.java | 10 +--- .../aspectj/weaver/CrosscuttingMembers.java | 50 ++++++++++++++++--- .../weaver/CrosscuttingMembersSet.java | 23 +++++++-- .../org/aspectj/weaver/bcel/BcelWeaver.java | 2 +- 5 files changed, 68 insertions(+), 23 deletions(-) diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java index 1524feb31..b00f606fa 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java @@ -165,7 +165,7 @@ public class AjLookupEnvironment extends LookupEnvironment implements AnonymousC typesToProcess.add(stb); } } - + factory.getWorld().getCrosscuttingMembersSet().reset(); while (typesToProcess.size()>0) { // removes types from the list as they are processed... collectAllITDsAndDeclares((SourceTypeBinding)typesToProcess.get(0),typesToProcess); @@ -391,10 +391,10 @@ public class AjLookupEnvironment extends LookupEnvironment implements AnonymousC if (dec instanceof AspectDeclaration) { ResolvedType typeX = factory.fromEclipse(dec.binding); - factory.getWorld().getCrosscuttingMembersSet().addOrReplaceAspect(typeX); + factory.getWorld().getCrosscuttingMembersSet().addOrReplaceAspect(typeX,false); if (typeX.getSuperclass().isAspect() && !typeX.getSuperclass().isExposedToWeaver()) { - factory.getWorld().getCrosscuttingMembersSet().addOrReplaceAspect(typeX.getSuperclass()); + factory.getWorld().getCrosscuttingMembersSet().addOrReplaceAspect(typeX.getSuperclass(),false); } } diff --git a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java index 4ec1144f2..528e6b584 100644 --- a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java +++ b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java @@ -560,10 +560,7 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed { build("PR113257"); alter("PR113257","inc1"); build("PR113257"); - // THIS should be a full build, however, so that the patches - // for bug 129163 can be applied incrementally have changed this... - //checkWasFullBuild(); // back to the source - checkWasntFullBuild(); + checkWasFullBuild(); // back to the source alter("PR113257","inc1"); build("PR113257"); } @@ -573,10 +570,7 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed { build("PR123612"); alter("PR123612","inc1"); build("PR123612"); - // THIS should be a full build, however, so that the patches - // for bug 129163 can be applied incrementally have changed this... - //checkWasFullBuild(); // back to the source - checkWasntFullBuild(); + checkWasFullBuild(); // back to the source } diff --git a/weaver/src/org/aspectj/weaver/CrosscuttingMembers.java b/weaver/src/org/aspectj/weaver/CrosscuttingMembers.java index ed2d8b52e..3b4ef2f8e 100644 --- a/weaver/src/org/aspectj/weaver/CrosscuttingMembers.java +++ b/weaver/src/org/aspectj/weaver/CrosscuttingMembers.java @@ -15,8 +15,10 @@ package org.aspectj.weaver; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Set; import org.aspectj.weaver.patterns.Declare; import org.aspectj.weaver.patterns.DeclareAnnotation; @@ -26,6 +28,7 @@ import org.aspectj.weaver.patterns.DeclarePrecedence; import org.aspectj.weaver.patterns.DeclareSoft; import org.aspectj.weaver.patterns.PerClause; import org.aspectj.weaver.patterns.Pointcut; +import org.aspectj.weaver.patterns.PointcutRewriter; /** @@ -192,7 +195,22 @@ public class CrosscuttingMembers { return ret; } - public boolean replaceWith(CrosscuttingMembers other) { + /** + * 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 + */ + public boolean replaceWith(CrosscuttingMembers other,boolean careAboutShadowMungers) { boolean changed = false; if (perClause == null || !perClause.equals(other.perClause)) { @@ -202,12 +220,32 @@ public class CrosscuttingMembers { //XXX all of the below should be set equality rather than list equality //System.err.println("old: " + shadowMungers + " new: " + other.shadowMungers); - if (!shadowMungers.equals(other.shadowMungers)) { - changed = true; - shadowMungers = other.shadowMungers; - } - if (!typeMungers.equals(other.typeMungers)) { + if (careAboutShadowMungers) { + // bug 129163: use set equality rather than list equality + Set theseShadowMungers = new HashSet(); + theseShadowMungers.addAll(shadowMungers); + Set otherShadowMungers = new HashSet(); + otherShadowMungers.addAll(other.shadowMungers); + + PointcutRewriter pr = new PointcutRewriter(); + for (Iterator iter = otherShadowMungers.iterator(); iter.hasNext();) { + ShadowMunger munger = (ShadowMunger) iter.next(); + Pointcut p = munger.getPointcut(); + Pointcut newP = pr.rewrite(p); + munger.setPointcut(newP); + } + if (!theseShadowMungers.equals(otherShadowMungers)) { + changed = true; + shadowMungers = other.shadowMungers; + } + } + // bug 129163: use set equality rather than list equality + Set theseTypeMungers = new HashSet(); + theseTypeMungers.addAll(typeMungers); + Set otherTypeMungers = new HashSet(); + otherTypeMungers.addAll(other.typeMungers); + if (!theseTypeMungers.equals(otherTypeMungers)) { changed = true; typeMungers = other.typeMungers; } diff --git a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java index cbaff05ea..e0e65bdcd 100644 --- a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java +++ b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java @@ -44,17 +44,21 @@ public class CrosscuttingMembersSet { private List declareAnnotationOnFields = null; private List declareAnnotationOnMethods= null; // includes ctors private List declareDominates = null; + private boolean changedSinceLastReset = false; public CrosscuttingMembersSet(World world) { this.world = world; } + public boolean addOrReplaceAspect(ResolvedType aspectType) { + return addOrReplaceAspect(aspectType,true); + } /** * @return whether or not that was a change to the global signature * XXX for efficiency we will need a richer representation than this */ - public boolean addOrReplaceAspect(ResolvedType aspectType) { + public boolean addOrReplaceAspect(ResolvedType aspectType,boolean careAboutShadowMungers) { boolean change = false; CrosscuttingMembers xcut = (CrosscuttingMembers)members.get(aspectType); if (xcut == null) { @@ -63,7 +67,7 @@ public class CrosscuttingMembersSet { CflowPointcut.clearCaches(aspectType); change = true; } else { - if (xcut.replaceWith(aspectType.collectCrosscuttingMembers())) { + if (xcut.replaceWith(aspectType.collectCrosscuttingMembers(),careAboutShadowMungers)) { clearCaches(); CflowPointcut.clearCaches(aspectType); @@ -74,13 +78,14 @@ public class CrosscuttingMembersSet { } if (aspectType.isAbstract()) { // we might have sub-aspects that need to re-collect their crosscutting members from us - boolean ancestorChange = addOrReplaceDescendantsOf(aspectType); + boolean ancestorChange = addOrReplaceDescendantsOf(aspectType,careAboutShadowMungers); change = change || ancestorChange; } + changedSinceLastReset = changedSinceLastReset || change; return change; } - private boolean addOrReplaceDescendantsOf(ResolvedType aspectType) { + private boolean addOrReplaceDescendantsOf(ResolvedType aspectType,boolean careAboutShadowMungers) { //System.err.println("Looking at descendants of "+aspectType.getName()); Set knownAspects = members.keySet(); Set toBeReplaced = new HashSet(); @@ -93,7 +98,7 @@ public class CrosscuttingMembersSet { boolean change = false; for (Iterator it = toBeReplaced.iterator(); it.hasNext(); ) { ResolvedType next = (ResolvedType)it.next(); - boolean thisChange = addOrReplaceAspect(next); + boolean thisChange = addOrReplaceAspect(next,careAboutShadowMungers); change = change || thisChange; } return change; @@ -255,5 +260,13 @@ public class CrosscuttingMembersSet { return null; } + public void reset() { + changedSinceLastReset = false; + } + + public boolean hasChangedSinceLastReset() { + return changedSinceLastReset; + } + } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java index a10ad3480..23895d54f 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java @@ -430,7 +430,7 @@ public class BcelWeaver implements IWeaver { // ---- weave preparation public void prepareForWeave() { - needToReweaveWorld = false; + needToReweaveWorld = xcutSet.hasChangedSinceLastReset(); CflowPointcut.clearCaches(); -- 2.39.5