Fix for Bugzilla Bug 50641

   Better binary compatibility for advice method names 
- I've run the tests a thousand times and they all pass, I'm still nervous about this first big commit though *gulp*

10 files changed, 107 insertions, 26 deletions

diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java
index 6b6b801f5..9d10f6381 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AdviceDeclaration.java
@@ -263,11 +263,18 @@ public class AdviceDeclaration extends MethodDeclaration {
 		return ret;
 	}
 
-
 	
 	public void postParse(TypeDeclaration typeDec) {
+		int adviceSequenceNumberInType = ((AspectDeclaration)typeDec).adviceCounter++;
+		
+		StringBuffer stringifiedPointcut = new StringBuffer(30);
+		pointcutDesignator.print(0,stringifiedPointcut);
 		this.selector =
-			NameMangler.adviceName(EclipseFactory.fromBinding(typeDec.binding), kind, sourceStart).toCharArray();
+			NameMangler.adviceName(
+			  EclipseFactory.fromBinding(typeDec.binding), 
+			  kind, 
+			  adviceSequenceNumberInType,
+			  stringifiedPointcut.toString().hashCode()).toCharArray();
 		if (arguments != null) {
 			baseArgumentCount = arguments.length;
 		}
diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java
index a3cff8ae6..27db10c3e 100644
--- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java
+++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AspectDeclaration.java
@@ -53,6 +53,7 @@ public class AspectDeclaration extends TypeDeclaration {
 	
 	public EclipseFactory factory;  //??? should use this consistently
 
+    public int adviceCounter = 1; // Used as a part of the generated name for advice methods
 
 	// for better error messages in 1.0 to 1.1 transition
 	public TypePattern dominatesPattern;
diff --git a/org.aspectj.ajdt.core/testsrc/AroundAMain.java b/org.aspectj.ajdt.core/testsrc/AroundAMain.java
index 2f101dd74..fcdeab18e 100644
--- a/org.aspectj.ajdt.core/testsrc/AroundAMain.java
+++ b/org.aspectj.ajdt.core/testsrc/AroundAMain.java
@@ -33,12 +33,12 @@ public class AroundAMain extends TestCase {
 		Object instance = Reflection.getStaticField(Class.forName("AroundA"),
 			"ajc$perSingletonInstance");
 
-		Reflection.invoke(Class.forName("AroundA"), instance, "ajc$around$AroundA$46",
+		Reflection.invoke(Class.forName("AroundA"), instance, "ajc$around$AroundA$1$73ebb943", // was $AroundA$46
 					new Integer(10), new Boolean(true), closure);
 
-		Reflection.invoke(Class.forName("AroundA"), instance, "ajc$around$AroundA$c5",
+		Reflection.invoke(Class.forName("AroundA"), instance, "ajc$around$AroundA$2$a758212d",  // Was $AroundA$c5
 					"hello there", closure);
-		Reflection.invoke(Class.forName("AroundA"), instance, "ajc$around$AroundA$150",
+		Reflection.invoke(Class.forName("AroundA"), instance, "ajc$around$AroundA$3$a758212d",  // Was $AroundA$150
 					new String[1], closure);
 
 	}
diff --git a/tests/ajcTests.xml b/tests/ajcTests.xml
index 57b496aaa..4f68d8e00 100644
--- a/tests/ajcTests.xml
+++ b/tests/ajcTests.xml
@@ -7142,9 +7142,34 @@
         <run class="HandlerSig"/>
     </ajc-test>
     
+
     <ajc-test dir="new" pr="42668"
    	 title="after returning with parameter: matching rules">
         <compile files="AfterReturningParamMatching.java" />
         <run class="AfterReturningParamMatching"/>
     </ajc-test>
+    
+    <ajc-test dir="bugs/binaryCompat" pr="50641"
+      title="binary compatibility of advice method names - expect no error">
+        <compile files="Main.java,TraceV1.aj"/>
+        <run class="Main"/>
+        <compile files="TraceV2.aj"/>
+        <run class="Main"/>
+    </ajc-test>
+   
+    <ajc-test dir="bugs/binaryCompat" pr="50641"
+      title="binary compatibility of advice method names - expect error">
+        <compile files="Main.java,TraceV1.aj"/>
+        <run class="Main"/>
+        <compile files="TraceRE.aj"/>
+        <run class="Main"/>
+    </ajc-test>
+    
+     <ajc-test dir="bugs/binaryCompat" pr="50641"
+      title="binary compatibility of advice method names - expect no error">
+        <compile files="Main.java,TraceWithInnerV1.aj"/>
+        <run class="Main"/>
+        <compile files="TraceWithInnerV2.aj"/>
+        <run class="Main"/>
+    </ajc-test>
 </suite>
diff --git a/tests/ajcTestsFailing.xml b/tests/ajcTestsFailing.xml
index cea32f4dd..c2fb46b05 100644
--- a/tests/ajcTestsFailing.xml
+++ b/tests/ajcTestsFailing.xml
@@ -127,22 +127,6 @@
         <run class="InterfaceMethodDeclarationNonPublic"/>
     </ajc-test>
 
-    <ajc-test dir="bugs/binaryCompat" pr="50641"
-      title="binary compatibility of advice method names - expect no error">
-        <compile files="Main.java,TraceV1.aj"/>
-        <run class="Main"/>
-        <compile files="TraceV2.aj"/>
-        <run class="Main"/>
-    </ajc-test>
-   
-    <ajc-test dir="bugs/binaryCompat" pr="50641"
-      title="binary compatibility of advice method names - expect error">
-        <compile files="Main.java,TraceV1.aj"/>
-        <run class="Main"/>
-        <compile files="TraceRE.aj"/>
-        <run class="Main"/>
-    </ajc-test>
-
     <ajc-test dir="bugs" pr="36787"
       title="interface initialization order">
         <compile files="InterfaceInitializerOrder.java"/>
diff --git a/tests/bugs/binaryCompat/Main.java b/tests/bugs/binaryCompat/Main.java
index d91978f9f..0903463d9 100644
--- a/tests/bugs/binaryCompat/Main.java
+++ b/tests/bugs/binaryCompat/Main.java
@@ -14,4 +14,4 @@ public class Main {
 	private static void doit() {
 		System.out.println("hello world");
 	}
-}
\ No newline at end of file
+}
diff --git a/tests/bugs/binaryCompat/TraceV2.aj b/tests/bugs/binaryCompat/TraceV2.aj
index c71416af3..5764387b0 100644
--- a/tests/bugs/binaryCompat/TraceV2.aj
+++ b/tests/bugs/binaryCompat/TraceV2.aj
@@ -3,8 +3,10 @@ aspect Trace {
 	

 	before(): execution(void doit(..)) {

 		System.out.println("entering");

+		

 	}

 	

+	

 	after() returning: execution(void doit(..)) {

 		System.out.println("exiting");

 	}

diff --git a/tests/bugs/binaryCompat/TraceWithInnerV1.aj b/tests/bugs/binaryCompat/TraceWithInnerV1.aj
new file mode 100644
index 000000000..2238a2323
--- /dev/null
+++ b/tests/bugs/binaryCompat/TraceWithInnerV1.aj
@@ -0,0 +1,24 @@
+aspect Trace {

+	public static boolean expectNoSuchMethodError = false;

+	

+	before(): execution(void doit(..)) {

+		System.out.println("enter");

+	}

+

+	static aspect InnerTrace {

+		before(): execution(void doit(..)) {

+			System.out.println("Inner enter");

+		}

+		

+		after() returning: execution(void doit(..)) {

+			System.out.println("Inner exit");

+		}

+		

+		after() throwing: execution(void doit(..)) {

+			System.out.println("Inner after throwing");

+		}

+	}

+	after() returning: execution(void doit(..)) {

+		System.out.println("exit");

+	}

+}
\ No newline at end of file
diff --git a/tests/bugs/binaryCompat/TraceWithInnerV2.aj b/tests/bugs/binaryCompat/TraceWithInnerV2.aj
new file mode 100644
index 000000000..297fdfba7
--- /dev/null
+++ b/tests/bugs/binaryCompat/TraceWithInnerV2.aj
@@ -0,0 +1,34 @@
+aspect Trace {

+	public static boolean expectNoSuchMethodError = false;

+	

+	before(): execution(void doit(..)) {

+		System.out.println("entering");

+		

+	}

+

+	public void method() {

+		// Extra method to do nothing but test if the numbering still behaves	

+	}

+

+	static aspect InnerTrace {

+		before(): execution(void doit(..)) {

+			System.out.println("Inner entering");

+		}

+		

+		after() returning: execution(void doit(..)) {

+			System.out.println("Inner exiting");

+		}

+		

+		after() throwing: execution(void doit(..)) {

+			System.out.println("Inner chucking");

+		}

+		

+		before(): execution(* noMatch(..)) {

+			System.out.println("This doesn't match anything, but checks the sequence number for the next bit of advice is OK");

+		}

+	}

+	

+	after() returning: execution(void doit(..)) {

+		System.out.println("exiting");

+	}

+}
\ No newline at end of file
diff --git a/weaver/src/org/aspectj/weaver/NameMangler.java b/weaver/src/org/aspectj/weaver/NameMangler.java
index 7db5107ce..e73973541 100644
--- a/weaver/src/org/aspectj/weaver/NameMangler.java
+++ b/weaver/src/org/aspectj/weaver/NameMangler.java
@@ -89,13 +89,17 @@ public class NameMangler {
 	}
 	
 	
-	
 	/**
 	 * The name of methods corresponding to advice declarations
+	 * Of the form: "ajc$[AdviceKind]$[AspectName]$[NumberOfAdviceInAspect]$[PointcutHash]"
 	 */
-	public static String adviceName(TypeX aspectType, AdviceKind kind, int position) {
-		return makeName(kind.getName(), aspectType.getNameAsIdentifier(),
-					Integer.toHexString(position));
+	public static String adviceName(TypeX aspectType, AdviceKind kind, int adviceSeqNumber,int pcdHash) {
+		String newname = makeName(
+			kind.getName(),
+			aspectType.getNameAsIdentifier(),
+			Integer.toString(adviceSeqNumber),
+			Integer.toHexString(pcdHash));
+		return newname;
 	}
 	
 	/**