From a57ec8d9cff0ca35ce9efb59c79a5f1ec90c13cb Mon Sep 17 00:00:00 2001 From: jhugunin Date: Wed, 12 Mar 2003 19:51:43 +0000 Subject: [PATCH] fix and tests for checked exception on advice being checked at woven join points --- tests/ajcTests.xml | 19 +++++ tests/ajcTestsFailing.xml | 12 ---- tests/jimTests.xml | 15 ++-- tests/new/AdviceThrowsCf.java | 13 ++++ weaver/src/org/aspectj/weaver/Checker.java | 5 ++ weaver/src/org/aspectj/weaver/Shadow.java | 48 +++++++++---- .../src/org/aspectj/weaver/ShadowMunger.java | 8 +++ weaver/src/org/aspectj/weaver/TypeX.java | 2 + .../org/aspectj/weaver/bcel/BcelAdvice.java | 67 +++++++++++++----- weaver/testdata/dummyAspect.jar | Bin 591 -> 591 bytes weaver/testdata/megatrace.jar | Bin 3562 -> 3562 bytes weaver/testdata/megatrace0easy.jar | Bin 2902 -> 2902 bytes weaver/testdata/megatraceNoweave.jar | Bin 2717 -> 2717 bytes weaver/testdata/tracing.jar | Bin 2298 -> 2298 bytes 14 files changed, 139 insertions(+), 50 deletions(-) diff --git a/tests/ajcTests.xml b/tests/ajcTests.xml index 31f5a7607..b8486e3b5 100644 --- a/tests/ajcTests.xml +++ b/tests/ajcTests.xml @@ -5666,4 +5666,23 @@ + + + + + + + + + + + + + + + + + diff --git a/tests/ajcTestsFailing.xml b/tests/ajcTestsFailing.xml index 5964c4aee..578f0aa75 100644 --- a/tests/ajcTestsFailing.xml +++ b/tests/ajcTestsFailing.xml @@ -2,18 +2,6 @@ - - - - - - - - - - diff --git a/tests/jimTests.xml b/tests/jimTests.xml index e55e54ff7..f4adb0f97 100644 --- a/tests/jimTests.xml +++ b/tests/jimTests.xml @@ -1,14 +1,15 @@ - - - - + + + \ No newline at end of file diff --git a/tests/new/AdviceThrowsCf.java b/tests/new/AdviceThrowsCf.java index f8564d979..279bd2d55 100644 --- a/tests/new/AdviceThrowsCf.java +++ b/tests/new/AdviceThrowsCf.java @@ -77,4 +77,17 @@ aspect A { } before() throws CheckedExc: staticinitialization(C) { //ERR: can't throw } + + void around() throws CheckedExc: canThrowChecked() { + proceed(); + } + + void around() throws CheckedExc: canThrowUnchecked() { // ERR: can't throw + proceed(); + } + + void around() throws UncheckedExc: canThrowUnchecked() || set(int C.x) { + proceed(); + } + } diff --git a/weaver/src/org/aspectj/weaver/Checker.java b/weaver/src/org/aspectj/weaver/Checker.java index ab4541de3..69285c37f 100644 --- a/weaver/src/org/aspectj/weaver/Checker.java +++ b/weaver/src/org/aspectj/weaver/Checker.java @@ -13,6 +13,9 @@ package org.aspectj.weaver; +import java.util.Collection; +import java.util.Collections; + import org.aspectj.bridge.IMessage; import org.aspectj.bridge.Message; import org.aspectj.weaver.patterns.DeclareErrorOrWarning; @@ -58,5 +61,7 @@ public class Checker extends ShadowMunger { public int compareTo(Object other) { return 0; } + + public Collection getThrownExceptions() { return Collections.EMPTY_LIST; } } diff --git a/weaver/src/org/aspectj/weaver/Shadow.java b/weaver/src/org/aspectj/weaver/Shadow.java index 9a3f79082..fb3fddb20 100644 --- a/weaver/src/org/aspectj/weaver/Shadow.java +++ b/weaver/src/org/aspectj/weaver/Shadow.java @@ -268,8 +268,42 @@ public abstract class Shadow { } } + protected boolean checkMunger(ShadowMunger munger) { + for (Iterator i = munger.getThrownExceptions().iterator(); i.hasNext(); ) { + if (!checkCanThrow(munger, (ResolvedTypeX)i.next() )) return false; + } + return true; + } + + protected boolean checkCanThrow(ShadowMunger munger, ResolvedTypeX resolvedTypeX) { + if (getKind() == ExceptionHandler) { + //XXX much too lenient rules here, need to walk up exception handlers + return true; + } + + if (!isDeclaredException(resolvedTypeX, getSignature())) { + getIWorld().showMessage(IMessage.ERROR, "can't throw checked exception \'" + resolvedTypeX + + "\' at this join point \'" + this +"\'", // from advice in \'" + munger. + "\'", + getSourceLocation(), munger.getSourceLocation()); + } + + return true; + } + + private boolean isDeclaredException( + ResolvedTypeX resolvedTypeX, + Member member) + { + ResolvedTypeX[] excs = getIWorld().resolve(member.getExceptions(getIWorld())); + for (int i=0, len=excs.length; i < len; i++) { + if (excs[i].isAssignableFrom(resolvedTypeX)) return true; + } + return false; + } + + public void addMunger(ShadowMunger munger) { - this.mungers.add(munger); + if (checkMunger(munger)) this.mungers.add(munger); } public final void implement() { @@ -323,16 +357,4 @@ public abstract class Shadow { public String toString() { return getKind() + "(" + getSignature() + ")"; // + getSourceLines(); } - - - - - - - - - // ---- type access methods - - - } diff --git a/weaver/src/org/aspectj/weaver/ShadowMunger.java b/weaver/src/org/aspectj/weaver/ShadowMunger.java index f6788f6da..a8cf1dd80 100644 --- a/weaver/src/org/aspectj/weaver/ShadowMunger.java +++ b/weaver/src/org/aspectj/weaver/ShadowMunger.java @@ -13,6 +13,8 @@ package org.aspectj.weaver; +import java.util.Collection; + import org.aspectj.bridge.ISourceLocation; import org.aspectj.util.PartialOrder; import org.aspectj.weaver.patterns.PerClause; @@ -88,4 +90,10 @@ public abstract class ShadowMunger implements PartialOrder.PartialComparable, IH return pointcut; } + + /** + * @return a Collection of ResolvedTypeX for all checked exceptions that + * might be thrown by this munger + */ + public abstract Collection getThrownExceptions(); } diff --git a/weaver/src/org/aspectj/weaver/TypeX.java b/weaver/src/org/aspectj/weaver/TypeX.java index f5bab0db3..313e2b81a 100644 --- a/weaver/src/org/aspectj/weaver/TypeX.java +++ b/weaver/src/org/aspectj/weaver/TypeX.java @@ -541,6 +541,8 @@ public class TypeX { public static final TypeX CLONEABLE = forSignature("Ljava/lang/Cloneable;"); public static final TypeX SERIALIZABLE = forSignature("Ljava/io/Serializable;"); public static final TypeX THROWABLE = forSignature("Ljava/lang/Throwable;"); + public static final TypeX RUNTIME_EXCEPTION = forSignature("Ljava/lang/RuntimeException;"); + public static final TypeX ERROR = forSignature("Ljava/lang/Error;"); // ---- helpers diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java index 8aa4c37a2..a62bbd927 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java @@ -13,25 +13,15 @@ package org.aspectj.weaver.bcel; -import org.apache.bcel.generic.InstructionFactory; -import org.apache.bcel.generic.InstructionHandle; -import org.apache.bcel.generic.InstructionList; -import org.aspectj.weaver.Advice; -import org.aspectj.weaver.AdviceKind; -import org.aspectj.weaver.AjAttribute; -import org.aspectj.weaver.BCException; -import org.aspectj.weaver.ISourceContext; -import org.aspectj.weaver.Member; -import org.aspectj.weaver.ResolvedTypeX; -import org.aspectj.weaver.Shadow; -import org.aspectj.weaver.TypeX; -import org.aspectj.weaver.WeaverStateKind; -import org.aspectj.weaver.World; +import java.util.*; +import java.util.Collection; +import java.util.Collections; + +import org.apache.bcel.generic.*; +import org.aspectj.weaver.*; import org.aspectj.weaver.ast.Literal; import org.aspectj.weaver.ast.Test; -import org.aspectj.weaver.patterns.ExactTypePattern; -import org.aspectj.weaver.patterns.ExposedState; -import org.aspectj.weaver.patterns.Pointcut; +import org.aspectj.weaver.patterns.*; /** * Advice implemented for bcel. @@ -53,13 +43,14 @@ public class BcelAdvice extends Advice { this.concreteAspect = concreteAspect; } - // !!! only used for testing + // !!! must only be used for testing public BcelAdvice(AdviceKind kind, Pointcut pointcut, Member signature, int extraArgumentFlags, int start, int end, ISourceContext sourceContext, ResolvedTypeX concreteAspect) { this(new AjAttribute.AdviceAttribute(kind, pointcut, extraArgumentFlags, start, end, sourceContext), pointcut, signature, concreteAspect); + thrownExceptions = Collections.EMPTY_LIST; //!!! interaction with unit tests } // ---- implementations of ShadowMunger's methods @@ -147,6 +138,46 @@ public class BcelAdvice extends Advice { } // ---- implementations + + private Collection collectCheckedExceptions(TypeX[] excs) { + if (excs == null || excs.length == 0) return Collections.EMPTY_LIST; + + Collection ret = new ArrayList(); + World world = concreteAspect.getWorld(); + ResolvedTypeX runtimeException = world.resolve(TypeX.RUNTIME_EXCEPTION); + ResolvedTypeX error = world.resolve(TypeX.ERROR); + + for (int i=0, len=excs.length; i < len; i++) { + ResolvedTypeX t = world.resolve(excs[i]); + if (!(runtimeException.isAssignableFrom(t) || error.isAssignableFrom(t))) { + ret.add(t); + } + } + + return ret; + } + + private Collection thrownExceptions = null; + public Collection getThrownExceptions() { + if (thrownExceptions == null) { + //??? can we really lump in Around here, how does this interact with Throwable + if (concreteAspect != null && concreteAspect.getWorld() != null && // null tests for test harness + (getKind().isAfter() || getKind() == AdviceKind.Before || getKind() == AdviceKind.Around)) + { + World world = concreteAspect.getWorld(); + ResolvedMember m = world.resolve(signature); + if (m == null) { + thrownExceptions = Collections.EMPTY_LIST; + } else { + thrownExceptions = collectCheckedExceptions(m.getExceptions()); + } + } else { + thrownExceptions = Collections.EMPTY_LIST; + } + } + return thrownExceptions; + } + // only call me after prepare has been called public boolean hasDynamicTests() { diff --git a/weaver/testdata/dummyAspect.jar b/weaver/testdata/dummyAspect.jar index fd37338f440f2102bff86a3c2938a2df5923d815..a1972891f64a49221d1b0af9d13b00f812a3a7d8 100644 GIT binary patch delta 30 icmX@la-M}Zz?+#xgnJ delta 30 jcmX@la-M}Zz?+#xgnGPwZ&dQ=Ey diff --git a/weaver/testdata/megatrace.jar b/weaver/testdata/megatrace.jar index 13fde4cc1e561e88afa12ce3849d8b0227fda05c..a6c410d7b56b603535d4c61b75880ef4acca26b6 100644 GIT binary patch delta 67 zcmaDQ{YsiQz?+#xgnm7Up#T(_v70 HCT}1Ba(oj2 delta 67 zcmaDQ{YsiQz?+#xgn