From a9abf53e92cc92a49c0c3924ad076c4f37596c0d Mon Sep 17 00:00:00 2001 From: aclement Date: Mon, 12 Dec 2005 13:45:46 +0000 Subject: [PATCH] fixes for 119539 --- .../compiler/lookup/EclipseSourceType.java | 8 +-- .../systemtest/ajc150/Ajc150Tests.java | 6 +- .../tools/MultiProjectIncrementalTests.java | 2 +- .../weaver/CrosscuttingMembersSet.java | 42 +++++++----- .../src/org/aspectj/weaver/ReferenceType.java | 9 ++- .../weaver/patterns/CflowPointcut.java | 67 +++++++++++++++---- 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); + } + } } -- 2.39.5