Fix 461323: around advice on default methods

7 files changed, 94 insertions, 21 deletions

diff --git a/tests/bugs188/defaultMethods/Code.java b/tests/bugs188/defaultMethods/Code.java
new file mode 100644
index 000000000..a162fe75e
--- /dev/null
+++ b/tests/bugs188/defaultMethods/Code.java
@@ -0,0 +1,18 @@
+public class Code implements I {
+  public static void main(String[] argv) {
+    new Code().m();
+  }
+}
+aspect X {
+  void around(): execution(* I.*()) {
+    System.out.println("a");
+    proceed();
+  }
+}
+
+interface I {
+  default void m(){
+    System.out.println("b");
+  }
+}
+
diff --git a/tests/bugs188/defaultMethods/Code2.java b/tests/bugs188/defaultMethods/Code2.java
new file mode 100644
index 000000000..ea5469eeb
--- /dev/null
+++ b/tests/bugs188/defaultMethods/Code2.java
@@ -0,0 +1,18 @@
+public class Code2 implements I {
+  public static void main(String[] argv) {
+    new Code2().m();
+  }
+}
+aspect X {
+  void around(): execution(* I.*()) {
+    System.out.println("a");
+    proceed();
+  }
+}
+
+interface I {
+  default void m(){
+    System.out.println("b");
+  }
+}
+
diff --git a/tests/src/org/aspectj/systemtest/ajc188/Ajc188Tests.java b/tests/src/org/aspectj/systemtest/ajc188/Ajc188Tests.java
index 11c503a89..c2a994855 100644
--- a/tests/src/org/aspectj/systemtest/ajc188/Ajc188Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc188/Ajc188Tests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2014 Contributors
+ * Copyright (c) 2015 Contributors
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -11,9 +11,6 @@
 package org.aspectj.systemtest.ajc188;
 
 import java.io.File;
-import java.lang.reflect.Method;
-import java.net.URL;
-import java.net.URLClassLoader;
 
 import junit.framework.Test;
 
@@ -24,6 +21,14 @@ import org.aspectj.testing.XMLBasedAjcTestCase;
  */
 public class Ajc188Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
 
+	public void testDefaultMethodsWithXnoInline() throws Exception {
+		runTest("default methods 1");
+	}
+
+	public void testDefaultMethodsWithoutXnoInline() throws Exception {
+		runTest("default methods 2");
+	}
+	
 	public void testCompileError_478003() throws Exception {
 		runTest("compile error");
 	}
diff --git a/tests/src/org/aspectj/systemtest/ajc188/ajc188.xml b/tests/src/org/aspectj/systemtest/ajc188/ajc188.xml
index cc1214c18..2e8a8b62f 100644
--- a/tests/src/org/aspectj/systemtest/ajc188/ajc188.xml
+++ b/tests/src/org/aspectj/systemtest/ajc188/ajc188.xml
@@ -2,6 +2,27 @@
 
 <suite>
 
+<ajc-test dir="bugs188/defaultmethods" title="default methods 1">
+<compile files="Code.java" options="-1.8 -XnoInline"/>
+<run class="Code">
+<stdout>
+<line text="a"/>
+<line text="b"/>
+</stdout>
+</run>
+</ajc-test>
+
+
+<ajc-test dir="bugs188/defaultmethods" title="default methods 2">
+<compile files="Code2.java" options="-1.8"/>
+<run class="Code2">
+<stdout>
+<line text="a"/>
+<line text="b"/>
+</stdout>
+</run>
+</ajc-test>
+
 <ajc-test dir="bugs188/478003" title="compile error">
 <compile files="OrientDBKeyIO.java" options="-1.8"/>
 </ajc-test>
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
index e6676900f..94462d5ff 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
@@ -1039,8 +1039,8 @@ public class BcelShadow extends Shadow {
 
 	private boolean checkLazyTjp() {
 		// check for around advice
-		for (Iterator i = mungers.iterator(); i.hasNext();) {
-			ShadowMunger munger = (ShadowMunger) i.next();
+		for (Iterator<ShadowMunger> i = mungers.iterator(); i.hasNext();) {
+			ShadowMunger munger = i.next();
 			if (munger instanceof Advice) {
 				if (((Advice) munger).getKind() == AdviceKind.Around) {
 					if (munger.getSourceLocation() != null) { // do we know enough to bother reporting?
@@ -2183,8 +2183,10 @@ public class BcelShadow extends Shadow {
 		// at the shadow, then tjp
 		String extractedShadowMethodName = NameMangler.aroundShadowMethodName(getSignature(), shadowClass.getNewGeneratedNameTag());
 		List<String> parameterNames = new ArrayList<String>();
-		LazyMethodGen extractedShadowMethod = extractShadowInstructionsIntoNewMethod(extractedShadowMethodName, Modifier.PRIVATE,
-				munger.getSourceLocation(), parameterNames);
+		boolean shadowClassIsInterface = shadowClass.isInterface();
+		LazyMethodGen extractedShadowMethod = extractShadowInstructionsIntoNewMethod(extractedShadowMethodName, 
+				shadowClassIsInterface?Modifier.PUBLIC:Modifier.PRIVATE,
+				munger.getSourceLocation(), parameterNames,shadowClassIsInterface);
 
 		List<BcelVar> argsToCallLocalAdviceMethodWith = new ArrayList<BcelVar>();
 		List<BcelVar> proceedVarList = new ArrayList<BcelVar>();
@@ -2205,7 +2207,6 @@ public class BcelShadow extends Shadow {
 			proceedVarList.add(new BcelVar(targetVar.getType(), extraParamOffset));
 			extraParamOffset += targetVar.getType().getSize();
 		}
-		int idx = 0;
 		for (int i = 0, len = getArgCount(); i < len; i++) {
 			argsToCallLocalAdviceMethodWith.add(argVars[i]);
 			proceedVarList.add(new BcelVar(argVars[i].getType(), extraParamOffset));
@@ -2242,8 +2243,8 @@ public class BcelShadow extends Shadow {
 		// Extract the advice into a new method. This will go in the same type as the shadow
 		// name will be something like foo_aroundBody1$advice
 		String localAdviceMethodName = NameMangler.aroundAdviceMethodName(getSignature(), shadowClass.getNewGeneratedNameTag());
-		LazyMethodGen localAdviceMethod = new LazyMethodGen(Modifier.PRIVATE | (world.useFinal() ? Modifier.FINAL : 0)
-				| Modifier.STATIC, BcelWorld.makeBcelType(mungerSig.getReturnType()), localAdviceMethodName, parameterTypes,
+		int localAdviceMethodModifiers = Modifier.PRIVATE | (world.useFinal() & !shadowClassIsInterface ? Modifier.FINAL : 0) | Modifier.STATIC;
+		LazyMethodGen localAdviceMethod = new LazyMethodGen(localAdviceMethodModifiers, BcelWorld.makeBcelType(mungerSig.getReturnType()), localAdviceMethodName, parameterTypes,
 				NoDeclaredExceptions, shadowClass);
 
 		// Doesnt work properly, so leave it out:
@@ -2833,9 +2834,12 @@ public class BcelShadow extends Shadow {
 
 		int linenumber = getSourceLine();
 		// MOVE OUT ALL THE INSTRUCTIONS IN MY SHADOW INTO ANOTHER METHOD!
+		
+		// callbackMethod will be something like: "static final void m_aroundBody0(I)"
+		boolean shadowClassIsInterface = getEnclosingClass().isInterface();
 		LazyMethodGen callbackMethod = extractShadowInstructionsIntoNewMethod(
-				NameMangler.aroundShadowMethodName(getSignature(), getEnclosingClass().getNewGeneratedNameTag()), 0,
-				munger.getSourceLocation(), new ArrayList<String>());
+				NameMangler.aroundShadowMethodName(getSignature(), getEnclosingClass().getNewGeneratedNameTag()), shadowClassIsInterface?Modifier.PUBLIC:0,
+				munger.getSourceLocation(), new ArrayList<String>(),shadowClassIsInterface);
 
 		BcelVar[] adviceVars = munger.getExposedStateAsBcelVars(true);
 
@@ -3123,15 +3127,16 @@ public class BcelShadow extends Shadow {
 	 * @param extractedMethodName name for the new method
 	 * @param extractedMethodVisibilityModifier visibility modifiers for the new method
 	 * @param adviceSourceLocation source location of the advice affecting the shadow
+	 * @param beingPlacedInInterface is this new method going into an interface
 	 */
 	LazyMethodGen extractShadowInstructionsIntoNewMethod(String extractedMethodName, int extractedMethodVisibilityModifier,
-			ISourceLocation adviceSourceLocation, List<String> parameterNames) {
+			ISourceLocation adviceSourceLocation, List<String> parameterNames, boolean beingPlacedInInterface) {
 		// LazyMethodGen.assertGoodBody(range.getBody(), extractedMethodName);
 		if (!getKind().allowsExtraction()) {
 			throw new BCException("Attempt to extract method from a shadow kind (" + getKind()
 					+ ") that does not support this operation");
 		}
-		LazyMethodGen newMethod = createShadowMethodGen(extractedMethodName, extractedMethodVisibilityModifier, parameterNames);
+		LazyMethodGen newMethod = createShadowMethodGen(extractedMethodName, extractedMethodVisibilityModifier, parameterNames, beingPlacedInInterface);
 		IntMap remapper = makeRemap();
 		range.extractInstructionsInto(newMethod, remapper, (getKind() != PreInitialization) && isFallsThrough());
 		if (getKind() == PreInitialization) {
@@ -3230,9 +3235,9 @@ public class BcelShadow extends Shadow {
 	 * The new method always static. It may take some extra arguments: this, target. If it's argsOnStack, then it must take both
 	 * this/target If it's argsOnFrame, it shares this and target. ??? rewrite this to do less array munging, please
 	 */
-	private LazyMethodGen createShadowMethodGen(String newMethodName, int visibilityModifier, List<String> parameterNames) {
+	private LazyMethodGen createShadowMethodGen(String newMethodName, int visibilityModifier, List<String> parameterNames, boolean beingPlacedInInterface) {
 		Type[] shadowParameterTypes = BcelWorld.makeBcelTypes(getArgTypes());
-		int modifiers = (world.useFinal() ? Modifier.FINAL : 0) | Modifier.STATIC | visibilityModifier;
+		int modifiers = (world.useFinal() && !beingPlacedInInterface ? Modifier.FINAL : 0) | Modifier.STATIC | visibilityModifier;
 		if (targetVar != null && targetVar != thisVar) {
 			UnresolvedType targetType = getTargetType();
 			targetType = ensureTargetTypeIsCorrect(targetType);
diff --git a/weaver/src/org/aspectj/weaver/bcel/Utility.java b/weaver/src/org/aspectj/weaver/bcel/Utility.java
index 20ec45ad0..afbfbc7ef 100644
--- a/weaver/src/org/aspectj/weaver/bcel/Utility.java
+++ b/weaver/src/org/aspectj/weaver/bcel/Utility.java
@@ -198,7 +198,12 @@ public class Utility {
 	public static Instruction createInvoke(InstructionFactory fact, LazyMethodGen m) {
 		short kind;
 		if (m.getEnclosingClass().isInterface()) {
-			kind = Constants.INVOKEINTERFACE;
+			if (m.isStatic()) {
+				// For static methods on interfaces
+				kind = Constants.INVOKESTATIC;				
+			} else {
+				kind = Constants.INVOKEINTERFACE;
+			}
 		} else if (m.isStatic()) {
 			kind = Constants.INVOKESTATIC;
 		} else if (m.isPrivate() || m.getName().equals("<init>")) {
diff --git a/weaver/testsrc/org/aspectj/weaver/bcel/MoveInstructionsWeaveTestCase.java b/weaver/testsrc/org/aspectj/weaver/bcel/MoveInstructionsWeaveTestCase.java
index 0155cb03c..bd7c2ae65 100644
--- a/weaver/testsrc/org/aspectj/weaver/bcel/MoveInstructionsWeaveTestCase.java
+++ b/weaver/testsrc/org/aspectj/weaver/bcel/MoveInstructionsWeaveTestCase.java
@@ -39,7 +39,7 @@ public class MoveInstructionsWeaveTestCase extends WeaveTestCase {
 				BcelShadow shadow = (BcelShadow) s;
 				LazyMethodGen newMethod = shadow.extractShadowInstructionsIntoNewMethod(NameMangler.getExtractableName(shadow
 						.getSignature())
-						+ "_extracted", 0, this.getSourceLocation(), new ArrayList());
+						+ "_extracted", 0, this.getSourceLocation(), new ArrayList(),shadow.getEnclosingClass().isInterface());
 				shadow.getRange().append(shadow.makeCallToCallback(newMethod));
 
 				if (!shadow.isFallsThrough()) {
@@ -63,9 +63,10 @@ public class MoveInstructionsWeaveTestCase extends WeaveTestCase {
 
 			public boolean implementOn(Shadow s) {
 				BcelShadow shadow = (BcelShadow) s;
-				LazyMethodGen newMethod = shadow.extractShadowInstructionsIntoNewMethod(NameMangler.getExtractableName(shadow
+				LazyMethodGen newMethod = 
+						shadow.extractShadowInstructionsIntoNewMethod(NameMangler.getExtractableName(shadow
 						.getSignature())
-						+ "_extracted" + counter++, 0, this.getSourceLocation(), new ArrayList());
+						+ "_extracted" + counter++, 0, this.getSourceLocation(), new ArrayList(),shadow.getEnclosingClass().isInterface());
 				shadow.getRange().append(shadow.makeCallToCallback(newMethod));
 
 				if (!shadow.isFallsThrough()) {