fixes for 119539

6 files changed, 93 insertions, 41 deletions

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 2785778e5..81491116d 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
@@ -451,10 +451,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;
-//        	   }
+        	   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/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
index de9881e39..faf86ad29 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java
@@ -42,9 +42,9 @@ public class Ajc150Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
     return new File("../tests/src/org/aspectj/systemtest/ajc150/ajc150.xml");
   }
 
-  //public void testGenericPTW_pr119539_1() { runTest("generic pertypewithin aspect - 1");}
-  //public void testGenericPTW_pr119539_2() { runTest("generic pertypewithin aspect - 2");}
-  //public void testGenericPTW_pr119539_3() { runTest("generic pertypewithin aspect - 3");}
+  public void testGenericPTW_pr119539_1() { runTest("generic pertypewithin aspect - 1");}
+  public void testGenericPTW_pr119539_2() { runTest("generic pertypewithin aspect - 2");}
+  public void testGenericPTW_pr119539_3() { runTest("generic pertypewithin aspect - 3");}
   /*
   public void testBrokenDispatchByITD_pr72834() { runTest("broken dispatch");}
   public void testMissingAccessor_pr73856() { runTest("missing accessor");}
diff --git a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
index b8e1c69bf..d7ddaceb7 100644
--- a/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
+++ b/tests/src/org/aspectj/systemtest/incremental/tools/MultiProjectIncrementalTests.java
@@ -349,7 +349,7 @@ public class MultiProjectIncrementalTests extends AjdeInteractionTestbed {
 		alter("PR117882_2","inc1");
 		build("PR117882_2");
 		checkWasntFullBuild();
-//		checkCompileWeaveCount(1,4);
+		checkCompileWeaveCount(1,4);
 		//fullBuild("PR117882_2");
 		//checkWasFullBuild();
 //		AjdeInteractionTestbed.VERBOSE=false;
diff --git a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java
index 668071518..5f9eaf1f0 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.patterns.CflowPointcut;
 import org.aspectj.weaver.patterns.DeclareParents;
 
 /**
@@ -32,17 +33,17 @@ import org.aspectj.weaver.patterns.DeclareParents;
 public class CrosscuttingMembersSet {
 	private World world;
 	//FIXME AV - ? we may need a sequencedHashMap there to ensure source based precedence for @AJ advice
-    private Map members = new HashMap();
-	
-	private List shadowMungers = null;
-	private List typeMungers = null;
-    private List lateTypeMungers = null;
-	private List declareSofts = null;
-	private List declareParents = null;
-	private List declareAnnotationOnTypes   = null;
+    private Map /* ResolvedType (the aspect) > CrosscuttingMembers */members = new HashMap();
+	
+	private List shadowMungers             = null;
+	private List typeMungers               = null;
+    private List lateTypeMungers           = null;
+	private List declareSofts              = null;
+	private List declareParents            = null;
+	private List declareAnnotationOnTypes  = null;
 	private List declareAnnotationOnFields = null; 
 	private List declareAnnotationOnMethods= null; // includes ctors
-	private List declareDominates = null;
+	private List declareDominates          = null;
 	
 	public CrosscuttingMembersSet(World world) {
 		this.world = world;
@@ -59,35 +60,40 @@ public class CrosscuttingMembersSet {
 		if (xcut == null) {
 			members.put(aspectType, aspectType.collectCrosscuttingMembers());
 			clearCaches();
+			CflowPointcut.clearCaches(aspectType);
 			change = true;
 		} else {
 			if (xcut.replaceWith(aspectType.collectCrosscuttingMembers())) {
 				clearCaches();
+
+				CflowPointcut.clearCaches(aspectType);
 				change = true;
 			} else {
 				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;
-//		}
+		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) {
+		//System.err.println("Looking at descendants of "+aspectType.getName());
 		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);
+			ResolvedType candidateDescendant = (ResolvedType)it.next();
+			if ((candidateDescendant != aspectType) && (aspectType.isAssignableFrom(candidateDescendant))) {
+				toBeReplaced.add(candidateDescendant);
 			}
 		}
 		boolean change = false;
 		for (Iterator it = toBeReplaced.iterator(); it.hasNext(); ) {
-			boolean thisChange = addOrReplaceAspect((ResolvedType)it.next());
+			ResolvedType next = (ResolvedType)it.next();
+			boolean thisChange = addOrReplaceAspect(next);
 			change = change || thisChange;
 		}
 		return change;
diff --git a/weaver/src/org/aspectj/weaver/ReferenceType.java b/weaver/src/org/aspectj/weaver/ReferenceType.java
index 7cc34533a..d16360eea 100644
--- a/weaver/src/org/aspectj/weaver/ReferenceType.java
+++ b/weaver/src/org/aspectj/weaver/ReferenceType.java
@@ -469,7 +469,14 @@ public class ReferenceType extends ResolvedType {
 		return this.typeVariables;
 	}
 	
-	public PerClause getPerClause() { return delegate.getPerClause(); }
+	public PerClause getPerClause() { 
+		PerClause pclause = delegate.getPerClause();
+		if (isParameterizedType()) { // could cache the result here...
+			Map parameterizationMap = getAjMemberParameterizationMap();
+			pclause = (PerClause)pclause.parameterizeWith(parameterizationMap);
+		}
+		return pclause;
+	}
 	
 	
 	protected Collection getDeclares() {
diff --git a/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java b/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java
index 91bd756a1..4eea612fd 100644
--- a/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java
+++ b/weaver/src/org/aspectj/weaver/patterns/CflowPointcut.java
@@ -18,7 +18,9 @@ import java.io.IOException;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Enumeration;
 import java.util.Hashtable;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -149,11 +151,13 @@ public class CflowPointcut extends Pointcut {
 	}
 
 	protected Test findResidueInternal(Shadow shadow, ExposedState state) {
-		throw new RuntimeException("unimplemented");
+		throw new RuntimeException("unimplemented - did concretization fail?");
 	}
 	
 	public Pointcut concretize1(ResolvedType inAspect, ResolvedType declaringType, IntMap bindings) {
 		
+		// the pointcut is marked as CONCRETE after returning from this 
+		// call - so we can't skip concretization
 //		if (this.entry.state == Pointcut.SYMBOLIC) {
 //			// too early to concretize, return unchanged
 //			return this;
@@ -169,10 +173,12 @@ public class CflowPointcut extends Pointcut {
 		
 		//make this remap from formal positions to arrayIndices
 		IntMap entryBindings = new IntMap();
-		for (int i=0, len=freeVars.length; i < len; i++) {
+		if (freeVars!=null) {
+		  for (int i=0, len=freeVars.length; i < len; i++) {
 			int freeVar = freeVars[i];
 			//int formalIndex = bindings.get(freeVar);
 			entryBindings.put(freeVar, i);
+		  }
 		}
 		entryBindings.copyContext(bindings);
 		//System.out.println(this + " bindings: " + entryBindings);
@@ -198,8 +204,7 @@ public class CflowPointcut extends Pointcut {
 		List innerCflowEntries = new ArrayList(xcut.getCflowEntries());
 		innerCflowEntries.removeAll(previousCflowEntries);
 
-		  
-		Object field = getCflowfield(concreteEntry);
+		Object field = getCflowfield(concreteEntry,concreteAspect);
 		
 		// Four routes of interest through this code (did I hear someone say refactor??)
 		// 1) no state in the cflow - we can use a counter *and* we have seen this pointcut
@@ -211,7 +216,7 @@ public class CflowPointcut extends Pointcut {
 		// 4) state in the cflow - we need to use a stack, but this is the first time 
 		//    we have seen this pointcut, so build the infrastructure.
 		
-		if (freeVars.length == 0) { // No state, so don't use a stack, use a counter.
+		if (freeVars==null || freeVars.length == 0) { // No state, so don't use a stack, use a counter.
 		  ResolvedMember localCflowField = null;
 
 		  // Check if we have already got a counter for this cflow pointcut
@@ -229,9 +234,9 @@ public class CflowPointcut extends Pointcut {
 		  
 		  	// Create shadow munger to push stuff onto the stack
 		  	concreteAspect.crosscuttingMembers.addConcreteShadowMunger(
-		    Advice.makeCflowEntry(world,concreteEntry,isBelow,localCflowField,freeVars.length,innerCflowEntries,inAspect));
+		    Advice.makeCflowEntry(world,concreteEntry,isBelow,localCflowField,freeVars==null?0:freeVars.length,innerCflowEntries,inAspect));
 	    
-			putCflowfield(concreteEntry,localCflowField); // Remember it
+			putCflowfield(concreteEntry,concreteAspect,localCflowField); // Remember it
 	      }
 		    
 		  Pointcut ret = new ConcreteCflowPointcut(localCflowField, null,true);
@@ -289,7 +294,7 @@ public class CflowPointcut extends Pointcut {
 			
 			  concreteAspect.crosscuttingMembers.addTypeMunger(
 				world.makeCflowStackFieldAdder(localCflowField));
-			  putCflowfield(concreteEntry,localCflowField);
+			  putCflowfield(concreteEntry,concreteAspect,localCflowField);
 		    }
 			Pointcut ret = new ConcreteCflowPointcut(localCflowField, slots,false);
 			ret.copyLocationFrom(this);
@@ -303,23 +308,57 @@ public class CflowPointcut extends Pointcut {
 		cflowBelowFields.clear();
 	}
 	
-	private Object getCflowfield(Pointcut pcutkey) {
+	private String getKey(Pointcut p,ResolvedType a) {
+	  StringBuffer sb = new StringBuffer();
+	  sb.append(a.getName());
+	  sb.append("::");
+	  sb.append(p.toString());
+	  return sb.toString();
+	}
+	
+	private Object getCflowfield(Pointcut pcutkey, ResolvedType concreteAspect) {
+		String key = getKey(pcutkey,concreteAspect);
+		Object o =null;
 		if (isBelow) {
-			return cflowBelowFields.get(pcutkey);
+			o = cflowBelowFields.get(key);
 		} else {
-			return cflowFields.get(pcutkey);
+			o = cflowFields.get(key);
 		}
+		//System.err.println("Retrieving for key "+key+" returning "+o);
+		return o;
 	}
 	
-	private void putCflowfield(Pointcut pcutkey,Object o) {
+	private void putCflowfield(Pointcut pcutkey,ResolvedType concreteAspect,Object o) {
+		String key = getKey(pcutkey,concreteAspect);
+		//System.err.println("Storing cflow field for key"+key);
 		if (isBelow) {
-			cflowBelowFields.put(pcutkey,o);
+			cflowBelowFields.put(key,o);
 		} else {
-			cflowFields.put(pcutkey,o);
+			cflowFields.put(key,o);
 		}
 	}
 
     public Object accept(PatternNodeVisitor visitor, Object data) {
         return visitor.visit(this, data);
 	}
+
+	public static void clearCaches(ResolvedType aspectType) {
+		//System.err.println("Wiping entries starting "+aspectType.getName());
+		String key = aspectType.getName()+"::";
+		wipeKeys(key,cflowFields);
+		wipeKeys(key,cflowBelowFields);
+	}
+	
+	private static void wipeKeys(String keyPrefix,Hashtable ht) {
+		Enumeration keys = ht.keys();
+		List forRemoval = new ArrayList();
+		while (keys.hasMoreElements()) {
+			String s = (String)keys.nextElement();
+			if (s.startsWith(keyPrefix)) forRemoval.add(s);
+		}
+		for (Iterator iter = forRemoval.iterator(); iter.hasNext();) {
+			String element = (String) iter.next();
+			ht.remove(element);
+		}
+	}
 }