diff options
7 files changed, 93 insertions, 4 deletions
diff --git a/tests/bugs/oxford/PR65319.java b/tests/bugs/oxford/PR65319.java new file mode 100644 index 000000000..2a00b9acd --- /dev/null +++ b/tests/bugs/oxford/PR65319.java @@ -0,0 +1,30 @@ +class Test +{ + public static void main(String args[]) { + new Test().method(); + } + public void method() { + new Test2().method2(); + } + + public void method3() { + new Test2().method3(new Test()); + } + + public void method4(Test t) { + new Test().method4(new Test()); + } +} +class Test2 { + public void method2() {} + public void method3(Test t) {} +} +aspect Plain { + before(Test x): call(void *.* (..)) && (target(x) || this(x)) {} + + before(Test x): call(void *.* (..)) && (this(x) || target(x)){} + + before(Test x): call(void *.*(..)) && (this(x) || args(x)) {} + + before(Test x): call(void *.*(..)) && (args(x) || target(x)) {} +}
\ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java b/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java index 5a3c6d496..e32a91e37 100644 --- a/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java +++ b/tests/src/org/aspectj/systemtest/ajc121/Ajc121Tests.java @@ -10,7 +10,9 @@ package org.aspectj.systemtest.ajc121; import java.io.File; + import junit.framework.Test; + import org.aspectj.testing.XMLBasedAjcTestCase; public class Ajc121Tests extends org.aspectj.testing.XMLBasedAjcTestCase { @@ -139,5 +141,10 @@ public class Ajc121Tests extends org.aspectj.testing.XMLBasedAjcTestCase { public void test025_proceedInAround3() { runTest("proceed used as method name in around advice (3)"); } + + public void test026_bindingThisAndTargetToTheSameFormal() { + runTest("ajc crashes when compiling the following program (binding this() and target())"); + } + } diff --git a/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml b/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml index 37ac4da2d..579adf58d 100644 --- a/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml +++ b/tests/src/org/aspectj/systemtest/ajc121/ajc121-tests.xml @@ -201,4 +201,18 @@ title="proceed used as method name in around advice (3)"> <compile files="Proceeding3.aj"/> <run class="Proceeding3"/> - </ajc-test>
\ No newline at end of file + </ajc-test> + + <ajc-test dir="bugs/oxford" pr="65319" + title="ajc crashes when compiling the following program (binding this() and target())"> + <compile files="PR65319.java"> + <message kind="error" line="7" text="Cannot use target() to match at this"/> + <message kind="error" line="7" text="Cannot use this() to match at this"/> + <message kind="error" line="11" text="Cannot use target() to match at this"/> + <message kind="error" line="11" text="Cannot use this() to match at this"/> + <message kind="error" line="11" text="Ambiguous binding of type Test"/> + <message kind="error" line="15" text="Cannot use target() to match at this"/> + <message kind="error" line="15" text="Cannot use this() to match at this"/> + <message kind="error" line="15" text="Ambiguous binding of type Test"/> + </compile> + </ajc-test> diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java index e35128a62..310e9466a 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java @@ -291,6 +291,7 @@ public class BcelAdvice extends Advice { BcelRenderer.renderExpr(fact, world, exposedState.getAspectInstance())); } 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) continue; TypeX desiredTy = getSignature().getParameterTypes()[i]; diff --git a/weaver/src/org/aspectj/weaver/patterns/ArgsPointcut.java b/weaver/src/org/aspectj/weaver/patterns/ArgsPointcut.java index 6cbe6ce56..6b6541758 100644 --- a/weaver/src/org/aspectj/weaver/patterns/ArgsPointcut.java +++ b/weaver/src/org/aspectj/weaver/patterns/ArgsPointcut.java @@ -173,9 +173,11 @@ public class ArgsPointcut extends NameBindingPointcut { ISourceLocation isl = getSourceLocation(); Message errorMessage = new Message( "Ambiguous binding of type "+type.getExactType().toString()+ - " using args(..) at this line. Use one args(..) per matched join point,"+"" +
" see secondary source location for location of extraneous args(..)", + " using args(..) at this line - formal is already bound"+ + ". See secondary source location for location of args(..)", shadow.getSourceLocation(),true,new ISourceLocation[]{getSourceLocation()}); shadow.getIWorld().getMessageHandler().handleMessage(errorMessage); + state.setErroneousVar(btp.getFormalIndex()); } } ret = Test.makeAnd(ret, diff --git a/weaver/src/org/aspectj/weaver/patterns/ExposedState.java b/weaver/src/org/aspectj/weaver/patterns/ExposedState.java index 063b5608e..f092ad54d 100644 --- a/weaver/src/org/aspectj/weaver/patterns/ExposedState.java +++ b/weaver/src/org/aspectj/weaver/patterns/ExposedState.java @@ -21,11 +21,13 @@ import org.aspectj.weaver.ast.Var; public class ExposedState { public Var[] vars; + private boolean[] erroneousVars; private Expr aspectInstance; public ExposedState(int size) { super(); vars = new Var[size]; + erroneousVars = new boolean[size]; } public ExposedState(Member signature) { @@ -35,7 +37,11 @@ public class ExposedState { public void set(int i, Var var) { //XXX add sanity checks - // Check (1) added to call of set(), verifies we aren't binding twice to the same formal + // Some checks added in ArgsPointcut and ThisOrTargetPointcut +// if (vars[i]!=null) { +// if (!var.getType().equals(vars[i].getType())) +// throw new RuntimeException("Shouldn't allow a slot to change type! Currently="+var.getType()+" New="+vars[i].getType()); +// } vars[i] = var; } public Var get(int i) { @@ -56,4 +62,14 @@ public class ExposedState { public String toString() { return "ExposedState(" + Arrays.asList(vars) + ", " + aspectInstance + ")"; } + + // Set to true if we have reported an error message against it, + // prevents us blowing up in later code gen. + public void setErroneousVar(int formalIndex) { + erroneousVars[formalIndex]=true; + } + + public boolean isErroneousVar(int formalIndex) { + return erroneousVars[formalIndex]; + } } diff --git a/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetPointcut.java b/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetPointcut.java index 37d8ac68c..31fe527e3 100644 --- a/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetPointcut.java +++ b/weaver/src/org/aspectj/weaver/patterns/ThisOrTargetPointcut.java @@ -18,6 +18,8 @@ import java.io.DataOutputStream; import java.io.IOException; import org.aspectj.bridge.IMessage; +import org.aspectj.bridge.ISourceLocation; +import org.aspectj.bridge.Message; import org.aspectj.lang.JoinPoint; import org.aspectj.util.FuzzyBoolean; import org.aspectj.weaver.ISourceContext; @@ -123,7 +125,24 @@ public class ThisOrTargetPointcut extends NameBindingPointcut { if (type == TypePattern.ANY) return Literal.TRUE; - Var var = isThis ? shadow.getThisVar() : shadow.getTargetVar(); + Var var = isThis ? shadow.getThisVar() : shadow.getTargetVar(); + + if (type instanceof BindingTypePattern) { + BindingTypePattern btp = (BindingTypePattern)type; + // Check if we have already bound something to this formal + if (state.get(btp.getFormalIndex())!=null) { + ISourceLocation pcdSloc = getSourceLocation(); + ISourceLocation shadowSloc = shadow.getSourceLocation(); + Message errorMessage = new Message( + "Cannot use "+(isThis?"this()":"target()")+" to match at this location and bind a formal to type '"+var.getType()+ + "' - the formal is already bound to type '"+state.get(btp.getFormalIndex()).getType()+"'"+ + ". The secondary source location points to the problematic "+(isThis?"this()":"target()")+".", + shadowSloc,true,new ISourceLocation[]{pcdSloc}); + shadow.getIWorld().getMessageHandler().handleMessage(errorMessage); + state.setErroneousVar(btp.getFormalIndex()); + //return null; + } + } return exposeStateForVar(var, type, state, shadow.getIWorld()); } |