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)

20 files changed, 291 insertions, 7 deletions

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<A> {
+  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<Number> {
+  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 @@
+<project name="Java 5 compilation of test source" default="default" basedir=".">
+
+    <target name="default">
+       <delete dir="output" failonerror="false"/>
+       <mkdir dir="output"/>
+       <javac destdir="output" debug="on" srcdir="." includes="*.java"/>
+       <zip file="testcode.jar" basedir="output" includes="*"/>
+       <delete dir="output"/>
+    </target>
+
+</project>
diff --git a/tests/java5/bridgeMethods/testcode.jar b/tests/java5/bridgeMethods/testcode.jar
new file mode 100644
index 000000000..f543454b0
--- /dev/null
+++ b/tests/java5/bridgeMethods/testcode.jar
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;
+
+
+/**
+ * <b>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.</b>
+ * 
+ * <p>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.
+ * 
+ * <p>They impact AspectJ in two ways:
+ * <ol>
+ * <li>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).
+ * <li> They create a potential call join point where a call can be made to the bridge method.
+ * </ol>  
+ * 
+ * <p>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 @@
+<!-- AspectJ v1.5.0 Tests: Bridge attribute on methods generated to support generics -->
+    
+    <ajc-test dir="java5/bridgeMethods" pr="72766" title="Ignore bridge methods">
+        <compile files="AspectX.aj" inpath="testcode.jar" options="-showWeaveInfo">
+          <message kind="warning" line="7" text="pointcut did not match on the method call to a bridge method."/>
+        </compile>
+    </ajc-test>
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 @@
+<!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd"[
+<!ENTITY tests SYSTEM "../tests/src/org/aspectj/systemtest/ajc150/ajc150-tests.xml">
+]>
+
+<!-- AspectJ v1.5.0 Tests -->
+
+<suite>
+
+&tests;
+
+</suite>
+
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("<clinit>");
 		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)) {