From: aclement Date: Tue, 30 Nov 2004 17:43:50 +0000 (+0000) Subject: Part of 72766: Bridge methods - we now do the right thing if we see one (i.e. we... X-Git-Tag: Root_AspectJ5_Development~231 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=4d1c2948a2f18012cd49dbe8e3a32b1a863d4d45;p=aspectj.git Part of 72766: Bridge methods - we now do the right thing if we see one (i.e. we ignore it as a source of join points) --- diff --git a/tests/java5/bridgeMethods/AspectX.aj b/tests/java5/bridgeMethods/AspectX.aj new file mode 100644 index 000000000..fb4827023 --- /dev/null +++ b/tests/java5/bridgeMethods/AspectX.aj @@ -0,0 +1,33 @@ +import java.util.*; + +public aspect AspectX { + + static Set matchedJps = new HashSet(); + + before(): call(* compareTo(..)) { + matchedJps.add(new String("call() matched on "+thisJoinPoint.toString())); + } + + before(): execution(* compareTo(..)) { + matchedJps.add(new String("execution() matched on "+thisJoinPoint.toString())); + } + + public static void main(String []argv) { + Number n1 = new Number(5); + Number n2 = new Number(7); + n1.compareTo(n2); + n1.compareTo("abc"); // A Java5 compiler would *not* allow this, a call to a bridge method: error should be: + /** + AspectX.java:19: compareTo(Number) in Number cannot be applied to (java.lang.String) + n1.compareTo("abc"); + ^ + 1 error + */ + + Iterator i = matchedJps.iterator(); + while (i.hasNext()) { + String s = (String)i.next(); + System.err.println(s); + } + } +} diff --git a/tests/java5/bridgeMethods/Comparable.java b/tests/java5/bridgeMethods/Comparable.java new file mode 100644 index 000000000..0236945e2 --- /dev/null +++ b/tests/java5/bridgeMethods/Comparable.java @@ -0,0 +1,3 @@ +interface Comparable { + public int compareTo(A that); +} diff --git a/tests/java5/bridgeMethods/Number.java b/tests/java5/bridgeMethods/Number.java new file mode 100644 index 000000000..1a866ec19 --- /dev/null +++ b/tests/java5/bridgeMethods/Number.java @@ -0,0 +1,6 @@ +public class Number implements Comparable { + private int i; + public Number(int i) { this.i = i; } + public int getValue() { return i;} + public int compareTo(Number that) { return this.getValue() - that.getValue(); } +} diff --git a/tests/java5/bridgeMethods/build.xml b/tests/java5/bridgeMethods/build.xml new file mode 100644 index 000000000..7170813dc --- /dev/null +++ b/tests/java5/bridgeMethods/build.xml @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/tests/java5/bridgeMethods/testcode.jar b/tests/java5/bridgeMethods/testcode.jar new file mode 100644 index 000000000..f543454b0 Binary files /dev/null and b/tests/java5/bridgeMethods/testcode.jar differ diff --git a/tests/src/org/aspectj/systemtest/AllTests.java b/tests/src/org/aspectj/systemtest/AllTests.java index ec5c1d467..961d99693 100644 --- a/tests/src/org/aspectj/systemtest/AllTests.java +++ b/tests/src/org/aspectj/systemtest/AllTests.java @@ -6,10 +6,14 @@ */ package org.aspectj.systemtest; +import junit.framework.Test; +import junit.framework.TestSuite; + import org.aspectj.systemtest.ajc10x.Ajc10xTests; import org.aspectj.systemtest.ajc11.Ajc11Tests; import org.aspectj.systemtest.ajc120.Ajc120Tests; import org.aspectj.systemtest.ajc121.Ajc121Tests; +import org.aspectj.systemtest.ajc150.AllTestsJava5_binaryWeaving; import org.aspectj.systemtest.aspectpath.AspectPathTests; import org.aspectj.systemtest.base.BaseTests; import org.aspectj.systemtest.design.DesignTests; @@ -21,9 +25,6 @@ import org.aspectj.systemtest.pre10x.AjcPre10xTests; import org.aspectj.systemtest.serialVerUID.SUIDTests; import org.aspectj.systemtest.xlint.XLintTests; -import junit.framework.Test; -import junit.framework.TestSuite; - /** * @author colyer * @@ -35,6 +36,7 @@ public class AllTests { public static Test suite() { TestSuite suite = new TestSuite("AspectJ System Test Suite - JDK 1.3"); //$JUnit-BEGIN$ + suite.addTest(AllTestsJava5_binaryWeaving.suite()); suite.addTest(Ajc121Tests.suite()); suite.addTest(Ajc120Tests.suite()); suite.addTest(Ajc11Tests.suite()); diff --git a/tests/src/org/aspectj/systemtest/ajc150/AccBridgeMethods.java b/tests/src/org/aspectj/systemtest/ajc150/AccBridgeMethods.java new file mode 100644 index 000000000..f364528d9 --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc150/AccBridgeMethods.java @@ -0,0 +1,86 @@ +/******************************************************************************* + * Copyright (c) 2004 IBM + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Common Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/cpl-v10.html + * + * Contributors: + * Andy Clement - initial API and implementation + *******************************************************************************/ +package org.aspectj.systemtest.ajc150; + +import java.io.File; +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; + +import junit.framework.Test; + +import org.aspectj.bridge.IMessage; +import org.aspectj.testing.XMLBasedAjcTestCase; +import org.aspectj.tools.ajc.CompilationResult; + + +/** + * These tests check binary weaving of code compiled with the 1.5 compiler. If you need to rebuild + * the class files then you will have to run tests/java5/bridgeMethods/build.xml. + * + *

Bridge methods are generated when a type extends or implements a parameterized class or interface and + * type erasure changes the signature of any inherited method. + * + *

They impact AspectJ in two ways: + *

    + *
  1. They exist as a method execution join point, and their 'body' exists as a set of new join points + * (although their body is normally coded simply to delegate to the method they are bridging too). + *
  2. They create a potential call join point where a call can be made to the bridge method. + *
+ * + *

The principal things we have to do are avoid weaving their body and ignore their existence + * as a method execution join point. Their existence as a potential target for a call join point are + * more complicated. Although they exist in the code, a 1.5 compiler will prevent a call to them with + * an error like this: + * + * M.java:3: compareTo(Number) in Number cannot be applied to (java.lang.String) + * new Number(5).compareTo("abc"); + * + * Our problem is that a Java 1.4 or earlier compiler will allow you to write calls to this bridge method + * and it will let them through. + */ +public class AccBridgeMethods extends org.aspectj.testing.XMLBasedAjcTestCase { + + public static Test suite() { + return XMLBasedAjcTestCase.loadSuite(AccBridgeMethods.class); + } + + protected File getSpecFile() { + return new File("../tests/src/org/aspectj/systemtest/ajc150/ajc150.xml"); + } + + + /** + * AspectX attempts to weave call and execution of the method for which a 'bridge method' is also created. + * If the test works then only two weaving messages come out. If it fails then usually 4 messages come out + * and we have incorrectly woven the bridge method (the 3rd message is execution of the bridge method and + * the 4th message is the call within the bridge method to the real method). + */ + public void test001_bridgeMethodIgnored() { + runTest("Ignore bridge methods"); + List weaveMessages = getWeaveMessages(ajc.getLastCompilationResult(),false); + assertTrue("Should only be two weave messages",weaveMessages.size()==2); + } + + private List getWeaveMessages(CompilationResult cr,boolean printThem) { + List weaveMessages = new ArrayList(); + List infoMessages = cr.getInfoMessages(); + for (Iterator iter = infoMessages.iterator(); iter.hasNext();) { + IMessage element = (IMessage) iter.next(); + if (element.getKind()==IMessage.WEAVEINFO) { + weaveMessages.add(element); + if (printThem) System.err.println(element); + } + } + return weaveMessages; + } + +} \ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java new file mode 100644 index 000000000..e79edf0ba --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc150/Ajc150Tests.java @@ -0,0 +1,32 @@ +/******************************************************************************* + * Copyright (c) 2004 IBM + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Common Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/cpl-v10.html + * + * Contributors: + * Andy Clement - initial API and implementation + *******************************************************************************/ +package org.aspectj.systemtest.ajc150; + +import java.io.File; + +import junit.framework.Test; + +import org.aspectj.testing.XMLBasedAjcTestCase; + +public class Ajc150Tests extends org.aspectj.testing.XMLBasedAjcTestCase { + + public static Test suite() { + return XMLBasedAjcTestCase.loadSuite(Ajc150Tests.class); + } + + protected File getSpecFile() { + return new File("../tests/src/org/aspectj/systemtest/ajc150/ajc150.xml"); + } + + public void test() { + // placeholder for the first test... + } +} \ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/ajc150/AllTestsJava5_binaryWeaving.java b/tests/src/org/aspectj/systemtest/ajc150/AllTestsJava5_binaryWeaving.java new file mode 100644 index 000000000..c6b9d3787 --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc150/AllTestsJava5_binaryWeaving.java @@ -0,0 +1,32 @@ +/******************************************************************************* + * Copyright (c) 2004 IBM + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Common Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/cpl-v10.html + * + * Contributors: + * Andy Clement - initial API and implementation + *******************************************************************************/ +package org.aspectj.systemtest.ajc150; + +import junit.framework.Test; +import junit.framework.TestSuite; + +/** + * @author colyer + * + * TODO To change the template for this generated type comment go to + * Window - Preferences - Java - Code Style - Code Templates + */ +public class AllTestsJava5_binaryWeaving { + + public static Test suite() { + TestSuite suite = new TestSuite("AspectJ System Test Suite - Java5 binary weaving"); + //$JUnit-BEGIN$ + suite.addTest(Ajc150Tests.suite()); + suite.addTest(AccBridgeMethods.suite()); + //$JUnit-END$ + return suite; + } +} diff --git a/tests/src/org/aspectj/systemtest/ajc150/ajc150-tests.xml b/tests/src/org/aspectj/systemtest/ajc150/ajc150-tests.xml new file mode 100644 index 000000000..b770e0de2 --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc150/ajc150-tests.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml new file mode 100644 index 000000000..b85b36f54 --- /dev/null +++ b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml @@ -0,0 +1,12 @@ + +]> + + + + + +&tests; + + + diff --git a/weaver/src/org/aspectj/weaver/Constants.java b/weaver/src/org/aspectj/weaver/Constants.java new file mode 100644 index 000000000..d4cf4cc6e --- /dev/null +++ b/weaver/src/org/aspectj/weaver/Constants.java @@ -0,0 +1,24 @@ +/******************************************************************************* + * Copyright (c) 2004 IBM + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Common Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/cpl-v10.html + * + * Contributors: + * Andy Clement - initial API and implementation + *******************************************************************************/ +package org.aspectj.weaver; + +/** + * Some useful weaver constants. + * + * Current uses: + * 1. Holds values that are necessary for working with 1.5 code but + * which don't exist in a 1.4 world. + */ +public interface Constants { + + // modifier for a bridge method + public final static int ACC_BRIDGE = 0x0040; +} diff --git a/weaver/src/org/aspectj/weaver/Lint.java b/weaver/src/org/aspectj/weaver/Lint.java index 248ad0d57..fa5ef51d7 100644 --- a/weaver/src/org/aspectj/weaver/Lint.java +++ b/weaver/src/org/aspectj/weaver/Lint.java @@ -61,7 +61,10 @@ public class Lint { public final Kind noInterfaceCtorJoinpoint = new Kind("noInterfaceCtorJoinpoint","no interface constructor-execution join point - use {0}+ for implementing classes"); - + + public final Kind noJoinpointsForBridgeMethods = + new Kind("noJoinpointsForBridgeMethods","pointcut did not match on the method call to a bridge method. Bridge methods are generated by the compiler and have no join points"); + public Lint(World world) { this.world = world; } diff --git a/weaver/src/org/aspectj/weaver/ResolvedMember.java b/weaver/src/org/aspectj/weaver/ResolvedMember.java index ab1cdc90f..7f847510c 100644 --- a/weaver/src/org/aspectj/weaver/ResolvedMember.java +++ b/weaver/src/org/aspectj/weaver/ResolvedMember.java @@ -102,6 +102,10 @@ public class ResolvedMember extends Member implements IHasPosition { return true; } + public boolean isBridgeMethod() { + return (modifiers & Constants.ACC_BRIDGE)!=0; + } + public boolean isSynthetic() { return false; } diff --git a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java index 4342ce4fc..45d24e79e 100644 --- a/weaver/src/org/aspectj/weaver/ResolvedTypeX.java +++ b/weaver/src/org/aspectj/weaver/ResolvedTypeX.java @@ -1086,6 +1086,10 @@ public abstract class ResolvedTypeX extends TypeX { return samePackage(targetType, fromType); } } + + public static boolean hasBridgeModifier(int modifiers) { + return (modifiers & Constants.ACC_BRIDGE)!=0; + } private static boolean samePackage( ResolvedTypeX targetType, diff --git a/weaver/src/org/aspectj/weaver/XlintDefault.properties b/weaver/src/org/aspectj/weaver/XlintDefault.properties index 3b584d93f..3c54f58b0 100644 --- a/weaver/src/org/aspectj/weaver/XlintDefault.properties +++ b/weaver/src/org/aspectj/weaver/XlintDefault.properties @@ -14,4 +14,6 @@ canNotImplementLazyTjp = warning needsSerialVersionUIDField = ignore brokeSerialVersionCompatibility = ignore -noInterfaceCtorJoinpoint = warning \ No newline at end of file +noInterfaceCtorJoinpoint = warning + +noJoinpointsForBridgeMethods = warning \ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java index 9596f6ed2..a5752e5ed 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java @@ -906,6 +906,7 @@ class BcelClassWeaver implements IClassWeaver { } private boolean shouldWeaveBody(LazyMethodGen mg) { + if (mg.isBridgeMethod()) return false; if (mg.isAjSynthetic()) return mg.getName().equals(""); AjAttribute.EffectiveSignatureAttribute a = mg.getEffectiveSignature(); if (a != null) return a.isWeaveBody(); diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java index fac4f8eab..da83af2af 100644 --- a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java +++ b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java @@ -684,6 +684,10 @@ public final class LazyMethodGen { public boolean isAbstract() { return Modifier.isAbstract(getAccessFlags()); } + + public boolean isBridgeMethod() { + return (getAccessFlags() & Constants.ACC_BRIDGE) != 0; + } public void addExceptionHandler( InstructionHandle start, diff --git a/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java b/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java index 4dda76c47..662a21afe 100644 --- a/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java +++ b/weaver/src/org/aspectj/weaver/patterns/KindedPointcut.java @@ -24,6 +24,7 @@ import org.aspectj.weaver.Checker; import org.aspectj.weaver.ISourceContext; import org.aspectj.weaver.IntMap; import org.aspectj.weaver.Member; +import org.aspectj.weaver.ResolvedMember; import org.aspectj.weaver.ResolvedTypeX; import org.aspectj.weaver.Shadow; import org.aspectj.weaver.ShadowMunger; @@ -63,10 +64,16 @@ public class KindedPointcut extends Pointcut { public FuzzyBoolean match(Shadow shadow) { if (shadow.getKind() != kind) return FuzzyBoolean.NO; - + if (!signature.matches(shadow.getSignature(), shadow.getIWorld())){ + if(kind == Shadow.MethodCall) { warnOnConfusingSig(shadow); + int shadowModifiers = shadow.getSignature().getModifiers(shadow.getIWorld()); + if (ResolvedTypeX.hasBridgeModifier(shadowModifiers)) { + shadow.getIWorld().getLint().noJoinpointsForBridgeMethods.signal(new String[]{},getSourceLocation(), + new ISourceLocation[]{shadow.getSourceLocation()}); + } } return FuzzyBoolean.NO; } diff --git a/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java b/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java index 5e4e12185..f9a7373fc 100644 --- a/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java +++ b/weaver/src/org/aspectj/weaver/patterns/SignaturePattern.java @@ -110,8 +110,19 @@ public class SignaturePattern extends PatternNode { public boolean matches(Member member, World world) { //XXX performance gains would come from matching on name before resolving - // to fail fast + // to fail fast. ASC 30th Nov 04 => Not necessarily, it didn't make it faster for me. + // Here is the code I used: + // String n1 = member.getName(); + // String n2 = this.getName().maybeGetSimpleName(); + // if (n2!=null && !n1.equals(n2)) return false; + ResolvedMember sig = member.resolve(world); + + // Java5 introduces bridge methods, we don't want to match on them at all... + if (sig.isBridgeMethod()) { + return false; + } + if (sig == null) { //XXX if (member.getName().startsWith(NameMangler.PREFIX)) {