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

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();