tests and fix for pr115251

10 files changed, 129 insertions, 56 deletions

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java
index f87d388e4..9df8b0382 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseFactory.java
@@ -964,7 +964,15 @@ public class EclipseFactory {
 		// give it the same delegate and link it to the raw type
 		if (binding.isGenericType()) {
 			UnresolvedType complexTx = fromBinding(binding); // fully aware of any generics info
-			ReferenceType complexName = new ReferenceType(complexTx,world);//getWorld().lookupOrCreateName(complexTx);
+			ResolvedType cName = world.resolve(complexTx,true);
+			ReferenceType complexName = null;
+			if (cName != ResolvedType.MISSING) {
+				complexName = (ReferenceType) cName;
+				complexName = (ReferenceType) complexName.getGenericType();
+				if (complexName == null) complexName = new ReferenceType(complexTx,world);
+			} else {
+				complexName = new ReferenceType(complexTx,world);
+			}
 			name.setGenericType(complexName);
 			complexName.setDelegate(t);
 			complexName.setSourceContext(t.getResolvedTypeX().getSourceContext());
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java
index d8df9e7b9..91eff5bd1 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/EclipseSourceType.java
@@ -447,6 +447,10 @@ public class EclipseSourceType extends AbstractReferenceTypeDelegate {
 		// but we don't need this level of detail, and working with real per clauses
 		// at this stage of compilation is not worth the trouble
         if (!isAnnotationStyleAspect()) {
+//        	   if(declaration instanceof AspectDeclaration) {
+//        		   PerClause pc = ((AspectDeclaration)declaration).perClause;
+//        		   if (pc != null) return pc;
+//        	   }
             return new PerSingleton();
         } else {
             // for @Aspect, we do need the real kind though we don't need the real perClause
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java
index 622eb02d9..63ad260af 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/core/builder/AjBuildManager.java
@@ -264,10 +264,11 @@ public class AjBuildManager implements IOutputClassFileNameProvider,IBinarySourc
                     // dealt with on the first call to processDelta - we are going through this loop
                     // again because in compiling something we found something else we needed to
                     // rebuild.  But what case causes this?
-                    if (hereWeGoAgain) 
+                    if (hereWeGoAgain) {
 					  if (buildConfig.isEmacsSymMode() || buildConfig.isGenerateModelMode())
 					    if (AsmManager.attemptIncrementalModelRepairs)
 						  AsmManager.getDefault().processDelta(files,state.addedFiles,state.deletedFiles);
+                    }
                 }
                 if (!files.isEmpty()) {
                    	CompilationAndWeavingContext.leavingPhase(ct);
diff --git a/tests/src/org/aspectj/systemtest/incremental/tools/AjdeInteractionTestbed.java b/tests/src/org/aspectj/systemtest/incremental/tools/AjdeInteractionTestbed.java
index 146793f26..118d54211 100644
--- a/tests/src/org/aspectj/systemtest/incremental/tools/AjdeInteractionTestbed.java
+++ b/tests/src/org/aspectj/systemtest/incremental/tools/AjdeInteractionTestbed.java
@@ -72,6 +72,10 @@ public class AjdeInteractionTestbed extends TestCase {
 	
 	protected void setUp() throws Exception {
 		super.setUp();
+		MyStateListener.reset();
+		MyBuildProgressMonitor.reset();
+		MyTaskListManager.reset();
+		
 		// Create a sandbox in which to work
 		createEmptySandbox();
 	}
diff --git a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
index cf54d05c4..075840131 100644
--- a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
+++ b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
@@ -23,6 +23,7 @@ import org.aspectj.ajdt.internal.core.builder.AjState;
 import org.aspectj.ajdt.internal.core.builder.IncrementalStateManager;
 import org.aspectj.asm.AsmManager;
 import org.aspectj.asm.IProgramElement;
+import org.aspectj.bridge.IMessage;
 import org.aspectj.testing.util.FileUtil;
 
 /**
@@ -48,6 +49,7 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed {
 	
 	protected void setUp() throws Exception {
 		super.setUp();
+		AjdeInteractionTestbed.VERBOSE = VERBOSE;
 	}
 	
 	
@@ -299,28 +301,26 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed {
 		checkWasntFullBuild();
 	}
 	
-	
-	
-//	public void testPr115251() {
-//		AjdeInteractionTestbed.VERBOSE=true;
-//		initialiseProject("PR115251");
-//		build("PR115251");
-//		checkWasFullBuild();
-//		alter("PR115251","inc1");
-//		build("PR115251");
-//		checkWasntFullBuild();
-//	}
+	public void testPr115251() {
+		//AjdeInteractionTestbed.VERBOSE=true;
+		initialiseProject("PR115251");
+		build("PR115251");
+		checkWasFullBuild();
+		alter("PR115251","inc1");
+		build("PR115251");
+		checkWasntFullBuild();
+	}
 
 	
-//	public void testPr111779() {
-//		super.VERBOSE=true;
-//		initialiseProject("PR111779");
-//		build("PR111779");
-//		alter("PR111779","inc1");
-//		build("PR111779");
-//	}
+/*	public void testPr111779() {
+		super.VERBOSE=true;
+		initialiseProject("PR111779");
+		build("PR111779");
+		alter("PR111779","inc1");
+		build("PR111779");
+	}
+*/
 
-	
 //	public void testPr93310_1() {
 //		AjdeInteractionTestbed.VERBOSE = true;
 //		initialiseProject("PR93310_1");
@@ -344,30 +344,29 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed {
 //		build("PR93310_2");
 //		checkWasntFullBuild();
 //	}
-	
 	// Stage1: Compile two files, pack.A and pack.A1 - A1 sets a protected field in A. 
 	// Stage2: make the field private in class A > gives compile error
 	// Stage3: Add a new aspect whilst there is a compile error !
-//	public void testPr113531() {
-//		initialiseProject("PR113531");
-//		build("PR113531");
-//		assertFalse("build should have compiled ok",
-//				MyTaskListManager.hasErrorMessages());
-//		alter("PR113531","inc1");
-//		build("PR113531");
-//		assertEquals("error message should be 'foo cannot be resolved' ",
-//				"foo cannot be resolved",
-//				((IMessage)MyTaskListManager.getErrorMessages().get(0))
-//					.getMessage());
-//		alter("PR113531","inc2");
-//		build("PR113531");
-//		assertTrue("There should be no exceptions handled:\n"+MyErrorHandler.getErrorMessages(),
-//				MyErrorHandler.getErrorMessages().isEmpty());		
-//		assertEquals("error message should be 'foo cannot be resolved' ",
-//				"foo cannot be resolved",
-//				((IMessage)MyTaskListManager.getErrorMessages().get(0))
-//					.getMessage());
-//	}
+	public void testPr113531() {
+		initialiseProject("PR113531");
+		build("PR113531");
+		assertFalse("build should have compiled ok",
+				MyTaskListManager.hasErrorMessages());
+		alter("PR113531","inc1");
+		build("PR113531");
+		assertEquals("error message should be 'foo cannot be resolved' ",
+				"foo cannot be resolved",
+				((IMessage)MyTaskListManager.getErrorMessages().get(0))
+					.getMessage());
+		alter("PR113531","inc2");
+		build("PR113531");
+		assertTrue("There should be no exceptions handled:\n"+MyErrorHandler.getErrorMessages(),
+				MyErrorHandler.getErrorMessages().isEmpty());		
+		assertEquals("error message should be 'foo cannot be resolved' ",
+				"foo cannot be resolved",
+				((IMessage)MyTaskListManager.getErrorMessages().get(0))
+					.getMessage());
+	}
 
 //	public void testPr112736() {
 //		AjdeInteractionTestbed.VERBOSE = true;
@@ -381,6 +380,7 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed {
 //		checkWasntFullBuild();
 //	}
 	
+	
 	// other possible tests:
 	// - memory usage (freemem calls?)
 	// - relationship map
diff --git a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java
index 33319a80a..05c29f63b 100644
--- a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java
+++ b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java
@@ -21,6 +21,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.aspectj.weaver.bcel.UnwovenClassFile;
 import org.aspectj.weaver.patterns.DeclareParents;
 
 /**
@@ -48,26 +49,51 @@ public class CrosscuttingMembersSet {
 		this.world = world;
 	}
 
+
 	/**
 	 * @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) {
+		boolean change = false;
 		CrosscuttingMembers xcut = (CrosscuttingMembers)members.get(aspectType);
 		if (xcut == null) {
 			members.put(aspectType, aspectType.collectCrosscuttingMembers());
 			clearCaches();
-			return true;
+			change = true;
 		} else {
 			if (xcut.replaceWith(aspectType.collectCrosscuttingMembers())) {
 				clearCaches();
-				return true;
+				change = true;
 			} else {
-				return false;
+				change = false;
 			}
 		}
+//		if (aspectType.isAbstract()) {
+//			// we might have sub-aspects that need to re-collect their crosscutting members from us
+//			boolean ancestorChange = addOrReplaceDescendantsOf(aspectType); 
+//			change = change || ancestorChange;
+//		}
+		return change;
 	}
     
+	private boolean addOrReplaceDescendantsOf(ResolvedType aspectType) {
+		Set knownAspects = members.keySet();
+		Set toBeReplaced = new HashSet();
+		for(Iterator it = knownAspects.iterator(); it.hasNext(); ) {
+			ResolvedType candidateAncestor = (ResolvedType)it.next();
+			if ((candidateAncestor != aspectType) && (aspectType.isAssignableFrom(candidateAncestor))) {
+				toBeReplaced.add(candidateAncestor);
+			}
+		}
+		boolean change = false;
+		for (Iterator it = toBeReplaced.iterator(); it.hasNext(); ) {
+			boolean thisChange = addOrReplaceAspect((ResolvedType)it.next());
+			change = change || thisChange;
+		}
+		return change;
+	}
+	
     public void addAdviceLikeDeclares(ResolvedType aspectType) {
         CrosscuttingMembers xcut = (CrosscuttingMembers)members.get(aspectType);
         xcut.addDeclares(aspectType.collectDeclares(true));
diff --git a/weaver/src/org/aspectj/weaver/ReferenceType.java b/weaver/src/org/aspectj/weaver/ReferenceType.java
index a3a22052b..eb2fa46f2 100644
--- a/weaver/src/org/aspectj/weaver/ReferenceType.java
+++ b/weaver/src/org/aspectj/weaver/ReferenceType.java
@@ -15,6 +15,7 @@ package org.aspectj.weaver;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 
 import org.aspectj.bridge.ISourceLocation;
@@ -33,6 +34,14 @@ import org.aspectj.weaver.patterns.PerClause;
 public class ReferenceType extends ResolvedType {
 	
 	/**
+	 * For generic types, this list holds references to all the derived raw
+	 * and parameterized versions. We need this so that if the generic delegate
+	 * is swapped during incremental compilation, the delegate of the derivatives
+	 * is swapped also.
+	 */
+	private List/*ReferenceType*/ derivativeTypes = new ArrayList();
+	
+	/**
 	 * For parameterized types (or the raw type) - this field points to the actual
 	 * reference type from which they are derived.
 	 */
@@ -77,6 +86,7 @@ public class ReferenceType extends ResolvedType {
     	this.genericType = genericReferenceType;
     	this.typeKind = TypeKind.PARAMETERIZED;
     	this.delegate = genericReferenceType.getDelegate();
+    	genericReferenceType.addDependentType(this);
     }
     
     /**
@@ -93,6 +103,11 @@ public class ReferenceType extends ResolvedType {
     	this.genericType = genericReferenceType;
     	this.typeKind = TypeKind.RAW;
     	this.delegate = genericReferenceType.getDelegate();    	
+    	genericReferenceType.addDependentType(this);
+    }
+    
+    private void addDependentType(ReferenceType dependent) {
+    		this.derivativeTypes.add(dependent);
     }
     
     public String getSignatureForAttribute() {
@@ -500,10 +515,18 @@ public class ReferenceType extends ResolvedType {
 	public void setDelegate(ReferenceTypeDelegate delegate) {
 		this.delegate = delegate;
 		
+		for(Iterator it = this.derivativeTypes.iterator(); it.hasNext(); ) {
+			ReferenceType dependent = (ReferenceType) it.next();
+			dependent.setDelegate(delegate);
+		}
+		
 		// If we are raw, we have a generic type - we should ensure it uses the
 		// same delegate
-		if (isRawType() && getGenericType()!=null) {
-			((ReferenceType)getGenericType()).setDelegate(delegate);
+		if (isRawType() && getGenericType()!=null ) {
+			ReferenceType genType = (ReferenceType) getGenericType();
+			if (genType.getDelegate() != delegate) { // avoids circular updates
+				genType.setDelegate(delegate);
+			}
 		}
 		clearParameterizationCaches();
 	}
diff --git a/weaver/src/org/aspectj/weaver/ResolvedPointcutDefinition.java b/weaver/src/org/aspectj/weaver/ResolvedPointcutDefinition.java
index 4335957e1..39d664a40 100644
--- a/weaver/src/org/aspectj/weaver/ResolvedPointcutDefinition.java
+++ b/weaver/src/org/aspectj/weaver/ResolvedPointcutDefinition.java
@@ -115,7 +115,7 @@ public class ResolvedPointcutDefinition extends ResolvedMemberImpl {
 	 * Called when asking a parameterized super-aspect for its pointcuts.
 	 */
 	public ResolvedMemberImpl parameterizedWith(UnresolvedType[] typeParameters, ResolvedType newDeclaringType, boolean isParameterized) {
-		TypeVariable[] typeVariables = getDeclaringType().getTypeVariables();
+		TypeVariable[] typeVariables = getDeclaringType().resolve(newDeclaringType.getWorld()).getTypeVariables();
 		if (isParameterized && (typeVariables.length != typeParameters.length)) {
 			throw new IllegalStateException("Wrong number of type parameters supplied");
 		}
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index 21893b1bb..30d35cff0 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -1071,6 +1071,7 @@ public class BcelWeaver implements IWeaver {
 		        wovenClassNames.add(className);
 		    }
 		}
+		
 		CompilationAndWeavingContext.leavingPhase(aspectToken);
 
 		requestor.weavingClasses();
diff --git a/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java b/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java
index 9934b7237..91bd756a1 100644
--- a/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java
@@ -153,6 +153,11 @@ public class CflowPointcut extends Pointcut {
 	}
 	
 	public Pointcut concretize1(ResolvedType inAspect, ResolvedType declaringType, IntMap bindings) {
+		
+//		if (this.entry.state == Pointcut.SYMBOLIC) {
+//			// too early to concretize, return unchanged
+//			return this;
+//		}
 
 		// Enforce rule about which designators are supported in declare
 		if (isDeclare(bindings.getEnclosingAdvice())) {
@@ -218,16 +223,17 @@ public class CflowPointcut extends Pointcut {
 		  	// Create a counter field in the aspect
 		  	localCflowField = new ResolvedMemberImpl(Member.FIELD,concreteAspect,Modifier.STATIC | Modifier.PUBLIC | Modifier.FINAL,
 		  		NameMangler.cflowCounter(xcut),UnresolvedType.forName(NameMangler.CFLOW_COUNTER_TYPE).getSignature());
+		   
+		  	// Create type munger to add field to the aspect
+		  	concreteAspect.crosscuttingMembers.addTypeMunger(world.makeCflowCounterFieldAdder(localCflowField));
 		  
-		    // Create type munger to add field to the aspect
-		    concreteAspect.crosscuttingMembers.addTypeMunger(world.makeCflowCounterFieldAdder(localCflowField));
-		  
-		    // Create shadow munger to push stuff onto the stack
-		    concreteAspect.crosscuttingMembers.addConcreteShadowMunger(
-		  		Advice.makeCflowEntry(world,concreteEntry,isBelow,localCflowField,freeVars.length,innerCflowEntries,inAspect));
-		    
-		    putCflowfield(concreteEntry,localCflowField); // Remember it
+		  	// Create shadow munger to push stuff onto the stack
+		  	concreteAspect.crosscuttingMembers.addConcreteShadowMunger(
+		    Advice.makeCflowEntry(world,concreteEntry,isBelow,localCflowField,freeVars.length,innerCflowEntries,inAspect));
+	    
+			putCflowfield(concreteEntry,localCflowField); // Remember it
 	      }
+		    
 		  Pointcut ret = new ConcreteCflowPointcut(localCflowField, null,true);
 		  ret.copyLocationFrom(this);
 		  return ret;