From 639b4fd0a436e19726dc9f2a47dc66a726794b3d Mon Sep 17 00:00:00 2001 From: avasseur Date: Mon, 11 Jul 2005 15:05:44 +0000 Subject: [PATCH] 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 --- .../ataspectj/MultipleBindingTest.java | 167 ++++++++++++++++++ .../SingletonAspectBindingsTest.java | 10 +- .../ajc150/ataspectj/AtAjSyntaxTests.java | 8 +- .../systemtest/ajc150/ataspectj/syntax.xml | 11 +- .../org/aspectj/weaver/bcel/BcelAdvice.java | 43 ++++- 5 files changed, 223 insertions(+), 16 deletions(-) create mode 100644 tests/java5/ataspectj/ataspectj/MultipleBindingTest.java 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 Alexandre Vasseur + */ +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 @@ - - + + + + + + + + + \ 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()) { -- 2.39.5