diff options
24 files changed, 364 insertions, 125 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 2b9c61fbe..88b552b59 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 @@ -1,13 +1,14 @@ /* ******************************************************************* * Copyright (c) 2002 Palo Alto Research Center, Incorporated (PARC). - * All rights reserved. - * This program and the accompanying materials are made available - * under the terms of the Common Public License v1.0 - * which accompanies this distribution and is available at - * http://www.eclipse.org/legal/cpl-v10.html - * - * Contributors: - * PARC initial implementation + * All rights reserved. + * This program and the accompanying materials are made available + * under the terms of the Common Public License v1.0 + * which accompanies this distribution and is available at + * http://www.eclipse.org/legal/cpl-v10.html + * + * Contributors: + * PARC initial implementation + * Alexandre Vasseur support for @AJ perClause * ******************************************************************/ @@ -22,10 +23,13 @@ import org.aspectj.bridge.IMessage; import org.aspectj.weaver.*; import org.aspectj.weaver.patterns.PerClause; import org.aspectj.weaver.patterns.PerSingleton; +import org.aspectj.weaver.patterns.PerFromSuper; import org.aspectj.org.eclipse.jdt.core.compiler.CharOperation; import org.aspectj.org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration; import org.aspectj.org.eclipse.jdt.internal.compiler.ast.Annotation; import org.aspectj.org.eclipse.jdt.internal.compiler.ast.TypeDeclaration; +import org.aspectj.org.eclipse.jdt.internal.compiler.ast.SingleMemberAnnotation; +import org.aspectj.org.eclipse.jdt.internal.compiler.ast.StringLiteral; import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.*; /** @@ -35,6 +39,7 @@ import org.aspectj.org.eclipse.jdt.internal.compiler.lookup.*; */ public class EclipseSourceType extends ResolvedTypeX.ConcreteName { private static final char[] pointcutSig = "Lorg/aspectj/lang/annotation/Pointcut;".toCharArray(); + private static final char[] aspectSig = "Lorg/aspectj/lang/annotation/Aspect;".toCharArray(); protected ResolvedPointcutDefinition[] declaredPointcuts = null; protected ResolvedMember[] declaredMethods = null; protected ResolvedMember[] declaredFields = null; @@ -68,18 +73,19 @@ public class EclipseSourceType extends ResolvedTypeX.ConcreteName { public boolean isAspect() { - return declaration instanceof AspectDeclaration; + final boolean isCodeStyle = declaration instanceof AspectDeclaration; + return isCodeStyle?isCodeStyle:isAnnotationStyleAspect(); } - // FIXME ATAJ isAnnotationStyleAspect() needs implementing? public boolean isAnnotationStyleAspect() { if (declaration.annotations == null) { return false; } - for (int i = 0; i < declaration.annotations.length; i++) { - Annotation annotation = declaration.annotations[i]; - // do something there - ; + ResolvedTypeX[] annotations = getAnnotationTypes(); + for (int i = 0; i < annotations.length; i++) { + if ("org.aspectj.lang.annotation.Aspect".equals(annotations[i].getName())) { + return true; + } } return false; } @@ -338,9 +344,97 @@ public class EclipseSourceType extends ResolvedTypeX.ConcreteName { //should probably be: ((AspectDeclaration)declaration).perClause; // 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 - return new PerSingleton(); + if (!isAnnotationStyleAspect()) { + return new PerSingleton(); + } else { + // for @Aspect, we do need the real kind though we don't need the real perClause + PerClause.Kind kind = getPerClauseForTypeDeclaration(declaration); + //returning a perFromSuper is enough to get the correct kind.. (that's really a hack - AV) + return new PerFromSuper(kind); + } } - + + PerClause.Kind getPerClauseForTypeDeclaration(TypeDeclaration typeDeclaration) { + Annotation[] annotations = typeDeclaration.annotations; + for (int i = 0; i < annotations.length; i++) { + Annotation annotation = annotations[i]; + if (CharOperation.equals(aspectSig, annotation.resolvedType.signature())) { + // found @Aspect(...) + if (annotation.memberValuePairs() == null || annotation.memberValuePairs().length == 0) { + // it is an @Aspect or @Aspect() + // needs to use PerFromSuper if declaration extends a super aspect + PerClause.Kind kind = lookupPerClauseKind(typeDeclaration.binding.superclass); + // if no super aspect, we have a @Aspect() means singleton + if (kind == null) { + return PerClause.SINGLETON; + } else { + return kind; + } + } else if (annotation instanceof SingleMemberAnnotation) { + // it is an @Aspect(...something...) + SingleMemberAnnotation theAnnotation = (SingleMemberAnnotation)annotation; + String clause = new String(((StringLiteral)theAnnotation.memberValue).source());//TODO cast safe ? + if (clause.startsWith("perthis(")) { + return PerClause.PEROBJECT; + } else if (clause.startsWith("pertarget(")) { + return PerClause.PEROBJECT; + } else if (clause.startsWith("percflow(")) { + return PerClause.PERCFLOW; + } else if (clause.startsWith("percflowbelow(")) { + return PerClause.PERCFLOW; + } else if (clause.startsWith("pertypewithin(")) { + return PerClause.PERTYPEWITHIN; + } else if (clause.startsWith("issingleton(")) { + return PerClause.SINGLETON; + } else { + eclipseWorld().showMessage(IMessage.ABORT, + "cannot determine perClause '" + clause + "'", + new EclipseSourceLocation(typeDeclaration.compilationResult, typeDeclaration.sourceStart, typeDeclaration.sourceEnd), null); + return PerClause.SINGLETON;//fallback strategy just to avoid NPE + } + } else { + eclipseWorld().showMessage(IMessage.ABORT, + "@Aspect annotation is expected to be SingleMemberAnnotation with 'String value()' as unique element", + new EclipseSourceLocation(typeDeclaration.compilationResult, typeDeclaration.sourceStart, typeDeclaration.sourceEnd), null); + return PerClause.SINGLETON;//fallback strategy just to avoid NPE + } + } + } + return null;//no @Aspect annotation at all (not as aspect) + } + + // adapted from AspectDeclaration + private PerClause.Kind lookupPerClauseKind(ReferenceBinding binding) { + final PerClause.Kind kind; + if (binding instanceof BinaryTypeBinding) { + ResolvedTypeX superTypeX = factory.fromEclipse(binding); + PerClause perClause = superTypeX.getPerClause(); + // clause is null for non aspect classes since coming from BCEL attributes + if (perClause != null) { + kind = superTypeX.getPerClause().getKind(); + } else { + kind = null; + } + } else if (binding instanceof SourceTypeBinding ) { + SourceTypeBinding sourceSc = (SourceTypeBinding)binding; + if (sourceSc.scope.referenceContext instanceof AspectDeclaration) { + //code style + kind = ((AspectDeclaration)sourceSc.scope.referenceContext).perClause.getKind(); + } else if (sourceSc.scope.referenceContext instanceof TypeDeclaration) { + // if @Aspect: perFromSuper, else if @Aspect(..) get from anno value, else null + kind = getPerClauseForTypeDeclaration((TypeDeclaration)(sourceSc.scope.referenceContext)); + } else { + //XXX should not happen + kind = null; + } + } else { + //XXX need to handle this too + kind = null; + } + return kind; + } + + protected Collection getDeclares() { return declares; } diff --git a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java index 9e5f8cc2d..8f4d0dfb5 100644 --- a/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java +++ b/org.aspectj.ajdt.core/testsrc/org/aspectj/tools/ajc/Ajc.java @@ -46,11 +46,11 @@ public class Ajc { private static final String TESTER_PATH = ".."+File.separator+"testing-client"+File.separator+"bin" + File.pathSeparator + ".."+File.separator+"runtime"+File.separator+"bin" + File.pathSeparator + - //FIXME AV - can someone tell why those jar needs to be there ?? + //TODO AV - done, remove comments: can someone tell why those jar needs to be there ?? // 1/ see the line before: bin/ will take precedence.. // 2/ see below - aspectj5rt is added last which makes it UNconsistent with the way runtime is handled here.. - ".."+File.separator+"lib"+File.separator+"test"+File.separator+"aspectjrt.jar"+ File.pathSeparator+ - ".."+File.separator+"lib"+File.separator+"test"+File.separator+"testing-client.jar" + File.pathSeparator + + //".."+File.separator+"lib"+File.separator+"test"+File.separator+"aspectjrt.jar"+ File.pathSeparator+ + //".."+File.separator+"lib"+File.separator+"test"+File.separator+"testing-client.jar" + File.pathSeparator + ".."+File.separator+"aspectj5rt"+File.separator+"bin" + File.pathSeparator + File.pathSeparator+ ".."+File.separator+"lib"+File.separator+"junit"+File.separator+"junit.jar" + File.pathSeparator+ ".."+File.separator+"bridge"+File.separator+"bin" diff --git a/run-all-junit-tests/testsrc/AllTests.java b/run-all-junit-tests/testsrc/AllTests.java index 95c03d7e1..af06b53f8 100644 --- a/run-all-junit-tests/testsrc/AllTests.java +++ b/run-all-junit-tests/testsrc/AllTests.java @@ -33,7 +33,8 @@ public class AllTests extends TestCase { suite.addTest(BcweaverModuleTests.suite()); if (LangUtil.is15VMOrGreater()) { suite.addTest(Aspectj5rtModuleTests.suite()); - suite.addTest(Loadtime5ModuleTests.suite()); + suite.addTest(LoadtimeModuleTests.suite());//FIXME AV - should be run on 1.3 when xml deps si fixed in the build + suite.addTest(Loadtime515ModuleTests.suite()); } else { System.err.println("Warning: not running 1.5 tests"); } diff --git a/tests/java5/ataspectj/ataspectj/AllLTWTests.java b/tests/java5/ataspectj/ataspectj/AllLTWTests.java index 7ef68618e..da5856674 100644 --- a/tests/java5/ataspectj/ataspectj/AllLTWTests.java +++ b/tests/java5/ataspectj/ataspectj/AllLTWTests.java @@ -35,6 +35,7 @@ public class AllLTWTests extends TestCase { suite.addTestSuite(ataspectj.PerClauseTest.class); suite.addTestSuite(AroundInlineMungerTest.class); suite.addTestSuite(SingletonInheritanceTest.class); + suite.addTestSuite(PerClauseInheritanceTest.class); return suite; } diff --git a/tests/java5/ataspectj/ataspectj/PerClauseInheritanceTest.java b/tests/java5/ataspectj/ataspectj/PerClauseInheritanceTest.java new file mode 100644 index 000000000..7438dc409 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/PerClauseInheritanceTest.java @@ -0,0 +1,79 @@ +/******************************************************************************* + * Copyright (c) 2005 Contributors. + * All rights reserved. + * This program and the accompanying materials are made available + * under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution and is available at + * http://eclipse.org/legal/epl-v10.html + * + * Contributors: + * Alexandre Vasseur initial implementation + *******************************************************************************/ +package ataspectj; + +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Pointcut; +import org.aspectj.lang.annotation.Before; +import junit.framework.TestCase; + +/** + * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> + */ +public class PerClauseInheritanceTest extends TestCase { + + static StringBuffer s_log = new StringBuffer(); + + static void log(String s) { + s_log.append(s).append(" "); + } + + public static void main(String[] args) { + TestHelper.runAndThrowOnFailure(suite()); + } + + public static junit.framework.Test suite() { + return new junit.framework.TestSuite(PerClauseInheritanceTest.class); + } + + public void hello() { + log("hello"); + } + + public void testInheritance() { + s_log = new StringBuffer(); + hello(); + assertEquals("aop hello ", s_log.toString()); + + s_log = new StringBuffer(); + PerClauseInheritanceTest t = new PerClauseInheritanceTest(); + t.hello(); + assertEquals("aop hello ", s_log.toString()); + + assertEquals(2, ChildAspect.COUNT); + } + + @Aspect("perthis(execution(* ataspectj.PerClauseInheritanceTest.hello()))") + static abstract class ParentAspect { + @Pointcut("execution(* ataspectj.PerClauseInheritanceTest.hello())") + void pc() {} + } + + @Aspect//perFromSuper + static abstract class MidParentAspect extends ParentAspect {} + + @Aspect//perFromSuper + static class ChildAspect extends MidParentAspect { + static int COUNT = 0; + + public ChildAspect() { + COUNT++; + } + + @Before("pc()") + public void abefore() { + log("aop"); + } + } + + +} diff --git a/tests/java5/ataspectj/ataspectj/misuse/Test100.java b/tests/java5/ataspectj/ataspectj/misuse/Test100.java new file mode 100644 index 000000000..0b91d3bf8 --- /dev/null +++ b/tests/java5/ataspectj/ataspectj/misuse/Test100.java @@ -0,0 +1,35 @@ +/******************************************************************************* + * Copyright (c) 2005 Contributors. + * All rights reserved. + * This program and the accompanying materials are made available + * under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution and is available at + * http://eclipse.org/legal/epl-v10.html + * + * Contributors: + * Alexandre Vasseur initial implementation + *******************************************************************************/ +package ataspectj.misuse; + +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Before; +import org.aspectj.lang.ProceedingJoinPoint; + +/** + * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> + */ +public abstract class Test100 { + + void target() { + ; + } + + @Aspect + public static class Test100B { + + @Before("execution(* ataspectj.misuse.Test100.target())") + public void beforeA(ProceedingJoinPoint pjp) { + ;// invalid before advice since use Pjp + } + } +} diff --git a/tests/java5/ataspectj/ataspectj/pathentry/META-INF/aop.xml b/tests/java5/ataspectj/ataspectj/pathentry/META-INF/aop.xml index e2fa88d1b..7dcbabedf 100644 --- a/tests/java5/ataspectj/ataspectj/pathentry/META-INF/aop.xml +++ b/tests/java5/ataspectj/ataspectj/pathentry/META-INF/aop.xml @@ -24,5 +24,6 @@ <aspect name="ataspectj.AroundInlineMungerTestAspects.Open"/> <aspect name="ataspectj.SingletonInheritanceTest.ChildAspect"/> + <aspect name="ataspectj.PerClauseInheritanceTest.ChildAspect"/> </aspects> </aspectj> diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjMisuseTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjMisuseTests.java index 1d661cd36..df29b123e 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjMisuseTests.java +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjMisuseTests.java @@ -61,4 +61,8 @@ public class AtAjMisuseTests extends XMLBasedAjcTestCase { public void testQBeforeOnMethodNotReturningVoid() { runTest("@Before on method not returning void"); } + + public void testQBeforeWithPJP() { + runTest("@Before with PJP"); + } } diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java index 27803227a..53053a24a 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java @@ -11,26 +11,25 @@ *******************************************************************************/ package org.aspectj.systemtest.ajc150.ataspectj; -import java.io.File; - import junit.framework.Test; - import org.aspectj.testing.XMLBasedAjcTestCase; -/** +import java.io.File; + +/** * A suite for @AspectJ aspects located in java5/ataspectj * * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> */ public class AtAjSyntaxTests extends XMLBasedAjcTestCase { - - public static Test suite() { - return XMLBasedAjcTestCase.loadSuite(AtAjSyntaxTests.class); - } - protected File getSpecFile() { - return new File("../tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml"); - } + public static Test suite() { + return XMLBasedAjcTestCase.loadSuite(AtAjSyntaxTests.class); + } + + protected File getSpecFile() { + return new File("../tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml"); + } public void testSimpleBefore() { runTest("SimpleBefore"); @@ -95,4 +94,8 @@ public class AtAjSyntaxTests extends XMLBasedAjcTestCase { public void testSingletonInheritance() { runTest("singletonInheritance"); } + + public void testPerClauseInheritance() { + runTest("perClauseInheritance"); + } }
\ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/misuse.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/misuse.xml index ede182eb4..f78aa87b3 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/misuse.xml +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/misuse.xml @@ -79,7 +79,14 @@ </compile> </ajc-test> -<!-- + <ajc-test dir="java5/ataspectj" + pr="" title="@Before with PJP"> + <compile files="ataspectj/misuse/Test100.java" options="-1.5 -Xdev:NoAtAspectJProcessing -Xlint:ignore"> + <message kind="error" text="use of ProceedingJoinPoint is allowed only on around advice"/> + </compile> + </ajc-test> + + <!-- ALEX: todo <ajc-test dir="java5/ataspectj/coverage" pr="" title="@Pointcut with wrong number of args"> diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml index b3697fbb0..eeaaf79e7 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml +++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml @@ -12,13 +12,13 @@ <run class="SimpleBefore"/> </ajc-test> - <ajc-test dir="java5/ataspectj" title="SimpleAfter"> + <ajc-test dir="java5/ataspectj" title="SimpleAfter"> <compile files="SimpleAfter.java" options="-1.5 -showWeaveInfo -XnoInline"> - <message kind="weave" text="(SimpleAfter.java:13) advised by after advice from 'SimpleAfter$X'"/> + <message kind="weave" text="(SimpleAfter.java:13) advised by after advice from 'SimpleAfter$X'"/> </compile> <run class="SimpleAfter"/> <compile files="SimpleAfter.java" options="-1.5 -showWeaveInfo -XnoInline -Xdev:NoAtAspectJProcessing"> - <message kind="weave" text="(SimpleAfter.java:13) advised by after advice from 'SimpleAfter$X'"/> + <message kind="weave" text="(SimpleAfter.java:13) advised by after advice from 'SimpleAfter$X'"/> </compile> <run class="SimpleAfter"/> </ajc-test> @@ -28,7 +28,7 @@ <run class="ataspectj.SingletonAspectBindingsTest"/> <compile files="ataspectj/SingletonAspectBindingsTest.java,ataspectj/TestHelper.java" options="-1.5 -XnoInline -Xdev:NoAtAspectJProcessing"/> <run class="ataspectj.SingletonAspectBindingsTest"/> - </ajc-test> + </ajc-test> <ajc-test dir="java5/ataspectj" title="CflowTest"> <compile files="ataspectj/CflowTest.java,ataspectj/TestHelper.java" options="-1.5"/> @@ -63,11 +63,11 @@ <run class="ataspectj.AfterXTest"/> </ajc-test> -<!-- <comment>FIXME AV when we impl if support in pointcut parser and weaver</comment>--> -<!-- <ajc-test dir="java5/ataspectj" title="IfPointcutTest">--> -<!-- <compile files="ataspectj/IfPointcutTest.java,ataspectj/TestHelper.java" options="-1.5"/>--> -<!-- <run class="ataspectj.IfPointcutTest"/>--> -<!-- </ajc-test>--> + <!-- <comment>FIXME AV when we impl if support in pointcut parser and weaver</comment>--> + <!-- <ajc-test dir="java5/ataspectj" title="IfPointcutTest">--> + <!-- <compile files="ataspectj/IfPointcutTest.java,ataspectj/TestHelper.java" options="-1.5"/>--> + <!-- <run class="ataspectj.IfPointcutTest"/>--> + <!-- </ajc-test>--> <ajc-test dir="java5/ataspectj" title="BindingTest"> <compile files="ataspectj/BindingTest.java,ataspectj/TestHelper.java" options="-1.5"/> @@ -113,6 +113,13 @@ <run class="ataspectj.SingletonInheritanceTest"/> <compile files="ataspectj/SingletonInheritanceTest.java,ataspectj/TestHelper.java" options="-1.5 -XnoInline -Xdev:NoAtAspectJProcessing"/> <run class="ataspectj.SingletonInheritanceTest"/> - </ajc-test> + </ajc-test> + + <ajc-test dir="java5/ataspectj" title="perClauseInheritance"> +<!-- <compile files="ataspectj/PerClauseInheritanceTest.java,ataspectj/TestHelper.java" options="-1.5 -XnoInline"/>--> +<!-- <run class="ataspectj.PerClauseInheritanceTest"/>--> + <compile files="ataspectj/PerClauseInheritanceTest.java,ataspectj/TestHelper.java" options="-1.5 -XnoInline -Xdev:NoAtAspectJProcessing"/> + <run class="ataspectj.PerClauseInheritanceTest"/> + </ajc-test> </suite>
\ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java index 5154d1aa6..52813072d 100644 --- a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java +++ b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java @@ -782,6 +782,7 @@ public abstract class ResolvedTypeX extends TypeX implements AnnotatedElement { public void setDelegate(ConcreteName delegate) { this.delegate = delegate; } + public int getEndPos() { return endPos; } diff --git a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java index 595372848..164047189 100644 --- a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java +++ b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java @@ -51,6 +51,7 @@ import org.aspectj.weaver.patterns.PerTypeWithin; import org.aspectj.weaver.patterns.Pointcut; import org.aspectj.weaver.patterns.SimpleScope; import org.aspectj.weaver.patterns.TypePattern; +import org.aspectj.weaver.patterns.PerFromSuper; import java.util.ArrayList; import java.util.Collections; @@ -177,16 +178,15 @@ public class AtAjAttributes { // we don't need to look for several attribute occurence since it cannot happen as per JSR175 if (!isCodeStyleAspect) { hasAtAspectAnnotation = handleAspectAnnotation(rvs, struct); + //TODO AV - if put outside the if isCodeStyleAspect then we would enable mix style + hasAtPrecedenceAnnotation = handlePrecedenceAnnotation(rvs, struct); } - //TODO: below means mix style for @DeclarePrecedence - are we sure we want that ? - hasAtPrecedenceAnnotation = handlePrecedenceAnnotation(rvs, struct); // there can only be one RuntimeVisible bytecode attribute break; } } // basic semantic check - //FIXME AV TBD could be skipped and silently ignore - TBD with Andy if (hasAtPrecedenceAnnotation && !hasAtAspectAnnotation) { msgHandler.handleMessage( new Message( @@ -199,7 +199,7 @@ public class AtAjAttributes { // bypass what we have read return EMPTY_LIST; } - //FIXME turn on when ajcMightHaveAspect fixed + //FIXME turn on when ajcMightHaveAspect // if (hasAtAspectAnnotation && type.isInterface()) { // msgHandler.handleMessage( // new Message( @@ -214,7 +214,6 @@ public class AtAjAttributes { // } // the following block will not detect @Pointcut in non @Aspect types for optimization purpose - // FIXME AV TBD with Andy if (!hasAtAspectAnnotation) { return EMPTY_LIST; } @@ -222,8 +221,7 @@ public class AtAjAttributes { // code style pointcuts are class attributes // we need to gather the @AJ pointcut right now and not at method level annotation extraction time // in order to be able to resolve the pointcut references later on - //FIXME alex loop over class super class - //FIXME alex can that be too slow ? + // we don't need to look in super class, the pointcut reference in the grammar will do it for (int i = 0; i < javaClass.getMethods().length; i++) { Method method = javaClass.getMethods()[i]; if (method.getName().startsWith(NameMangler.PREFIX)) continue; // already dealt with by ajc... @@ -297,8 +295,10 @@ public class AtAjAttributes { // we remember if we found one @AJ annotation for minimal semantic error reporting // the real reporting beeing done thru AJDT and the compiler mapping @AJ to AjAtttribute // or thru APT - // FIXME AV we could actually skip the whole thing if type is not itself an @Aspect - // but then we would not see any warning. TBD with Andy + // + // Note: we could actually skip the whole thing if type is not itself an @Aspect + // but then we would not see any warning. We do bypass for pointcut but not for advice since it would + // be too silent. boolean hasAtAspectJAnnotation = false; boolean hasAtAspectJAnnotationMustReturnVoid = false; for (int i = 0; i < attributes.length; i++) { @@ -392,11 +392,25 @@ public class AtAjAttributes { private static boolean handleAspectAnnotation(RuntimeAnnotations runtimeAnnotations, AjAttributeStruct struct) { Annotation aspect = getAnnotation(runtimeAnnotations, AjcMemberMaker.ASPECT_ANNOTATION); if (aspect != null) { + // semantic check for inheritance (only one level up) + boolean extendsAspect = false; + if (!"java.lang.Object".equals(struct.enclosingType.getSuperclass().getName())) { + if (!struct.enclosingType.getSuperclass().isAbstract() && struct.enclosingType.getSuperclass().isAspect()) { + reportError("cannot extend a concrete aspect", struct); + return false; + } + extendsAspect = struct.enclosingType.getSuperclass().isAspect(); + } + ElementNameValuePair aspectPerClause = getAnnotationElement(aspect, VALUE); final PerClause perClause; if (aspectPerClause == null) { - // defaults to singleton - perClause = new PerSingleton(); + // empty value means singleton unless inherited + if (!extendsAspect) { + perClause = new PerSingleton(); + } else { + perClause = new PerFromSuper(struct.enclosingType.getSuperclass().getPerClause().getKind()); + } } else { String perX = aspectPerClause.getValue().stringifyValue(); if (perX == null || perX.length() <= 0) { @@ -409,13 +423,6 @@ public class AtAjAttributes { // could not parse it, ignore the aspect return false; } else { - // semantic check for inheritance (only one level up) - if (!"java.lang.Object".equals(struct.enclosingType.getSuperclass().getName())) { - if (!struct.enclosingType.getSuperclass().isAbstract() && struct.enclosingType.getSuperclass().isAspect()) { - reportError("cannot extend a concrete aspect", struct); - return false; - } - } perClause.setLocation(struct.context, -1, -1); struct.ajAttributes.add(new AjAttribute.Aspect(perClause)); return true; diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAccessForInlineMunger.java b/weaver/src/org/aspectj/weaver/bcel/BcelAccessForInlineMunger.java index a4727a460..e426711e5 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelAccessForInlineMunger.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelAccessForInlineMunger.java @@ -48,9 +48,6 @@ import java.util.List; * Specific state and logic is kept in the munger ala ITD so that call/get/set pointcuts can still be matched * on the wrapped member thanks to the EffectiveSignature attribute. * - * FIXME AV - this whole one should be skept when -XnoInline is used - * (add breakpoint on lazyMethodGen.setCanInline to see where things happening) - * * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a> */ public class BcelAccessForInlineMunger extends BcelTypeMunger { @@ -73,6 +70,9 @@ public class BcelAccessForInlineMunger extends BcelTypeMunger { public BcelAccessForInlineMunger(ResolvedTypeX aspectType) { super(null, aspectType); + if (aspectType.getWorld().isXnoInline()) { + throw new Error("This should not happen"); + } } public boolean munge(BcelClassWeaver weaver) { @@ -134,7 +134,7 @@ public class BcelAccessForInlineMunger extends BcelTypeMunger { InstructionHandle curr = aroundAdvice.getBody().getStart(); InstructionHandle end = aroundAdvice.getBody().getEnd(); ConstantPoolGen cpg = aroundAdvice.getEnclosingClass().getConstantPoolGen(); - InstructionFactory factory = aroundAdvice.enclosingClass.getFactory(); + InstructionFactory factory = aroundAdvice.getEnclosingClass().getFactory(); boolean realizedCannotInline = false; while (curr != end) { diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java index d3e8e9f9a..d7ba55716 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java @@ -22,6 +22,7 @@ import org.aspectj.apache.bcel.generic.InstructionFactory; import org.aspectj.apache.bcel.generic.InstructionHandle; import org.aspectj.apache.bcel.generic.InstructionList; import org.aspectj.apache.bcel.generic.ReferenceType; +import org.aspectj.apache.bcel.generic.InstructionConstants; import org.aspectj.bridge.IMessage; import org.aspectj.bridge.Message; import org.aspectj.weaver.Advice; @@ -142,23 +143,23 @@ public class BcelAdvice extends Advice { hasMatchedAtLeastOnce=true; BcelShadow shadow = (BcelShadow) s; - //FIXME AV ok ? - // callback for perObject AJC MightHaveAspect postMunge (#75442) - if (getConcreteAspect() != null - && getConcreteAspect().getPerClause() != null - && PerClause.PEROBJECT.equals(getConcreteAspect().getPerClause().getKind())) { - final PerObject clause; - if (getConcreteAspect().getPerClause() instanceof PerFromSuper) { - clause = (PerObject)((PerFromSuper) getConcreteAspect().getPerClause()).lookupConcretePerClause(getConcreteAspect()); - } else { - clause = (PerObject) getConcreteAspect().getPerClause(); - } - if (clause.isThis()) { - PerObjectInterfaceTypeMunger.registerAsAdvisedBy(s.getThisVar().getType(), getConcreteAspect()); - } else { - PerObjectInterfaceTypeMunger.registerAsAdvisedBy(s.getTargetVar().getType(), getConcreteAspect()); - } - } + //FIXME AV - see #75442, this logic is not enough so for now comment it out until we fix the bug +// // callback for perObject AJC MightHaveAspect postMunge (#75442) +// if (getConcreteAspect() != null +// && getConcreteAspect().getPerClause() != null +// && PerClause.PEROBJECT.equals(getConcreteAspect().getPerClause().getKind())) { +// final PerObject clause; +// if (getConcreteAspect().getPerClause() instanceof PerFromSuper) { +// clause = (PerObject)((PerFromSuper) getConcreteAspect().getPerClause()).lookupConcretePerClause(getConcreteAspect()); +// } else { +// clause = (PerObject) getConcreteAspect().getPerClause(); +// } +// if (clause.isThis()) { +// PerObjectInterfaceTypeMunger.registerAsAdvisedBy(s.getThisVar().getType(), getConcreteAspect()); +// } else { +// PerObjectInterfaceTypeMunger.registerAsAdvisedBy(s.getTargetVar().getType(), getConcreteAspect()); +// } +// } if (getKind() == AdviceKind.Before) { shadow.weaveBefore(this); @@ -260,8 +261,6 @@ public class BcelAdvice extends Advice { */ public boolean mustCheckExceptions() { if (getConcreteAspect() == null) { - //FIXME AV: not sure this is good to default to that. - // dig when do we reach that ie not yet concretized return true; } return !getConcreteAspect().isAnnotationStyleAspect(); @@ -362,19 +361,11 @@ public class BcelAdvice extends Advice { if (v == null) { // if not @AJ aspect, go on with the regular binding handling if (getConcreteAspect()==null || !getConcreteAspect().isAnnotationStyleAspect()) { - continue; + ; } else { // ATAJ: for @AJ aspects, handle implicit binding of xxJoinPoint if (getKind() == AdviceKind.Around) { il.append(closureInstantiation); - // -// if (canInline(shadow)) { -// //FIXME ALEX? passin a boolean instead of checking there -// il.append(fact.createCheckCast( -// (ReferenceType) BcelWorld.makeBcelType(TypeX.forName("org.aspectj.lang.ProceedingJoinPoint")) -// )); -// } - continue; } else if ("Lorg/aspectj/lang/JoinPoint$StaticPart;".equals(getSignature().getParameterTypes()[i].getSignature())) { if ((getExtraParameterFlags() & ThisJoinPointStaticPart) != 0) { shadow.getThisJoinPointStaticPartBcelVar().appendLoad(il, fact); @@ -393,9 +384,16 @@ public class BcelAdvice extends Advice { fact, getExtraParameterType().resolve(world)); } else { - //FIXME AV test for that - this code will throw an error if ProceedingJP is used in a before advice f.e. ok ?? - throw new Error("Should not happen - unbound advice argument at index " + i + " in [" + - toString() + "]"); + getConcreteAspect().getWorld().getMessageHandler().handleMessage( + new Message( + "use of ProceedingJoinPoint is allowed only on around advice (" + + "arg " + i + " in " + toString() + ")", + this.getSourceLocation(), + true + ) + ); + // try to avoid verify error and pass in null + il.append(InstructionConstants.ACONST_NULL); } } } else { diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java index eab392a17..5ea3a5278 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java @@ -382,8 +382,7 @@ class BcelClassWeaver implements IClassWeaver { isChanged = true; } } - //if (! isChanged) return false;//FIXME AV - was active, WHY ?? I need to reach the lateTypeMunger - + // now we weave all but the initialization shadows for (Iterator i = methodGens.iterator(); i.hasNext();) { LazyMethodGen mg = (LazyMethodGen)i.next(); @@ -404,14 +403,8 @@ class BcelClassWeaver implements IClassWeaver { // now proceed with late type mungers if (lateTypeMungers != null) { for (Iterator i = lateTypeMungers.iterator(); i.hasNext(); ) { - Object o = i.next(); - if ( !(o instanceof BcelTypeMunger) ) { - throw new Error("should not happen or what ?");//FIXME AV ??was System.err.println("surprising: " + o); - //continue; - } - BcelTypeMunger munger = (BcelTypeMunger)o; + BcelTypeMunger munger = (BcelTypeMunger)i.next(); if (munger.matches(clazz.getType())) { - //FIXME AV - Andy must track change by this lateMunging only and deal with a reweavable thing boolean typeMungerAffectedType = munger.munge(this); if (typeMungerAffectedType) { isChanged = true; @@ -420,8 +413,10 @@ class BcelClassWeaver implements IClassWeaver { } } } - // flush to save some memory - FIXME AV - extract in a better way ? - PerObjectInterfaceTypeMunger.unregisterFromAsAdvisedBy(clazz.getType()); + + //FIXME AV - see #75442, for now this is not enough to fix the bug, comment that out until we really fix it +// // flush to save some memory +// PerObjectInterfaceTypeMunger.unregisterFromAsAdvisedBy(clazz.getType()); // finally, if we changed, we add in the introduced methods. if (isChanged) { diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java index 75c46d5dd..47e814f6b 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java @@ -53,7 +53,12 @@ public class BcelObjectType extends ResolvedTypeX.ConcreteName { private ResolvedMember[] methods = null; private ResolvedTypeX[] annotationTypes = null; private AnnotationX[] annotations = null; - + + // track unpackAttribute. In some case (per clause inheritance) we encounter + // unpacked state when calling getPerClause + // this whole thing would require more clean up (AV) + private boolean isUnpacked = false; + // strangely non-lazy private ResolvedPointcutDefinition[] pointcuts = null; private PerClause perClause = null; @@ -191,6 +196,8 @@ public class BcelObjectType extends ResolvedTypeX.ConcreteName { } private void unpackAspectAttributes() { + isUnpacked = true; + List pointcuts = new ArrayList(); typeMungers = new ArrayList(); declares = new ArrayList(); @@ -245,6 +252,9 @@ public class BcelObjectType extends ResolvedTypeX.ConcreteName { } public PerClause getPerClause() { + if (!isUnpacked) { + unpackAspectAttributes(); + } return perClause; } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java index 218eed263..1b8a57f90 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java @@ -237,7 +237,6 @@ public class BcelWorld extends World implements Repository { protected BcelObjectType makeBcelObjectType(ResolvedTypeX.Name resolvedTypeX, JavaClass jc, boolean exposedToWeaver) { BcelObjectType ret = new BcelObjectType(resolvedTypeX, jc, exposedToWeaver); - resolvedTypeX.setDelegate(ret); return ret; } @@ -274,7 +273,6 @@ public class BcelWorld extends World implements Repository { nameTypeX = new ResolvedTypeX.Name(signature, this); } BcelObjectType ret = makeBcelObjectType(nameTypeX, jc, true); - nameTypeX.setDelegate(ret); typeMap.put(signature, nameTypeX); return ret; } diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java index 8aeec7f8f..8aae4570a 100644 --- a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java +++ b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java @@ -87,8 +87,8 @@ public final class LazyMethodGen { private InstructionList body; // leaving null for abstracts private Attribute[] attributes; // private AnnotationGen[] annotations; - /* private */ final LazyClassGen enclosingClass; - private /*final*/ BcelMethod memberView;//FIXME AV LTW + private final LazyClassGen enclosingClass; + private BcelMethod memberView; int highestLineNumber = 0; /** This is nonnull if this method is the result of an "inlining". We currently @@ -100,7 +100,7 @@ public final class LazyMethodGen { private int maxLocals; - private boolean canInline = true;//FIXME AV - ALEX? shouldn't that default to false or unknown? + private boolean canInline = true; private boolean hasExceptionHandlers; private boolean isSynthetic = false; @@ -218,9 +218,7 @@ public final class LazyMethodGen { public boolean hasAnnotation(TypeX annotationTypeX) { initialize(); if (memberView==null) { - memberView = new BcelMethod(getEnclosingClass().getBcelObjectType(), getMethod());//FIXME AV LTW - //System.err.println("REPORT THIS! 02: Can't determine if "+getEnclosingClass().getName() + "." + this.getName() + this.getSignature()+" has annotation "+annotationTypeX); - //return false + memberView = new BcelMethod(getEnclosingClass().getBcelObjectType(), getMethod()); return memberView.hasAnnotation(annotationTypeX); } return memberView.hasAnnotation(annotationTypeX); diff --git a/weaver/src/org/aspectj/weaver/patterns/PerCflow.java b/weaver/src/org/aspectj/weaver/patterns/PerCflow.java index 73eef47d7..b61223ccf 100644 --- a/weaver/src/org/aspectj/weaver/patterns/PerCflow.java +++ b/weaver/src/org/aspectj/weaver/patterns/PerCflow.java @@ -98,14 +98,14 @@ public class PerCflow extends PerClause { inAspect, innerCflowEntries)); //ATAJ: add a munger to add the aspectOf(..) to the @AJ aspects - if (inAspect.isAnnotationStyleAspect()) { + if (inAspect.isAnnotationStyleAspect() && !inAspect.isAbstract()) { inAspect.crosscuttingMembers.addLateTypeMunger( inAspect.getWorld().makePerClauseAspect(inAspect, getKind()) ); } //ATAJ inline around advice support - don't use a late munger to allow around inling for itself - if (inAspect.isAnnotationStyleAspect()) { + if (inAspect.isAnnotationStyleAspect() && !inAspect.getWorld().isXnoInline()) { inAspect.crosscuttingMembers.addTypeMunger(new BcelAccessForInlineMunger(inAspect)); } diff --git a/weaver/src/org/aspectj/weaver/patterns/PerObject.java b/weaver/src/org/aspectj/weaver/patterns/PerObject.java index 31eef9ade..a9294e853 100644 --- a/weaver/src/org/aspectj/weaver/patterns/PerObject.java +++ b/weaver/src/org/aspectj/weaver/patterns/PerObject.java @@ -111,14 +111,14 @@ public class PerObject extends PerClause { inAspect.crosscuttingMembers.addLateTypeMunger(world.concreteTypeMunger(munger, inAspect)); //ATAJ: add a munger to add the aspectOf(..) to the @AJ aspects - if (inAspect.isAnnotationStyleAspect()) { + if (inAspect.isAnnotationStyleAspect() && !inAspect.isAbstract()) { inAspect.crosscuttingMembers.addLateTypeMunger( inAspect.getWorld().makePerClauseAspect(inAspect, getKind()) ); } //ATAJ inline around advice support - don't use a late munger to allow around inling for itself - if (inAspect.isAnnotationStyleAspect()) { + if (inAspect.isAnnotationStyleAspect() && !inAspect.getWorld().isXnoInline()) { inAspect.crosscuttingMembers.addTypeMunger(new BcelAccessForInlineMunger(inAspect)); } diff --git a/weaver/src/org/aspectj/weaver/patterns/PerSingleton.java b/weaver/src/org/aspectj/weaver/patterns/PerSingleton.java index 385f9175c..5b3b9f954 100644 --- a/weaver/src/org/aspectj/weaver/patterns/PerSingleton.java +++ b/weaver/src/org/aspectj/weaver/patterns/PerSingleton.java @@ -96,7 +96,7 @@ public class PerSingleton extends PerClause { ret.inAspect = inAspect; //ATAJ: add a munger to add the aspectOf(..) to the @AJ aspects - if (!inAspect.isAbstract() && inAspect.isAnnotationStyleAspect()) { + if (inAspect.isAnnotationStyleAspect() && !inAspect.isAbstract()) { //TODO will those change be ok if we add a serializable aspect ? // dig: "can't be Serializable/Cloneable unless -XserializableAspects" inAspect.crosscuttingMembers.addLateTypeMunger( @@ -105,7 +105,7 @@ public class PerSingleton extends PerClause { } //ATAJ inline around advice support - if (inAspect.isAnnotationStyleAspect()) { + if (inAspect.isAnnotationStyleAspect() && !inAspect.getWorld().isXnoInline()) { inAspect.crosscuttingMembers.addTypeMunger(new BcelAccessForInlineMunger(inAspect)); } diff --git a/weaver/src/org/aspectj/weaver/patterns/PerTypeWithin.java b/weaver/src/org/aspectj/weaver/patterns/PerTypeWithin.java index 651d192a3..bd8873e27 100644 --- a/weaver/src/org/aspectj/weaver/patterns/PerTypeWithin.java +++ b/weaver/src/org/aspectj/weaver/patterns/PerTypeWithin.java @@ -148,14 +148,14 @@ public class PerTypeWithin extends PerClause { inAspect.crosscuttingMembers.addTypeMunger(world.concreteTypeMunger(munger, inAspect)); //ATAJ: add a munger to add the aspectOf(..) to the @AJ aspects - if (inAspect.isAnnotationStyleAspect()) { + if (inAspect.isAnnotationStyleAspect() && !inAspect.isAbstract()) { inAspect.crosscuttingMembers.addLateTypeMunger( inAspect.getWorld().makePerClauseAspect(inAspect, getKind()) ); } //ATAJ inline around advice support - don't use a late munger to allow around inling for itself - if (inAspect.isAnnotationStyleAspect()) { + if (inAspect.isAnnotationStyleAspect() && !inAspect.getWorld().isXnoInline()) { inAspect.crosscuttingMembers.addTypeMunger(new BcelAccessForInlineMunger(inAspect)); } diff --git a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java index 67bd70f3c..1eb97964f 100644 --- a/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java +++ b/weaver/src/org/aspectj/weaver/tools/WeavingAdaptor.java @@ -192,8 +192,8 @@ public class WeavingAdaptor { } private boolean shouldWeaveName (String name) { - return !((name.startsWith("org.apache.bcel.")//FIXME AV why ? bcel is wrapped in org.aspectj. - || name.startsWith("org.aspectj.") + return !((/*(name.startsWith("org.apache.bcel.")//FIXME AV why ? bcel is wrapped in org.aspectj. + ||*/ name.startsWith("org.aspectj.") || name.startsWith("java.") || name.startsWith("javax.")) || name.startsWith("$Proxy"));//JDK proxies |