From 76ebbc76add2abd815b3a8b5ea0beb11c94c8c49 Mon Sep 17 00:00:00 2001
From: avasseur <avasseur>
Date: Tue, 25 Oct 2005 10:00:58 +0000
Subject: concrete-aspect impl and doc for LTW - see #95529 pbly some issue on
 abstract @Pointcut() in ajdt core - fix coming

---
 .../org/aspectj/lang/annotation/Pointcut.java      |   3 +-
 docs/adk15ProgGuideDB/ltw.xml                      |  20 ++
 docs/devGuideDB/ltw.xml                            |  96 ++++++-
 lib/test/aspectjrt.jar                             | Bin 107654 -> 107208 bytes
 loadtime/.classpath                                |   1 +
 loadtime/src/aspectj_1_5_0.dtd                     |   1 +
 .../weaver/loadtime/ClassLoaderWeavingAdaptor.java |  64 ++++-
 .../weaver/loadtime/ConcreteAspectCodeGen.java     | 320 +++++++++++++++++++++
 .../weaver/loadtime/WeavingURLClassLoader.java     |   4 +-
 .../weaver/loadtime/definition/Definition.java     |  16 +-
 .../weaver/loadtime/definition/DocumentParser.java |   8 +-
 .../ataspectj/ataspectj/ConcreteAspectTest.aj      |  62 ++++
 .../ataspectj/ataspectj/ConcreteAtAspectTest.java  |  72 +++++
 .../ataspectj/ataspectj/aop-concreteaspect.xml     |   9 +
 .../ataspectj/ataspectj/aop-concreteataspect.xml   |  10 +
 .../systemtest/ajc150/ataspectj/AtAjLTWTests.java  |   7 +
 .../aspectj/systemtest/ajc150/ataspectj/ltw.xml    |  16 ++
 .../org/aspectj/weaver/bcel/AtAjAttributes.java    | 108 ++++---
 .../weaver/bcel/BcelPerClauseAspectAdder.java      |   4 +
 weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java |   8 +-
 20 files changed, 755 insertions(+), 74 deletions(-)
 create mode 100644 loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
 create mode 100644 tests/java5/ataspectj/ataspectj/ConcreteAspectTest.aj
 create mode 100644 tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java
 create mode 100644 tests/java5/ataspectj/ataspectj/aop-concreteaspect.xml
 create mode 100644 tests/java5/ataspectj/ataspectj/aop-concreteataspect.xml

diff --git a/aspectj5rt/java5-src/org/aspectj/lang/annotation/Pointcut.java b/aspectj5rt/java5-src/org/aspectj/lang/annotation/Pointcut.java
index ec4b3461e..6fae1e8b9 100644
--- a/aspectj5rt/java5-src/org/aspectj/lang/annotation/Pointcut.java
+++ b/aspectj5rt/java5-src/org/aspectj/lang/annotation/Pointcut.java
@@ -27,8 +27,9 @@ public @interface Pointcut {
 
     /**
      * The pointcut expression
+     * We allow "" as default for abstract pointcut
      */
-    String value();
+    String value() default "";
     
     /**
      * When compiling without debug info, or when interpreting pointcuts at runtime,
diff --git a/docs/adk15ProgGuideDB/ltw.xml b/docs/adk15ProgGuideDB/ltw.xml
index 5731e4e7b..f1c79c04d 100644
--- a/docs/adk15ProgGuideDB/ltw.xml
+++ b/docs/adk15ProgGuideDB/ltw.xml
@@ -1,4 +1,24 @@
 <chapter id="ltw" xreflabel="Load-Time Weaving">
+
+    <!--
+
+
+
+    DO NOT MODIFY THIS ONE
+    Wes is in the process of polishing docs
+    Do changes in the devGuideDB/ltw.xml instead
+
+
+
+
+
+
+
+
+
+    -->
+
+
     <title>Load-Time Weaving</title>
     
     <sect1 id="ltw-introduction">
diff --git a/docs/devGuideDB/ltw.xml b/docs/devGuideDB/ltw.xml
index b0abbf37a..09ac6520c 100644
--- a/docs/devGuideDB/ltw.xml
+++ b/docs/devGuideDB/ltw.xml
@@ -39,8 +39,8 @@
                 need to specify the <literal>-Xreweavable</literal> compiler option when building
                 them. This causes AspectJ to save additional state in the class files that is used
                 to support subsequent reweaving. </para>
-            <para><!-- FIXME AV -->As per AspectJ 1.5.0 M3 aspects (code style or annotation style) are
-            reweavable by default, and weaved classes may be as well in 1.5.0 final.</para>
+            <para>As per AspectJ 1.5.0 M3 aspects (code style or annotation style) are
+            reweavable by default, and weaved classes are reweavable by default as well as per AspectJ 1.5.0 M4.</para>
         </sect2>
     </sect1>
     
@@ -206,12 +206,7 @@
                 useful way of externalizing configuration for infrastructure and
                 auxiliary aspects where the pointcut definitions themselves can be
                 considered part of the configuration of the service.
-            </para>
-
-            <para>
-                <emphasis>
-                Note: concrete-aspect is not available in AspectJ 1.5 M3.
-                </emphasis>
+                Refer to the next section for more details.
             </para>
 
             <para>
@@ -272,6 +267,91 @@
                 each concrete aspect included in the JAR. </para>
         </sect2>
 
+        <sect2 id="concrete-aspect" xreflabel="concrete-aspect">
+            <title>Using Concrete Aspects</title>
+            <para>
+                It is possible to concretize an abstract aspect by the mean of the <literal>META-INF/aop.xml</literal>
+                file. This is usefull to define abstract pointcuts at deployment time.
+                Consider the following:
+            </para>
+            <programlisting><![CDATA[
+            package mypack;
+
+            @Aspect
+            public abstract class AbstractAspect {
+
+                // abstract pointcut: no expression is defined
+                @Pointcut
+                abstract void scope();
+
+                @Before("scope() && execution(* *..doSome(..))")
+                public void before(JoinPoint jp) {
+                   ....
+                }
+            }
+            ]]></programlisting>
+            <para>
+                This aspect is equivalent to the following in code style:
+            </para>
+            <programlisting><![CDATA[
+            package mypack;
+
+            public abstract aspect AbstractAspect {
+
+                // abstract pointcut: no expression is defined
+                abstract pointcut scope();
+
+                before() : scope() && execution(* *..doSome(..)) {
+                   ....
+                }
+            }
+            ]]></programlisting>
+            <para>
+                This aspect (in either of its style) is a good candidate for concretization through <literal>META-INF/aop.xml</literal>.
+                It defines the abstract pointcut <literal>within()</literal>. It is important to remember that
+                concretization in this case must obey to the following rules:
+                <itemizedlist>
+                    <listitem><para>The parent aspect must be abstract. It can be an @AspectJ or a
+                            regular code style aspect.</para></listitem>
+                    <listitem><para>Only simple abstract pointcut can be concretized ie pointcut that don't expose
+                            state (through <literal>args(), this(), target(), if()</literal>). In @AspectJ syntax
+                            as illustrated in this sample, this means the method that hosts the pointcut is abstract,
+                            has no arguments, and returns void.</para></listitem>
+                    <listitem><para>Concretization must defines all such abstract pointcuts ie it is not possible
+                            to have <literal>concrete-aspect</literal> inter dependancies.</para></listitem>
+                    <listitem><para>Concretization can only concretize pointcuts ie there cannot be abstract method
+                            left in the aspect.</para></listitem>
+                </itemizedlist>
+                If you have requirements for more complex aspect inheritance, you should consider regular aspect
+                inheritance instead of concretization through XML.
+                Given that the following XML is valid:
+            </para>
+            <programlisting><![CDATA[
+            <aspectj>
+                <conrete-aspect name="mypack.__My__AbstractAspect" extends="mypack.AbstractAspect">
+                    <pointcut name="scope" expression="within(yourpackage..*)"/>
+                </concrete-aspect>
+            </aspectj>
+            ]]></programlisting>
+            <para>
+                It is important to remember that the <literal>name</literal> attribute in the XML directive
+                <literal>concrete-aspect</literal> defines the fully qualified name that will be given to the
+                concrete aspect. It must then be a valid class name. This one will indeed be generated on the fly by the weaver internals. You must
+                then ensure that there won't be name collision. Also note that the concrete aspect will be
+                defined at the classloader level for which the aop.xml is visible. This implies that if you need
+                to use the <literal>aspectof</literal> methods to access the aspect instance(s) (depending on the perclause
+                of the aspect it extends) you have to use the helper API <literal>org.aspectj.lang.Aspects.aspectOf(..)</literal>
+                as in:
+            </para>
+            <programlisting><![CDATA[
+                // exception handling omitted
+                Class myConcreteAspectClass = Class.forName("mypack.__My__AbstractAspect");
+
+                // here we are using a singleton aspect
+                AbstractAspect concreteInstance = Aspects.aspectOf(myConcreteAspectClass);
+            ]]></programlisting>
+        </sect2>
+
         <!-- TODO someone implement that -->
         <!--
         <sect2 id="configuring-load-time-weaving-with-properties-files" xreflabel="configuring-load-time-weaving-with-properties-files">
diff --git a/lib/test/aspectjrt.jar b/lib/test/aspectjrt.jar
index 4a6eea4b9..8302072b1 100644
Binary files a/lib/test/aspectjrt.jar and b/lib/test/aspectjrt.jar differ
diff --git a/loadtime/.classpath b/loadtime/.classpath
index 5a9f37bcd..8ac0f898e 100644
--- a/loadtime/.classpath
+++ b/loadtime/.classpath
@@ -7,6 +7,7 @@
 	<classpathentry combineaccessrules="false" kind="src" path="/bridge"/>
 	<classpathentry combineaccessrules="false" kind="src" path="/util"/>
 	<classpathentry combineaccessrules="false" kind="src" path="/weaver"/>
+	<classpathentry combineaccessrules="false" kind="src" path="/bcel-builder"/>
 	<classpathentry kind="lib" path="/lib/ext/jrockit/managementapi-jrockit81.jar"/>
 	<classpathentry sourcepath="/lib/junit/junit-src.jar" kind="lib" path="/lib/junit/junit.jar"/>
 	<classpathentry kind="lib" path="/lib/ant/lib/xml-apis.jar"/>
diff --git a/loadtime/src/aspectj_1_5_0.dtd b/loadtime/src/aspectj_1_5_0.dtd
index 710c0dac8..060962745 100644
--- a/loadtime/src/aspectj_1_5_0.dtd
+++ b/loadtime/src/aspectj_1_5_0.dtd
@@ -130,6 +130,7 @@ concrete-aspect
 <!ATTLIST concrete-aspect
     name CDATA #REQUIRED
     extends CDATA #REQUIRED
+    precedence CDATA #IMPLIED
 >
 <!--*****************************************************************************************************************************
 pointcut
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
index 547b14170..ebfdce180 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
@@ -71,7 +71,6 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
              * @param bytes
              */
             public void acceptClass(String name, byte[] bytes) {
-                //TODO av make dump configurable
                 try {
                     if (shouldDump(name.replace('/', '.'))) {
                         Aj.dump(name, bytes);
@@ -79,6 +78,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
                 } catch (Throwable throwable) {
                     throwable.printStackTrace();
                 }
+
                 Aj.defineClass(loader, name, bytes);// could be done lazily using the hook
             }
         };
@@ -113,6 +113,23 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             messageHandler = bcelWorld.getMessageHandler();
             // after adding aspects
             weaver.prepareForWeave();
+//            // weave and flush what was registered so far
+//            for (Iterator iterator = m_codeGens.iterator(); iterator.hasNext();) {
+//                ConcreteAspectCodeGen concreteAspectCodeGen = (ConcreteAspectCodeGen) iterator.next();
+//                byte[] partiallyWoven = concreteAspectCodeGen.getBytes(this);
+//                this.generatedClassHandler.acceptClass(
+//                        concreteAspectCodeGen.m_concreteAspect.name,
+//                        partiallyWoven
+//                );
+//                ResolvedType aspect = weaver.addLibraryAspect(concreteAspectCodeGen.m_concreteAspect.name);
+//                //generate key for SC
+//                String aspectCode = readAspect(concreteAspectCodeGen.m_concreteAspect.name, loader);
+//                if(namespace==null){
+//                    namespace=new StringBuffer(aspectCode);
+//                }else{
+//                    namespace = namespace.append(";"+aspectCode);
+//                }
+//            }
         }
     }
 
@@ -257,16 +274,21 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
         //TODO: the exclude aspect allow to exclude aspect defined upper in the CL hierarchy - is it what we want ??
         // if not, review the getResource so that we track which resource is defined by which CL
 
-        //it aspectClassNames
+        //iterate aspectClassNames
         //exclude if in any of the exclude list
         for (Iterator iterator = definitions.iterator(); iterator.hasNext();) {
             Definition definition = (Definition) iterator.next();
             for (Iterator aspects = definition.getAspectClassNames().iterator(); aspects.hasNext();) {
                 String aspectClassName = (String) aspects.next();
                 if (acceptAspect(aspectClassName)) {
-                    weaver.addLibraryAspect(aspectClassName);
-                	
-                	//generate key for SC
+                    ResolvedType aspect = weaver.addLibraryAspect(aspectClassName);
+                    if (aspect.isAbstract()) {
+                        // this is a warning
+                        weaver.getWorld().getMessageHandler().handleMessage(
+                                new Message("Abstract aspect registered in aop.xml, use a <concrete-aspect> element instead", IMessage.WARNING, null, null)
+                        );
+                    }
+                    //generate key for SC
                 	String aspectCode = readAspect(aspectClassName, loader);
                     if(namespace==null){
                     	namespace=new StringBuffer(aspectCode);
@@ -277,9 +299,35 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
             }
         }
 
-        //it concreteAspects
-        //exclude if in any of the exclude list
-        //TODO
+        //iterate concreteAspects
+        //exclude if in any of the exclude list - note that the user defined name matters for that to happen
+        for (Iterator iterator = definitions.iterator(); iterator.hasNext();) {
+            Definition definition = (Definition) iterator.next();
+            for (Iterator aspects = definition.getConcreteAspects().iterator(); aspects.hasNext();) {
+                Definition.ConcreteAspect concreteAspect = (Definition.ConcreteAspect) aspects.next();
+                if (acceptAspect(concreteAspect.name)) {
+                    ConcreteAspectCodeGen gen = new ConcreteAspectCodeGen(concreteAspect, weaver.getWorld());
+                    if (!gen.validate()) {
+                        weaver.getWorld().getMessageHandler().handleMessage(
+                                new Message("Concrete-aspect '"+concreteAspect.name+"' could not be registered", IMessage.ERROR, null, null)
+                        );
+                        break;
+                    }
+                    this.generatedClassHandler.acceptClass(
+                            concreteAspect.name,
+                            gen.getBytes()
+                    );
+                    ResolvedType aspect = weaver.addLibraryAspect(concreteAspect.name);
+                    //generate key for SC
+                	String aspectCode = readAspect(concreteAspect.name, loader);
+                    if(namespace==null){
+                    	namespace=new StringBuffer(aspectCode);
+                    }else{
+                    	namespace = namespace.append(";"+aspectCode);
+                    }
+                }
+            }
+        }
     }
 
     /**
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
new file mode 100644
index 000000000..b90ddeda0
--- /dev/null
+++ b/loadtime/src/org/aspectj/weaver/loadtime/ConcreteAspectCodeGen.java
@@ -0,0 +1,320 @@
+/*******************************************************************************
+ * 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 org.aspectj.weaver.loadtime;
+
+import org.aspectj.apache.bcel.Constants;
+import org.aspectj.apache.bcel.classfile.JavaClass;
+import org.aspectj.apache.bcel.generic.InstructionConstants;
+import org.aspectj.apache.bcel.generic.InstructionList;
+import org.aspectj.apache.bcel.generic.ObjectType;
+import org.aspectj.apache.bcel.generic.Type;
+import org.aspectj.apache.bcel.generic.annotation.AnnotationGen;
+import org.aspectj.apache.bcel.generic.annotation.ElementNameValuePairGen;
+import org.aspectj.apache.bcel.generic.annotation.ElementValueGen;
+import org.aspectj.apache.bcel.generic.annotation.SimpleElementValueGen;
+import org.aspectj.bridge.IMessage;
+import org.aspectj.bridge.Message;
+import org.aspectj.weaver.AnnotationX;
+import org.aspectj.weaver.ResolvedMember;
+import org.aspectj.weaver.ResolvedType;
+import org.aspectj.weaver.UnresolvedType;
+import org.aspectj.weaver.World;
+import org.aspectj.weaver.bcel.BcelPerClauseAspectAdder;
+import org.aspectj.weaver.bcel.BcelWorld;
+import org.aspectj.weaver.bcel.LazyClassGen;
+import org.aspectj.weaver.bcel.LazyMethodGen;
+import org.aspectj.weaver.loadtime.definition.Definition;
+import org.aspectj.weaver.patterns.PerClause;
+
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+
+/**
+ * Generates bytecode for concrete-aspect
+ * <p/>
+ * The concrete aspect is @AspectJ code generated. As it is build during aop.xml definitions registration
+ * we perform the type munging for perclause ie aspectOf artifact directly, instead of waiting for it
+ * to go thru the weaver (that we are in the middle of configuring).
+ *
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class ConcreteAspectCodeGen {
+
+    private final static String[] EMPTY_STRINGS = new String[0];
+    private final static Type[] EMPTY_TYPES = new Type[0];
+
+    /**
+     * Concrete aspect definition we build for
+     */
+    private final Definition.ConcreteAspect m_concreteAspect;
+
+    /**
+     * World for which we build for
+     */
+    private final World m_world;
+
+    /**
+     * Set to true when all is checks are verified
+     */
+    private boolean m_isValid = false;
+
+    /**
+     * The parent aspect, not concretized
+     */
+    private ResolvedType m_parent;
+
+    /**
+     * Aspect perClause, used for direct munging of aspectOf artifacts
+     */
+    private PerClause m_perClause;
+
+    /**
+     * Create a new compiler for a concrete aspect
+     *
+     * @param concreteAspect
+     * @param world
+     */
+    ConcreteAspectCodeGen(Definition.ConcreteAspect concreteAspect, World world) {
+        m_concreteAspect = concreteAspect;
+        m_world = world;
+    }
+
+    /**
+     * Checks that concrete aspect is valid
+     *
+     * @return true if ok, false otherwise
+     */
+    public boolean validate() {
+        if (!(m_world instanceof BcelWorld)) {
+            reportError("Internal error: world must be of type BcelWorld");
+            return false;
+        }
+
+        m_parent = m_world.resolve(m_concreteAspect.extend, true);
+        // handle inner classes
+        if (m_parent.equals(ResolvedType.MISSING)) {
+            // fallback on inner class lookup mechanism
+            String fixedName = m_concreteAspect.extend;
+            int hasDot = fixedName.lastIndexOf('.');
+            while (hasDot > 0) {
+                char[] fixedNameChars = fixedName.toCharArray();
+                fixedNameChars[hasDot] = '$';
+                fixedName = new String(fixedNameChars);
+                hasDot = fixedName.lastIndexOf('.');
+                m_parent = m_world.resolve(UnresolvedType.forName(fixedName), true);
+                if (!m_parent.equals(ResolvedType.MISSING)) {
+                    break;
+                }
+            }
+        }
+        if (m_parent.isMissing()) {
+            reportError("Cannot find m_parent aspect for: " + stringify());
+            return false;
+        }
+
+        // extends must be abstract
+        if (!m_parent.isAbstract()) {
+            reportError("Attempt to concretize a non-abstract aspect: " + stringify());
+            return false;
+        }
+
+        // m_parent must be aspect
+        if (!m_parent.isAspect()) {
+            reportError("Attempt to concretize a non aspect: " + stringify());
+            return false;
+        }
+
+        // must be undefined so far
+        ResolvedType current = m_world.resolve(m_concreteAspect.name, true);
+        if (!current.isMissing()) {
+            reportError("Attempt to concretize but choosen aspect name already defined:" + stringify());
+            return false;
+        }
+
+        // must have all abstractions defined
+        List elligibleAbstractions = new ArrayList();
+        Iterator methods = m_parent.getMethods();
+        while (methods.hasNext()) {
+            ResolvedMember method = (ResolvedMember) methods.next();
+            if (method.isAbstract()) {
+                if ("()V".equals(method.getSignature())) {
+                    elligibleAbstractions.add(method.getName());
+                } else {
+                    reportError("Abstract member '" + method.getName() + "' cannot be concretized as a pointcut (illegal signature): " + stringify());
+                    return false;
+                }
+            }
+        }
+        List pointcutNames = new ArrayList();
+        for (Iterator it = m_concreteAspect.pointcuts.iterator(); it.hasNext();) {
+            Definition.Pointcut abstractPc = (Definition.Pointcut) it.next();
+            pointcutNames.add(abstractPc.name);
+        }
+        for (Iterator it = elligibleAbstractions.iterator(); it.hasNext();) {
+            String elligiblePc = (String) it.next();
+            if (!pointcutNames.contains(elligiblePc)) {
+                reportError("Abstract pointcut '" + elligiblePc + "' not configured: " + stringify());
+                return false;
+            }
+        }
+
+        m_perClause = m_parent.getPerClause();
+        m_isValid = true;
+        return m_isValid;
+    }
+
+    /**
+     * Rebuild the XML snip that defines this concrete aspect, for log error purpose
+     *
+     * @return string repr.
+     */
+    private String stringify() {
+        StringBuffer sb = new StringBuffer("<concrete-aspect name='");
+        sb.append(m_concreteAspect.name);
+        sb.append("' extends='");
+        sb.append(m_concreteAspect.extend);
+        sb.append("'/> in aop.xml");
+        return sb.toString();
+    }
+
+    /**
+     * Build the bytecode for the concrete aspect
+     *
+     * @return concrete aspect bytecode
+     */
+    public byte[] getBytes() {
+        if (!m_isValid) {
+            throw new RuntimeException("Must validate first");
+        }
+
+        //TODO AV - abstract away from BCEL...
+        // @Aspect //inherit clause from m_parent
+        // @DeclarePrecedence("....") // if any
+        // public class xxxName extends xxxExtends {
+        //    @Pointcut(xxxExpression-n)
+        //    private void xxxName-n() {}
+        // }
+
+        // @Aspect public class ...
+        LazyClassGen cg = new LazyClassGen(
+                m_concreteAspect.name.replace('.', '/'),
+                m_parent.getName(),
+                null,//TODO AV - we could point to the aop.xml that defines it and use JSR-45
+                Modifier.PUBLIC + Constants.ACC_SUPER,
+                EMPTY_STRINGS,
+                m_world
+        );
+        AnnotationGen ag = new AnnotationGen(
+                new ObjectType("org/aspectj/lang/annotation/Aspect"),
+                Collections.EMPTY_LIST,
+                true,
+                cg.getConstantPoolGen()
+        );
+        cg.addAnnotation(ag.getAnnotation());
+        if (m_concreteAspect.precedence != null) {
+            SimpleElementValueGen svg = new SimpleElementValueGen(
+                    ElementValueGen.STRING,
+                    cg.getConstantPoolGen(),
+                    m_concreteAspect.precedence
+            );
+            List elems = new ArrayList();
+            elems.add(new ElementNameValuePairGen("value", svg, cg.getConstantPoolGen()));
+            AnnotationGen agprec = new AnnotationGen(
+                    new ObjectType("org/aspectj/lang/annotation/DeclarePrecedence"),
+                    elems,
+                    true,
+                    cg.getConstantPoolGen()
+            );
+            cg.addAnnotation(agprec.getAnnotation());
+        }
+
+        // default constructor
+        LazyMethodGen init = new LazyMethodGen(
+                Modifier.PUBLIC,
+                Type.VOID,
+                "<init>",
+                EMPTY_TYPES,
+                EMPTY_STRINGS,
+                cg
+        );
+        InstructionList cbody = init.getBody();
+        cbody.append(InstructionConstants.ALOAD_0);
+        cbody.append(cg.getFactory().createInvoke(
+                m_parent.getName().replace('.', '/'),
+                "<init>",
+                Type.VOID,
+                EMPTY_TYPES,
+                Constants.INVOKESPECIAL
+        ));
+        cbody.append(InstructionConstants.RETURN);
+        cg.addMethodGen(init);
+
+        for (Iterator it = m_concreteAspect.pointcuts.iterator(); it.hasNext();) {
+            Definition.Pointcut abstractPc = (Definition.Pointcut) it.next();
+
+            LazyMethodGen mg = new LazyMethodGen(
+                    Modifier.PUBLIC,
+                    Type.VOID,
+                    abstractPc.name,
+                    EMPTY_TYPES,
+                    EMPTY_STRINGS,
+                    cg
+            );
+            SimpleElementValueGen svg = new SimpleElementValueGen(
+                    ElementValueGen.STRING,
+                    cg.getConstantPoolGen(),
+                    abstractPc.expression
+            );
+            List elems = new ArrayList();
+            elems.add(new ElementNameValuePairGen("value", svg, cg.getConstantPoolGen()));
+            AnnotationGen mag = new AnnotationGen(
+                    new ObjectType("org/aspectj/lang/annotation/Pointcut"),
+                    elems,
+                    true,
+                    cg.getConstantPoolGen()
+            );
+            AnnotationX max = new AnnotationX(mag.getAnnotation(), m_world);
+            mg.addAnnotation(max);
+
+            InstructionList body = mg.getBody();
+            body.append(InstructionConstants.RETURN);
+
+            cg.addMethodGen(mg);
+        }
+
+        // handle the perClause
+        BcelPerClauseAspectAdder perClauseMunger = new BcelPerClauseAspectAdder(
+                ResolvedType.forName(m_concreteAspect.name).resolve(m_world),
+                m_perClause.getKind()
+        );
+        perClauseMunger.forceMunge(cg);
+
+        //TODO AV - unsafe cast
+        // register the fresh new class into the world repository as it does not exist on the classpath anywhere
+        JavaClass jc = cg.getJavaClass((BcelWorld) m_world);
+        ((BcelWorld) m_world).addSourceObjectType(jc);
+
+        return jc.getBytes();
+    }
+
+    /**
+     * Error reporting
+     *
+     * @param message
+     */
+    private void reportError(String message) {
+        m_world.getMessageHandler().handleMessage(new Message(message, IMessage.ERROR, null, null));
+    }
+}
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
index a80a4a918..8c7035b5e 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
@@ -42,7 +42,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
 	 */
 	public WeavingURLClassLoader (ClassLoader parent) {
 		this(getURLs(getClassPath()),getURLs(getAspectPath()),parent);
-//		System.err.println("? WeavingURLClassLoader.<init>(" + parent + ")");
+//		System.err.println("? WeavingURLClassLoader.<init>(" + m_parent + ")");
 	}
 	
 	public WeavingURLClassLoader (URL[] urls, ClassLoader parent) {
@@ -55,7 +55,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
 //		System.err.println("? WeavingURLClassLoader.<init>() classURLs=" + classURLs.length + ", aspectURLs=" + aspectURLs.length);
 		this.aspectURLs = aspectURLs;
 		
-		/* If either we nor our parent is using an ASPECT_PATH use a new-style
+		/* If either we nor our m_parent is using an ASPECT_PATH use a new-style
 		 * adaptor
 		 */ 
 		if (this.aspectURLs.length > 0 || parent instanceof WeavingClassLoader) {
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
index 73677e60f..87c2f82e2 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/Definition.java
@@ -74,20 +74,26 @@ public class Definition {
     }
 
     public static class ConcreteAspect {
-        String name;
-        String extend;
-        List pointcuts;
+        public final String name;
+        public final String extend;
+        public final String precedence;
+        public final List pointcuts;
 
         public ConcreteAspect(String name, String extend) {
+            this(name, extend,  null);
+        }
+
+        public ConcreteAspect(String name, String extend, String precedence) {
             this.name = name;
             this.extend = extend;
+            this.precedence = precedence;
             this.pointcuts = new ArrayList();
         }
     }
 
     public static class Pointcut {
-        String name;
-        String expression;
+        public final String name;
+        public final String expression;
         public Pointcut(String name, String expression) {
             this.name = name;
             this.expression = expression;
diff --git a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
index 9dbbfa18f..1aeccaf1d 100644
--- a/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
+++ b/loadtime/src/org/aspectj/weaver/loadtime/definition/DocumentParser.java
@@ -56,6 +56,7 @@ public class DocumentParser extends DefaultHandler {
     private final static String CONCRETE_ASPECT_ELEMENT = "concrete-aspect";
     private final static String NAME_ATTRIBUTE = "name";
     private final static String EXTEND_ATTRIBUTE = "extends";
+    private final static String PRECEDENCE_ATTRIBUTE = "precedence";
     private final static String POINTCUT_ELEMENT = "pointcut";
     private final static String WITHIN_ATTRIBUTE = "within";
     private final static String EXPRESSION_ATTRIBUTE = "expression";
@@ -149,8 +150,13 @@ public class DocumentParser extends DefaultHandler {
         } else if (CONCRETE_ASPECT_ELEMENT.equals(qName)) {
             String name = attributes.getValue(NAME_ATTRIBUTE);
             String extend = attributes.getValue(EXTEND_ATTRIBUTE);
+            String precedence = attributes.getValue(PRECEDENCE_ATTRIBUTE);
             if (!isNull(name) && !isNull(extend)) {
-                m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend);
+                if (isNull(precedence)) {
+                    m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend);
+                } else {
+                    m_lastConcreteAspect = new Definition.ConcreteAspect(name, extend, precedence);
+                }
                 m_definition.getConcreteAspects().add(m_lastConcreteAspect);
             }
         } else if (POINTCUT_ELEMENT.equals(qName) && m_lastConcreteAspect != null) {
diff --git a/tests/java5/ataspectj/ataspectj/ConcreteAspectTest.aj b/tests/java5/ataspectj/ataspectj/ConcreteAspectTest.aj
new file mode 100644
index 000000000..d3bcd2136
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/ConcreteAspectTest.aj
@@ -0,0 +1,62 @@
+/*******************************************************************************
+ * 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 junit.framework.TestCase;
+import junit.framework.Test;
+import junit.framework.TestSuite;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+import org.aspectj.lang.annotation.Pointcut;
+
+/**
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class ConcreteAspectTest extends TestCase {
+
+    static int I;
+
+    void target() {
+        I++;
+    }
+
+    // abstract aspect
+    // pc() is undefined hence always false, and advice not applied
+    // this aspect is illegal as is in aop.xml
+    // ones must use a concrete-aspect
+    static abstract aspect ConcreteAspect {
+
+        abstract pointcut pc();
+        // must be abstract
+        // for concrete-aspect, must further be no-arg, void
+        // but can be more complex for non-xml inheritance
+
+        before() : pc() {
+            I++;
+        }
+    }
+
+    public void testConcrete() {
+        I = 0;
+        target();
+        assertEquals(2, I);
+    }
+
+    public static void main(String[] args) {
+        TestHelper.runAndThrowOnFailure(suite());
+    }
+
+    public static Test suite() {
+        return new TestSuite(ConcreteAspectTest.class);
+    }
+
+}
diff --git a/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java b/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java
new file mode 100644
index 000000000..180ec4f2b
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/ConcreteAtAspectTest.java
@@ -0,0 +1,72 @@
+/*******************************************************************************
+ * 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 junit.framework.TestCase;
+import junit.framework.Test;
+import junit.framework.TestSuite;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+import org.aspectj.lang.annotation.Pointcut;
+
+/**
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class ConcreteAtAspectTest extends TestCase {
+
+    static int I;
+
+    void target() {
+        I++;
+    }
+
+    // abstract aspect
+    // pc() is undefined hence always false, and advice not applied
+    // this aspect is illegal as is in aop.xml
+    // ones must use a concrete-aspect
+    @Aspect
+    abstract static class ConcreteAtAspect {
+
+        @Pointcut()
+        abstract void pc();
+        // must be abstract
+        // for concrete-aspect, must further be no-arg, void
+        // but can be more complex for non-xml inheritance
+
+        @Before("pc()")
+        public void before() {
+            I++;
+        }
+    }
+
+    // legal abstract aspect resolved with inheritance
+    @Aspect
+    static class ConcreteAtAspectSub extends ConcreteAtAspect {
+        @Pointcut("execution(* ataspectj.ConcreteAtAspectTest.target())")
+        void pc() {}
+    }
+
+    public void testConcrete() {
+        I = 0;
+        target();
+        assertEquals(3, I);
+    }
+
+    public static void main(String[] args) {
+        TestHelper.runAndThrowOnFailure(suite());
+    }
+
+    public static Test suite() {
+        return new TestSuite(ConcreteAtAspectTest.class);
+    }
+
+}
diff --git a/tests/java5/ataspectj/ataspectj/aop-concreteaspect.xml b/tests/java5/ataspectj/ataspectj/aop-concreteaspect.xml
new file mode 100644
index 000000000..a9f40938d
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/aop-concreteaspect.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0"?>
+<aspectj>
+    <weaver options="-XmessageHandlerClass:ataspectj.TestHelper -1.5"/>
+    <aspects>
+        <concrete-aspect name="ataspectj.Foo" extends="ataspectj.ConcreteAspectTest.ConcreteAspect">
+            <pointcut name="pc" expression="execution(* ataspectj.ConcreteAspectTest.target())"/>
+        </concrete-aspect>
+    </aspects>
+</aspectj>
diff --git a/tests/java5/ataspectj/ataspectj/aop-concreteataspect.xml b/tests/java5/ataspectj/ataspectj/aop-concreteataspect.xml
new file mode 100644
index 000000000..9b38b3d64
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/aop-concreteataspect.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+<aspectj>
+    <weaver options="-XmessageHandlerClass:ataspectj.TestHelper -1.5"/>
+    <aspects>
+        <concrete-aspect name="ataspectj.Foo" extends="ataspectj.ConcreteAtAspectTest.ConcreteAtAspect">
+            <pointcut name="pc" expression="execution(* ataspectj.ConcreteAtAspectTest.target())"/>
+        </concrete-aspect>
+        <aspect name="ataspectj.ConcreteAtAspectTest.ConcreteAtAspectSub"/>
+    </aspects>
+</aspectj>
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
index 79e3dc307..d0a88a27e 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjLTWTests.java
@@ -94,4 +94,11 @@ public class AtAjLTWTests extends XMLBasedAjcTestCase {
         runTest("Compile time aspects declared to ltw weaver");
     }
 
+    public void testConcreteAtAspect() {
+        runTest("Concrete@Aspect");
+    }
+
+    public void testConcreteAspect() {
+        runTest("ConcreteAspect");
+    }
 }
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
index a316e6799..fd2b922f1 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/ltw.xml
@@ -131,4 +131,20 @@
         <ant file="ajc-ant.xml" target="ltw.oldAspectsDeclared" verbose="true"/>
     </ajc-test>
 
+    <ajc-test dir="java5/ataspectj" title="Concrete@Aspect">
+        <compile
+            files="ataspectj/ConcreteAtAspectTest.java,ataspectj/TestHelper.java"
+            options="-1.5 -Xdev:NoAtAspectJProcessing -XnoWeave"
+            />
+        <run class="ataspectj.ConcreteAtAspectTest" ltw="ataspectj/aop-concreteataspect.xml"/>
+    </ajc-test>
+
+    <ajc-test dir="java5/ataspectj" title="ConcreteAspect">
+        <compile
+            files="ataspectj/ConcreteAspectTest.aj,ataspectj/TestHelper.java"
+            options="-1.5 -Xdev:NoAtAspectJProcessing -XnoWeave"
+            />
+        <run class="ataspectj.ConcreteAspectTest" ltw="ataspectj/aop-concreteaspect.xml"/>
+    </ajc-test>
+
 </suite>
\ No newline at end of file
diff --git a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
index 1160ac791..f3846c53a 100644
--- a/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
+++ b/weaver/src/org/aspectj/weaver/bcel/AtAjAttributes.java
@@ -1051,57 +1051,73 @@ public class AtAjAttributes {
         Annotation pointcut = getAnnotation(runtimeAnnotations, AjcMemberMaker.POINTCUT_ANNOTATION);
         if (pointcut != null) {
             ElementNameValuePair pointcutExpr = getAnnotationElement(pointcut, VALUE);
-            if (pointcutExpr != null) {
 
-                // semantic check: the method must return void, or be "public static boolean" for if() support
-                if (!(Type.VOID.equals(struct.method.getReturnType())
-                      || (Type.BOOLEAN.equals(struct.method.getReturnType()) && struct.method.isStatic() && struct.method.isPublic()))) {
-                    reportWarning("Found @Pointcut on a method not returning 'void' or not 'public static boolean'", struct);
-                    ;//no need to stop
-                }
-                // semantic check: the method must not throw anything
-                if (struct.method.getExceptionTable() != null) {
-                    reportWarning("Found @Pointcut on a method throwing exception", struct);
-                    ;// no need to stop
-                }
-
-                // this/target/args binding
-                final IScope binding;
-                try {
-                    binding = new BindingScope(
-                            struct.enclosingType,
-                            struct.context,
-                            extractBindings(struct)
-                    );
-                } catch (UnreadableDebugInfoException e) {
-                    return;
-                }
+            // semantic check: the method must return void, or be "public static boolean" for if() support
+            if (!(Type.VOID.equals(struct.method.getReturnType())
+                  || (Type.BOOLEAN.equals(struct.method.getReturnType()) && struct.method.isStatic() && struct.method.isPublic()))) {
+                reportWarning("Found @Pointcut on a method not returning 'void' or not 'public static boolean'", struct);
+                ;//no need to stop
+            }
 
-                UnresolvedType[] argumentTypes = new UnresolvedType[struct.method.getArgumentTypes().length];
-                for (int i = 0; i < argumentTypes.length; i++) {
-                    argumentTypes[i] = UnresolvedType.forSignature(struct.method.getArgumentTypes()[i].getSignature());
-                }
+            // semantic check: the method must not throw anything
+            if (struct.method.getExceptionTable() != null) {
+                reportWarning("Found @Pointcut on a method throwing exception", struct);
+                ;// no need to stop
+            }
 
-                // use a LazyResolvedPointcutDefinition so that the pointcut is resolved lazily
-                // since for it to be resolved, we will need other pointcuts to be registered as well
-                Pointcut pc = parsePointcut(pointcutExpr.getValue().stringifyValue(), struct, true);
-                if (pc == null) return;//parse error
-                // do not resolve binding now but lazily
-                pc.setLocation(struct.context, -1, -1);//FIXME AVASM !! bMethod is null here..
-                struct.ajAttributes.add(
-                        new AjAttribute.PointcutDeclarationAttribute(
-                                new LazyResolvedPointcutDefinition(
-                                        struct.enclosingType,
-                                        struct.method.getModifiers(),
-                                        struct.method.getName(),
-                                        argumentTypes,
-                                        UnresolvedType.forSignature(struct.method.getReturnType().getSignature()),
-                                        pc,
-                                        binding
-                                )
-                        )
+            // this/target/args binding
+            final IScope binding;
+            try {
+                binding = new BindingScope(
+                        struct.enclosingType,
+                        struct.context,
+                        extractBindings(struct)
                 );
+            } catch (UnreadableDebugInfoException e) {
+                return;
+            }
+
+            UnresolvedType[] argumentTypes = new UnresolvedType[struct.method.getArgumentTypes().length];
+            for (int i = 0; i < argumentTypes.length; i++) {
+                argumentTypes[i] = UnresolvedType.forSignature(struct.method.getArgumentTypes()[i].getSignature());
+            }
+
+            Pointcut pc = null;
+            if (struct.method.isAbstract()) {
+                if ((pointcutExpr != null && isNullOrEmpty(pointcutExpr.getValue().stringifyValue()))
+                    || pointcutExpr == null) {
+                    // abstract pointcut
+                    // leave pc = null
+                } else {
+                    reportError("Found defined @Pointcut on an abstract method", struct);
+                    return;//stop
+                }
+            } else {
+                if (pointcutExpr != null) {
+                    // use a LazyResolvedPointcutDefinition so that the pointcut is resolved lazily
+                    // since for it to be resolved, we will need other pointcuts to be registered as well
+                    pc = parsePointcut(pointcutExpr.getValue().stringifyValue(), struct, true);
+                    if (pc == null) return;//parse error
+                    pc.setLocation(struct.context, -1, -1);//FIXME AVASM !! bMethod is null here..
+                } else {
+                    reportError("Found undefined @Pointcut on a non-abstract method", struct);
+                    return;
+                }
             }
+            // do not resolve binding now but lazily
+            struct.ajAttributes.add(
+                    new AjAttribute.PointcutDeclarationAttribute(
+                            new LazyResolvedPointcutDefinition(
+                                    struct.enclosingType,
+                                    struct.method.getModifiers(),
+                                    struct.method.getName(),
+                                    argumentTypes,
+                                    UnresolvedType.forSignature(struct.method.getReturnType().getSignature()),
+                                    pc,//can be null for abstract pointcut
+                                    binding
+                            )
+                    )
+            );
         }
     }
 
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java b/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java
index 242e8b76d..79427eb4c 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelPerClauseAspectAdder.java
@@ -85,6 +85,10 @@ public class BcelPerClauseAspectAdder extends BcelTypeMunger {
              return false;
          }
 
+        return forceMunge(gen);
+    }
+
+    public boolean forceMunge(LazyClassGen gen) {
         generatePerClauseMembers(gen);
 
         if (kind == PerClause.SINGLETON) {
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
index cf4c29927..6ab999ec3 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java
@@ -141,8 +141,9 @@ public class BcelWeaver implements IWeaver {
      * The type is resolved to support DOT for static inner classes as well as DOLLAR
      *
      * @param aspectName
+     * @return aspect
      */
-    public void addLibraryAspect(String aspectName) {
+    public ResolvedType addLibraryAspect(String aspectName) {
         // 1 - resolve as is
         ResolvedType type = world.resolve(UnresolvedType.forName(aspectName), true);
         if (type.equals(ResolvedType.MISSING)) {
@@ -167,8 +168,9 @@ public class BcelWeaver implements IWeaver {
             //TODO AV - happens to reach that a lot of time: for each type flagged reweavable X for each aspect in the weaverstate
             //=> mainly for nothing for LTW - pbly for something in incremental build...
 			xcutSet.addOrReplaceAspect(type);
-		} else {
-            // FIXME : Alex: better warning upon no such aspect from aop.xml
+            return type;
+        } else {
+            // FIXME AV - better warning upon no such aspect from aop.xml
 			throw new RuntimeException("Cannot register non aspect: " + type.getName() + " , " + aspectName);
 		}
     }
-- 
cgit v1.2.3