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*

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;
 		}

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;

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);
 
 	}

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>

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"/>

tests/bugs/binaryCompat/Main.java

@@ -14,4 +14,4 @@ public class Main {
 	private static void doit() {
 		System.out.println("hello world");
 	}
-}
+}

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");
 	}

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");
+	}
+}

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");
+	}
+}

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;
 	}
 	
 	/**