perClause inheritance in @AJ (in ajdt module), fixed FIXME AV

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;
 	}

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" 

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");
         }

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;
     }

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");
+        }
+    }
+
+
+}

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
+        }
+    }
+}

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>

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");
+    }
 }

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");
+    }
 }

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">

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>

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;
 		}

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;

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) {

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 {

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) {

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;
 	}
     

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;
 	}

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);

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));
         }
 

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));
         }
 

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));
         }
 

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));
         }
 

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