129163: more bits... recording whether crosscutting members has changed 'since the...

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 1524feb31b4e2c86dbb97ff9b6e204b89fbc5e74..b00f606fa28778b22b616bcc5bd72ba325da1565 100644 (file)
--- 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 4ec1144f20523a8d19c1ca5bf323bb55148ef700..528e6b5841dea8a615eef1d271f184c24ff13964 100644 (file)
--- 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 ed2d8b52eb39aeed46f2cb6a232246a50416af27..3b4ef2f8e62b8b6eeca8722e9ec7ffad2c5b27e7 100644 (file)
--- 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 cbaff05eae91c9285641ae0020c0af46bdb2e596..e0e65bdcd8d4d233ea01b3b4d34c8b450c15ff58 100644 (file)
--- 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 a10ad34804b0541f523e0c36889a622c2b3c17d7..23895d54f5cfa55b628c001238cab488176b2dd4 100644 (file)
--- 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();