fix 83935 where Jp / Pjp is an arg and bound in a formal binding as well as another (or 2+) used as implicit bindings in @AJ

5 files changed, 223 insertions, 16 deletions

diff --git a/tests/java5/ataspectj/ataspectj/MultipleBindingTest.java b/tests/java5/ataspectj/ataspectj/MultipleBindingTest.java
new file mode 100644
index 000000000..366a127cb
--- /dev/null
+++ b/tests/java5/ataspectj/ataspectj/MultipleBindingTest.java
@@ -0,0 +1,167 @@
+/*******************************************************************************
+ * 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 org.aspectj.lang.JoinPoint;
+import org.aspectj.lang.ProceedingJoinPoint;
+import org.aspectj.lang.Signature;
+import org.aspectj.lang.reflect.SourceLocation;
+import org.aspectj.lang.annotation.Aspect;
+import org.aspectj.lang.annotation.Before;
+import org.aspectj.lang.annotation.Around;
+import org.aspectj.runtime.internal.AroundClosure;
+
+/**
+ * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
+ */
+public class MultipleBindingTest 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(MultipleBindingTest.class);
+    }
+
+    public void testMultipleJoinPoint() {
+        s_log = new StringBuffer();
+        Target.dump(new JoinPoint() {
+            public String toShortString() {
+                return "jpFromApp";
+            }
+
+            public String toLongString() {
+                return null;
+            }
+
+            public Object getThis() {
+                return null;
+            }
+
+            public Object getTarget() {
+                return null;
+            }
+
+            public Object[] getArgs() {
+                return new Object[0];
+            }
+
+            public Signature getSignature() {
+                return null;
+            }
+
+            public SourceLocation getSourceLocation() {
+                return null;
+            }
+
+            public String getKind() {
+                return null;
+            }
+
+            public StaticPart getStaticPart() {
+                return null;
+            }
+        });
+        assertEquals("jpFromApp execution(MultipleBindingTest.Target.dump(..)) execution(MultipleBindingTest.Target.dump(..)) jpFromApp ", s_log.toString());
+    }
+
+    public void testMultipleProceedingJoinPoint() {
+        s_log = new StringBuffer();
+        Target.dump2(new ProceedingJoinPoint() {
+            public void set$AroundClosure(AroundClosure arc) {
+
+            }
+
+            public Object proceed() throws Throwable {
+                return null;
+            }
+
+            public Object proceed(Object[] args) throws Throwable {
+                return null;
+            }
+
+            public String toShortString() {
+                return "pjpFromApp";
+            }
+
+            public String toLongString() {
+                return null;
+            }
+
+            public Object getThis() {
+                return null;
+            }
+
+            public Object getTarget() {
+                return null;
+            }
+
+            public Object[] getArgs() {
+                return new Object[0];
+            }
+
+            public Signature getSignature() {
+                return null;
+            }
+
+            public SourceLocation getSourceLocation() {
+                return null;
+            }
+
+            public String getKind() {
+                return null;
+            }
+
+            public StaticPart getStaticPart() {
+                return null;
+            }
+
+        });
+        assertEquals("pjpFromApp execution(MultipleBindingTest.Target.dump2(..)) execution(MultipleBindingTest.Target.dump2(..)) pjpFromApp ", s_log.toString());
+    }
+
+    static class Target {
+        static void dump(JoinPoint jp) {
+            log(jp.toShortString());
+        }
+        static void dump2(ProceedingJoinPoint pjp) {
+            log(pjp.toShortString());
+        }
+    }
+
+    @Aspect
+    public static class TestAspect {
+
+        @Before("execution(* ataspectj.MultipleBindingTest.Target.dump(..)) && args(ajp)")
+        public void before(JoinPoint ajp, JoinPoint jp, JoinPoint jpbis) {
+            log(ajp.toShortString());
+            log(jp.toShortString());
+            log(jpbis.toShortString());
+        }
+
+        @Around("execution(* ataspectj.MultipleBindingTest.Target.dump2(..)) && args(apjp)")
+        public Object around(ProceedingJoinPoint apjp, ProceedingJoinPoint pjp, ProceedingJoinPoint pjpbis) throws Throwable {
+            log(apjp.toShortString());
+            log(pjp.toShortString());
+            log(pjpbis.toShortString());
+            return pjp.proceed();
+        }
+    }
+
+}
diff --git a/tests/java5/ataspectj/ataspectj/SingletonAspectBindingsTest.java b/tests/java5/ataspectj/ataspectj/SingletonAspectBindingsTest.java
index e59821cb8..d94d2af0a 100644
--- a/tests/java5/ataspectj/ataspectj/SingletonAspectBindingsTest.java
+++ b/tests/java5/ataspectj/ataspectj/SingletonAspectBindingsTest.java
@@ -134,15 +134,15 @@ public class SingletonAspectBindingsTest extends TestCase {
     }
 
 //    public void testHe() throws Throwable {
-//        //Allow to look inn file based on advises/advised-by offset numbers   
-//        File f = new File("../tests/java5/ataspectj/ataspectj/SingletonAspectBindingsTest2.aj");
+//        //Allow to look inn file based on advises/advised-by offset numbers
+//        File f = new File("../tests/java5/ataspectj/ataspectj/SingletonAspectBindingsTest.java");
 //        FileReader r = new FileReader(f);
 //        int i = 0;
-//        for (i = 0; i < 2800; i++) {
+//        for (i = 0; i < 3700; i++) {
 //            r.read();
 //        }
-//        for (;i < 2900; i++) {
-//            if (i == 2817) System.out.print("X");
+//        for (;i < 3800; i++) {
+//            if (i==3721 || i == 3742 || i == 3777) System.out.print("X");
 //            System.out.print((char)r.read());
 //        }
 //        System.out.print("|DONE");
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java
index 7cec3892b..8d3813021 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/AtAjSyntaxTests.java
@@ -41,11 +41,11 @@ public class AtAjSyntaxTests extends XMLBasedAjcTestCase {
 
     public void testSingletonAspectBindings() {
         //Note AV: uncomment setReporting to get it in modules/tests folder
-        org.aspectj.asm.AsmManager.setReporting("debug.txt",true,true,true,true);
+        //org.aspectj.asm.AsmManager.setReporting("debug.txt",true,true,true,true);
         runTest("singletonAspectBindings");
         // same stuff with AJ
         //org.aspectj.asm.AsmManager.setReporting("debug-aj.txt",true,true,true,true);
-        //runTest("singletonAspectBindings2");
+        runTest("singletonAspectBindings2");
 
     }
 
@@ -112,4 +112,8 @@ public class AtAjSyntaxTests extends XMLBasedAjcTestCase {
     public void testIfPointcut2() {
         runTest("IfPointcut2Test");
     }
+
+    public void testMultipleBinding() {
+        runTest("MultipleBinding");
+    }
 }
\ No newline at end of file
diff --git a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml
index f4e209b07..212dbd4f9 100644
--- a/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ataspectj/syntax.xml
@@ -131,10 +131,17 @@
     </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"/>
+        <run class="ataspectj.PerClauseInheritanceTest"/>
         <compile files="ataspectj/PerClauseInheritanceTest.java,ataspectj/TestHelper.java" options="-1.5 -XnoInline -Xdev:NoAtAspectJProcessing"/>
         <run class="ataspectj.PerClauseInheritanceTest"/>
     </ajc-test>
 
+    <ajc-test dir="java5/ataspectj" title="MultipleBinding">
+        <compile files="ataspectj/MultipleBindingTest.java,ataspectj/TestHelper.java" options="-1.5 -Xdev:NoAtAspectJProcessing -XnoInline"/>
+        <run class="ataspectj.MultipleBindingTest"/>
+        <compile files="ataspectj/MultipleBindingTest.java,ataspectj/TestHelper.java" options="-1.5 -Xdev:NoAtAspectJProcessing"/>
+        <run class="ataspectj.MultipleBindingTest"/>
+    </ajc-test>
+
 </suite>
\ No newline at end of file
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
index c242f729b..9cea221f5 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
@@ -346,39 +346,68 @@ public class BcelAdvice extends Advice {
 
 
 		if (exposedState.getAspectInstance() != null) {
-			il.append(
-				BcelRenderer.renderExpr(fact, world, exposedState.getAspectInstance()));
+			il.append(BcelRenderer.renderExpr(fact, world, exposedState.getAspectInstance()));
 		}
+        final boolean isAnnotationStyleAspect = getConcreteAspect()!=null && getConcreteAspect().isAnnotationStyleAspect();
+        boolean previousIsClosure = false;
         for (int i = 0, len = exposedState.size(); i < len; i++) {
         	if (exposedState.isErroneousVar(i)) continue; // Erroneous vars have already had error msgs reported!
             BcelVar v = (BcelVar) exposedState.get(i);
 
             if (v == null) {
                 // if not @AJ aspect, go on with the regular binding handling
-            	if (getConcreteAspect()==null || !getConcreteAspect().isAnnotationStyleAspect()) {
+            	if (!isAnnotationStyleAspect) {
             		;
             	} else {
                     // ATAJ: for @AJ aspects, handle implicit binding of xxJoinPoint
-	                if (getKind() == AdviceKind.Around) {
-	                    il.append(closureInstantiation);
+	                //if (getKind() == AdviceKind.Around) {
+                    //    previousIsClosure = true;
+	                //    il.append(closureInstantiation);
+                    if ("Lorg/aspectj/lang/ProceedingJoinPoint;".equals(getSignature().getParameterTypes()[i].getSignature())) {
+                        //make sure we are in an around, since we deal with the closure, not the arg here
+                        if (getKind() != AdviceKind.Around) {
+                            previousIsClosure = false;
+                            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 {
+                            if (previousIsClosure) {
+                                il.append(InstructionConstants.DUP);
+                            } else {
+                                previousIsClosure = true;
+                                il.append(closureInstantiation.copy());
+                            }
+                        }
 	                } else if ("Lorg/aspectj/lang/JoinPoint$StaticPart;".equals(getSignature().getParameterTypes()[i].getSignature())) {
+                        previousIsClosure = false;
 	                    if ((getExtraParameterFlags() & ThisJoinPointStaticPart) != 0) {
 	                        shadow.getThisJoinPointStaticPartBcelVar().appendLoad(il, fact);
 	                    }
 	                } else if ("Lorg/aspectj/lang/JoinPoint;".equals(getSignature().getParameterTypes()[i].getSignature())) {
+                        previousIsClosure = false;
 	                    if ((getExtraParameterFlags() & ThisJoinPoint) != 0) {
 	                        il.append(shadow.loadThisJoinPoint());
 	                    }
 	                } else if ("Lorg/aspectj/lang/JoinPoint$EnclosingStaticPart;".equals(getSignature().getParameterTypes()[i].getSignature())) {
+                        previousIsClosure = false;
 	                    if ((getExtraParameterFlags() & ThisEnclosingJoinPointStaticPart) != 0) {
 	                        shadow.getThisEnclosingJoinPointStaticPartBcelVar().appendLoad(il, fact);
 	                    }
 	                } else if (hasExtraParameter()) {
+                        previousIsClosure = false;
 	                    extraVar.appendLoadAndConvert(
 	                        il,
 	                        fact,
 	                        getExtraParameterType().resolve(world));
 	                } else {
+                        previousIsClosure = false;
                         getConcreteAspect().getWorld().getMessageHandler().handleMessage(
                                 new Message(
                                         "use of ProceedingJoinPoint is allowed only on around advice ("
@@ -397,10 +426,10 @@ public class BcelAdvice extends Advice {
             }
         }
 
-        
+
         // ATAJ: for code style aspect, handles the extraFlag as usual ie not
         // in the middle of the formal bindings but at the end, in a rock solid ordering
-        if (getConcreteAspect()==null || !getConcreteAspect().isAnnotationStyleAspect()) {
+        if (!isAnnotationStyleAspect) {
             if (getKind() == AdviceKind.Around) {
                 il.append(closureInstantiation);
             } else if (hasExtraParameter()) {