From 8dc8d1e0b897d9ab515ed3d99abaee930b45c9d5 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Wed, 23 Jan 2013 13:25:49 -0800 Subject: [PATCH] more changes to make if point cut generated names stable --- .../compiler/ast/AdviceDeclaration.java | 4 +- .../internal/compiler/ast/IfPseudoToken.java | 67 +++++++++-------- .../compiler/ast/PointcutDeclaration.java | 2 +- .../internal/compiler/ast/PseudoToken.java | 3 +- .../internal/compiler/ast/PseudoTokens.java | 3 +- .../aspectj/testing/XMLBasedAjcTestCase.java | 9 ++- tests/bugs172/pr398246/Code5.java | 20 ++++++ tests/bugs172/pr398246/Code5a.java | 25 +++++++ tests/bugs172/pr398246/Code6.java | 22 ++++++ tests/bugs172/pr398246/Code7.java | 20 ++++++ .../systemtest/ajc172/Ajc172Tests.java | 71 ++++++++++++++----- .../org/aspectj/systemtest/ajc172/ajc172.xml | 35 +++++++++ 12 files changed, 229 insertions(+), 52 deletions(-) create mode 100644 tests/bugs172/pr398246/Code5.java create mode 100644 tests/bugs172/pr398246/Code5a.java create mode 100644 tests/bugs172/pr398246/Code6.java create mode 100644 tests/bugs172/pr398246/Code7.java diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java index c5320f08a..e9aca3644 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java @@ -61,6 +61,8 @@ public class AdviceDeclaration extends AjMethodDeclaration { public AdviceKind kind; // set during parsing, referenced by Proceed and AsmElementFormatter private int extraArgumentFlags = 0; + + public int adviceSequenceNumberInType; public MethodBinding proceedMethodBinding; // set during this.resolveStaments, referenced by Proceed public List proceedCalls = new ArrayList(2); // populated during Proceed.findEnclosingAround @@ -323,7 +325,7 @@ public class AdviceDeclaration extends AjMethodDeclaration { // override, Called by ClassScope.postParse public void postParse(TypeDeclaration typeDec) { AspectDeclaration aspectDecl = (AspectDeclaration) typeDec; - int adviceSequenceNumberInType = aspectDecl.adviceCounter++; + adviceSequenceNumberInType = aspectDecl.adviceCounter++; StringBuffer stringifiedPointcut = new StringBuffer(30); pointcutDesignator.print(0, stringifiedPointcut); diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/IfPseudoToken.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/IfPseudoToken.java index 821b8b3be..7d7842c6d 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/IfPseudoToken.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/IfPseudoToken.java @@ -34,6 +34,7 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeReference; import org.aspectj.org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants; import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.TypeBinding; import org.aspectj.org.eclipse.jdt.internal.compiler.parser.Parser; +import org.aspectj.weaver.BCException; import org.aspectj.weaver.Member; import org.aspectj.weaver.ResolvedMemberImpl; import org.aspectj.weaver.UnresolvedType; @@ -82,17 +83,17 @@ public class IfPseudoToken extends PseudoToken { /** * enclosingDec is either AdviceDeclaration or PointcutDeclaration */ - public void postParse(TypeDeclaration typeDec, - MethodDeclaration enclosingDec) { + public int postParse(TypeDeclaration typeDec, + MethodDeclaration enclosingDec, int counter) { // typeDec.scope.problemReporter().signalError(sourceStart, sourceEnd, // "if pcd is not implemented in 1.1alpha1"); // XXX need to implement correctly if (pointcut == null) - return; + return 0; - testMethod = makeIfMethod(enclosingDec.compilationResult, enclosingDec, - typeDec); + testMethod = makeIfMethod(enclosingDec.compilationResult, enclosingDec, typeDec, counter); AstUtil.addMethodDeclaration(typeDec, testMethod); + return 1; } private final static char[] CodeGenerationHint = "CodeGenerationHint".toCharArray(); @@ -100,11 +101,9 @@ public class IfPseudoToken extends PseudoToken { private final static char[] IfNameSuffix = "ifNameSuffix".toCharArray(); // XXX todo: make sure that errors in Arguments only get displayed once - private MethodDeclaration makeIfMethod(CompilationResult result, - MethodDeclaration enclosingDec, TypeDeclaration containingTypeDec) { + private MethodDeclaration makeIfMethod(CompilationResult result, MethodDeclaration enclosingDec, TypeDeclaration containingTypeDec, int counter) { MethodDeclaration ret = new IfMethodDeclaration(result, pointcut); - ret.modifiers = ClassFileConstants.AccStatic - | ClassFileConstants.AccFinal | ClassFileConstants.AccPublic; + ret.modifiers = ClassFileConstants.AccStatic | ClassFileConstants.AccFinal | ClassFileConstants.AccPublic; ret.returnType = AstUtil.makeTypeReference(TypeBinding.BOOLEAN); String nameSuffix = null; @@ -144,29 +143,41 @@ public class IfPseudoToken extends PseudoToken { StringBuffer ifSelector = new StringBuffer(); ifSelector.append("ajc$if$"); if (nameSuffix == null || nameSuffix.length()==0) { - ifSelector.append(Integer.toHexString(expr.sourceStart)); + boolean computedName = false; + try { + // possibly even better logic for more reliable name: + if (enclosingDec instanceof AdviceDeclaration) { + // name is ajc$if$[$]$ + ifSelector.append(((AdviceDeclaration)enclosingDec).adviceSequenceNumberInType).append("$"); + if (counter!=0) { + ifSelector.append(counter); + ifSelector.append("$"); + } + ifSelector.append(Integer.toHexString(expr.toString().hashCode())); + computedName = true; + } else if (enclosingDec instanceof PointcutDeclaration) { + if (counter!=0) { + ifSelector.append(counter); + ifSelector.append("$"); + } + StringBuilder toHash = new StringBuilder(((PointcutDeclaration) enclosingDec).getPointcutText()); + toHash.append(expr.toString()); + // name is pointcut selector then $if[$]$ + ifSelector.append(Integer.toHexString(toHash.toString().hashCode())); + computedName = true; + } + } catch (Throwable t) { + throw new IllegalStateException(t); + // let it build a name the 'old way' + } + if (!computedName) { + ifSelector.append(Integer.toHexString(expr.sourceStart)); + } } else { ifSelector.append(nameSuffix); } - // possibly even better logic for more reliable name: - // if (enclosingDec instanceof AdviceDeclaration) { - // // name is - // ajc$if$$ - // ifSelector.append("ajc$if$"); - // ifSelector.append(((AdviceDeclaration) - // enclosingDec).adviceSequenceNumberInType); - // ifSelector.append("$").append(Integer.toHexString(expr.toString().hashCode())); - // } else if (enclosingDec instanceof PointcutDeclaration) { - // // name is pointcut selector then $if$ - // ifSelector.append(((PointcutDeclaration) enclosingDec).selector); - // ifSelector.append("$if$"); - // ifSelector.append(Integer.toHexString(expr.toString().hashCode())); - // } else { - // throw new BCException("Unexpected enclosing declaration of " + - // enclosingDec + " for if pointcut designator"); - // } - // hashcode of expression + ret.selector = ifSelector.toString().toCharArray(); ret.arguments = makeArguments(enclosingDec, containingTypeDec); ret.statements = new Statement[] { new ReturnStatement(expr, diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PointcutDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PointcutDeclaration.java index a95a3e7f9..f10492047 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PointcutDeclaration.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PointcutDeclaration.java @@ -146,7 +146,7 @@ public class PointcutDeclaration extends AjMethodDeclaration { generateSyntheticPointcutMethod = true; } - private String getPointcutText() { + public String getPointcutText() { String text = getPointcut().toString(); if (text.indexOf("BindingTypePattern") == -1) return text; diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoToken.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoToken.java index 0d75f5f98..ac679ad1a 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoToken.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoToken.java @@ -80,8 +80,9 @@ public class PseudoToken extends ASTNode implements IToken { return "unknown"; } - public void postParse(TypeDeclaration typeDec, MethodDeclaration enclosingDec) { + public int postParse(TypeDeclaration typeDec, MethodDeclaration enclosingDec, int tokenNumber) { // nothing to do typically + return 0; } /* (non-Javadoc) diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoTokens.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoTokens.java index c71d2fbb1..4777d0faf 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoTokens.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/PseudoTokens.java @@ -152,8 +152,9 @@ public class PseudoTokens extends ASTNode { public void postParse(TypeDeclaration typeDec, MethodDeclaration enclosingDec) { + int counter = 0; // Counter can be used by postParse as a value to compute uniqueness (if required) for (int i=0, len=tokens.length; i < len; i++) { - tokens[i].postParse(typeDec, enclosingDec); + counter+=tokens[i].postParse(typeDec, enclosingDec, counter); } } diff --git a/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java b/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java index 9ecd5d885..29ada2727 100644 --- a/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java +++ b/testing/newsrc/org/aspectj/testing/XMLBasedAjcTestCase.java @@ -319,12 +319,19 @@ public abstract class XMLBasedAjcTestCase extends AjcTestCase { } protected Method getMethodStartsWith(JavaClass jc, String prefix) { + return getMethodStartsWith(jc,prefix,1); + } + + protected Method getMethodStartsWith(JavaClass jc, String prefix, int whichone) { Method[] meths = jc.getMethods(); for (int i = 0; i < meths.length; i++) { Method method = meths[i]; System.out.println(method); if (method.getName().startsWith(prefix)) { - return method; + whichone--; + if (whichone==0) { + return method; + } } } return null; diff --git a/tests/bugs172/pr398246/Code5.java b/tests/bugs172/pr398246/Code5.java new file mode 100644 index 000000000..50512d496 --- /dev/null +++ b/tests/bugs172/pr398246/Code5.java @@ -0,0 +1,20 @@ +import java.lang.annotation.*; + +public class Code5 { + public static boolean isTrue = true; + + public void m() { + } + public static void main(String []argv) { + new Code5().m(); + } +} + +aspect X { + + pointcut p(): execution(* Code*.*(..)) && if(Code5.isTrue); + + before(): p() { + System.out.println("advice"); + } +} diff --git a/tests/bugs172/pr398246/Code5a.java b/tests/bugs172/pr398246/Code5a.java new file mode 100644 index 000000000..0f5562640 --- /dev/null +++ b/tests/bugs172/pr398246/Code5a.java @@ -0,0 +1,25 @@ +import java.lang.annotation.*; + +public class Code5a { + public static boolean isTrue = true; + + public void m() { + } + public static void main(String []argv) { + new Code5a().m(); + } +} + + +// more white space, on purpose + + + +aspect X2 { + + + pointcut p(): execution(* Code*.*(..)) && if(Code5.isTrue); + before(): p() { + System.out.println("advice"); + } +} diff --git a/tests/bugs172/pr398246/Code6.java b/tests/bugs172/pr398246/Code6.java new file mode 100644 index 000000000..e032eb7c9 --- /dev/null +++ b/tests/bugs172/pr398246/Code6.java @@ -0,0 +1,22 @@ +import java.lang.annotation.*; + +public class Code6 { + public static boolean isTrue = true; + public static boolean isTrue2 = true; + + public void m() { + } + + public static void main(String []argv) { + new Code6().m(); + } +} + +aspect X { + + pointcut p(): execution(* Code*.*(..)) && if(Code6.isTrue) && if(Code6.isTrue2); + + before(): p() { + System.out.println("advice"); + } +} diff --git a/tests/bugs172/pr398246/Code7.java b/tests/bugs172/pr398246/Code7.java new file mode 100644 index 000000000..e8d47a5a1 --- /dev/null +++ b/tests/bugs172/pr398246/Code7.java @@ -0,0 +1,20 @@ +import java.lang.annotation.*; + +public class Code7 { + public static boolean isTrue = true; + public static boolean isTrue2 = true; + + public void m() { + } + + public static void main(String []argv) { + new Code7().m(); + } +} + +aspect X { + + before(): execution(* Code*.*(..)) && if(Code7.isTrue) && if(Code7.isTrue2) { + System.out.println("advice"); + } +} diff --git a/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java b/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java index eea4030d0..fd9454164 100644 --- a/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java +++ b/tests/src/org/aspectj/systemtest/ajc172/Ajc172Tests.java @@ -23,36 +23,69 @@ import org.aspectj.testing.XMLBasedAjcTestCase; */ public class Ajc172Tests extends org.aspectj.testing.XMLBasedAjcTestCase { - public void testIfPointcutNames_pr398246() throws Exception{ + public void testIfPointcutNames_pr398246() throws Exception { runTest("if pointcut names"); - JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"X"); - Method m = getMethodStartsWith(jc,"ajc$if"); - assertEquals("ajc$if$andy",m.getName()); + JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); + Method m = getMethodStartsWith(jc, "ajc$if"); + assertEquals("ajc$if$andy", m.getName()); } - + public void testIfPointcutNames_pr398246_2() throws Exception { runTest("if pointcut names 2"); - JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"X"); - Method m = getMethodStartsWith(jc,"ajc$if"); - assertEquals("ajc$if$fred",m.getName()); + JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); + Method m = getMethodStartsWith(jc, "ajc$if"); + assertEquals("ajc$if$fred", m.getName()); } - + // fully qualified annotation name is used public void testIfPointcutNames_pr398246_3() throws Exception { runTest("if pointcut names 3"); - JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"X"); - Method m = getMethodStartsWith(jc,"ajc$if"); - assertEquals("ajc$if$barney",m.getName()); + JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); + Method m = getMethodStartsWith(jc, "ajc$if"); + assertEquals("ajc$if$barney", m.getName()); } - - // compiling a class later than the initial build - does it pick up the right if clause name? - public void testIfPointcutNames_pr398246_4() throws Exception{ + + // compiling a class later than the initial build - does it pick up the + // right if clause name? + public void testIfPointcutNames_pr398246_4() throws Exception { runTest("if pointcut names 4"); - JavaClass jc = getClassFrom(ajc.getSandboxDirectory(),"X"); - Method m = getMethodStartsWith(jc,"ajc$if"); - assertEquals("ajc$if$sid",m.getName()); + JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); + Method m = getMethodStartsWith(jc, "ajc$if"); + assertEquals("ajc$if$sid", m.getName()); + } + + // new style generated names + public void testIfPointcutNames_pr398246_5() throws Exception { + runTest("if pointcut names 5"); + JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); + Method m = getMethodStartsWith(jc, "ajc$if"); + assertEquals("ajc$if$ac0cb804", m.getName()); + + jc = getClassFrom(ajc.getSandboxDirectory(), "X2"); + m = getMethodStartsWith(jc, "ajc$if"); + assertEquals("ajc$if$ac0cb804", m.getName()); } - + + // new style generated names - multiple ifs in one pointcut + public void testIfPointcutNames_pr398246_6() throws Exception { + runTest("if pointcut names 6"); + JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); + Method m = getMethodStartsWith(jc, "ajc$if",1); + assertEquals("ajc$if$aac93da8", m.getName()); + m = getMethodStartsWith(jc, "ajc$if",2); + assertEquals("ajc$if$1$ae5e778a", m.getName()); + } + + // new style generated names - multiple ifs in one advice + public void testIfPointcutNames_pr398246_7() throws Exception { + runTest("if pointcut names 7"); + JavaClass jc = getClassFrom(ajc.getSandboxDirectory(), "X"); + Method m = getMethodStartsWith(jc, "ajc$if",1); + assertEquals("ajc$if$1$ac0607c", m.getName()); + m = getMethodStartsWith(jc, "ajc$if",2); + assertEquals("ajc$if$1$1$4d4baf36", m.getName()); + } + public void testOptionalAspects_pr398588() { runTest("optional aspects"); } diff --git a/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml b/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml index 7dc6ce11b..606ae311e 100644 --- a/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml +++ b/tests/src/org/aspectj/systemtest/ajc172/ajc172.xml @@ -84,6 +84,41 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + -- 2.39.5