fix and better tests for

Bugzilla Bug 37152  
   java.lang.VerifyError:

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java

@@ -13,6 +13,8 @@
 
 package org.aspectj.ajdt.internal.compiler.ast;
 
+import java.util.Arrays;
+
 import org.aspectj.ajdt.internal.compiler.lookup.EclipseFactory;
 import org.aspectj.ajdt.internal.compiler.lookup.InlineAccessFieldBinding;
 import org.aspectj.ajdt.internal.compiler.lookup.InterTypeFieldBinding;
@@ -77,9 +79,18 @@ public class AccessForInlineVisitor extends AbstractSyntaxTreeVisitorAdapter {
 	}
 
 	public void endVisit(QualifiedNameReference ref, BlockScope scope) {
+		//System.err.println("qref: " + ref + ", " + ref.binding.getClass().getName() + ", " + ref.codegenBinding.getClass().getName());
+		//System.err.println("   others: " + Arrays.asList(ref.otherBindings));
 		if (ref.binding instanceof FieldBinding) {
 			ref.binding = getAccessibleField((FieldBinding)ref.binding);
 		}
+		if (ref.otherBindings != null) {
+			for (int i=0, len=ref.otherBindings.length; i < len; i++) {
+				if (ref.otherBindings[i] instanceof FieldBinding) {
+					ref.otherBindings[i] = getAccessibleField((FieldBinding)ref.otherBindings[i]);
+				}
+			}
+		}
 	}
 
 	public void endVisit(FieldReference ref, BlockScope scope) {
@@ -121,6 +132,7 @@ public class AccessForInlineVisitor extends AbstractSyntaxTreeVisitorAdapter {
 	}
 	
 	private FieldBinding getAccessibleField(FieldBinding binding) {
+		//System.err.println("checking field: " + binding);
 		if (!binding.isValidBinding()) return binding;
 		
 		makePublic(binding.declaringClass);
@@ -135,6 +147,9 @@ public class AccessForInlineVisitor extends AbstractSyntaxTreeVisitorAdapter {
 		ResolvedMember m = EclipseFactory.makeResolvedMember(binding);
 		if (inAspect.accessForInline.containsKey(m)) return (FieldBinding)inAspect.accessForInline.get(m);
 		FieldBinding ret = new InlineAccessFieldBinding(inAspect, binding);
+		
+		//System.err.println("   made accessor: " + ret);
+		
 		inAspect.accessForInline.put(m, ret);
 		return ret;
 	}

org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java

@@ -49,8 +49,8 @@ public class WorkingTestMain {
 		//args.add("-aspectpath");
 		//args.add("../weaver/testdata/megatrace.jar");
 		
-		args.add("testdata/src1/AroundA1.java");
-		args.add("-XnoInline");
+		args.add("c:/aspectj/scratch/arno/*.java");
+		//args.add("-XnoInline");
 		//args.add("../tests/new/Counting1.java");
 		//args.add("-Xlint:error");
 		//args.add("testdata/src1/InterType.java");
@@ -60,11 +60,11 @@ public class WorkingTestMain {
 		CommandTestCase.runCompiler(args, CommandTestCase.NO_ERRORS);
 		//CommandTestCase.runCompiler(args, new int[] {11, 14, 18, 32, 43});
 		
-		CommandTestCase.printGenerated("../out", "AroundA1");
+		//CommandTestCase.printGenerated("../out", "AroundA1");
 //		CommandTestCase.printGenerated("../out", "SuperC");
-//		CommandTestCase.printGenerated("../out", "SubC");
+		CommandTestCase.printGenerated("../out", "org.schmidmeier.unittests.cache.TimeCacheTestsWorking");
 
-		TestUtil.runMain("out;../lib/test/testing-client.jar", "AroundA1");
+		TestUtil.runMain("out;../lib/test/testing-client.jar", "org.schmidmeier.unittests.cache.AllTimeCacheTests");
 	}
 	
 	private static String examplesDir = "../docs/dist/doc/examples/";

tests/ajcTests.xml

@@ -13,7 +13,6 @@
                       so there may only be one copy marked "messages-vary" here.
   new-messages-vary   like messages-vary, except need to make ajcTest10 variant
 
-  incremental-test    incremental test.  All tests using inc-compile.
   fail-{...}          test fails in some configuration
   fail-unimplmented   eajc throwing "unimplemented" exception
   fail-commandLine    fails in ajc on command line (move to ajcTestsBroken.xml)
@@ -5745,7 +5744,7 @@
     </ajc-test>
 
     <ajc-test dir="incremental/stringliteral"   
-    	keywords="knownLimitation,incremental-test"
+    	keywords="knownLimitation"
     	title="incrementally change string size and wire in injar classes">
         <compile staging="true" options="-incremental" 
         	files="oneInjar.jar,twoInjar.jar"
@@ -6154,4 +6153,12 @@
         <run class="Main"/>
     </ajc-test>
 
+
+    <ajc-test dir="bugs/inlineAround" pr="37152"
+      title="perthis and inline arounds">
+        <compile files="aspect1/Base.java,aspect2/Concrete.java,p1/Main.java">
+        </compile>
+        <run class="p1.Main"/>
+    </ajc-test>
+
 </suite>

tests/bugs/inlineAround/aspect1/Base.java

@@ -0,0 +1,30 @@
+package aspect1;
+
+public abstract aspect Base {
+	private Helper h = new Helper();
+	{
+	    h.h1 = new Helper();
+	    h.h1.h1 = new Helper();
+	}
+
+	private class Inner {
+	    String data = "inner";
+	}
+	
+	protected abstract pointcut where();
+	
+	Object around(double d, int i): where() && args(i, d) {
+		String s = h.data + h.h1.data + h.h1.h1.data + d + i;
+		System.err.println(s);
+		return proceed(d, i);
+	}
+}
+
+
+class Helper {
+	String data = "helper";
+	Helper h1;
+	String getData() {
+		return data;
+	}
+}

tests/bugs/inlineAround/aspect2/Concrete.java

@@ -0,0 +1,7 @@
+package aspect2;
+
+import aspect1.Base;
+
+aspect Concrete extends Base perthis(where()) {
+	protected pointcut where(): call(* *..C.*(..));
+}

tests/bugs/inlineAround/p1/Main.java

@@ -0,0 +1,26 @@
+package p1;
+
+public class Main {
+	public static void main(String[] args) {
+		new Main().doit();
+	}
+	
+	private void doit() {
+		long l = 2l + testit(3.2, new C().doit(23, 3.14), 5l);
+		System.err.println(l);
+	}
+	
+	private long testit(double d, long l1, long l2) {
+		return (long)(d + l1 + l2);
+	}
+}
+
+class C {
+	long doit(int i, double d) {
+		return (long)(d + i + f1(d, i, i));
+	}
+	
+	int f1(double d1, long l1, int i1) {
+		return (int)(d1 + l1 + i1);
+	}
+}

tests/jimTests.xml

@@ -1,16 +1,4 @@
 <!DOCTYPE suite SYSTEM "../tests/ajcTestSuite.dtd">
 <suite> 
-    <ajc-test dir="options" 
-      title="options -warn:deprecation">
-        <compile files="WarnDeprecated.java"
-        	options="!eclipse,-warn:deprecation">
-            <message kind="warning" line="10"/>
-        </compile>
-    </ajc-test>
-    <ajc-test dir="options" 
-      title="options -warn:deprecation turned off">
-        <compile files="WarnDeprecated.java"
-        	options="!eclipse">
-        </compile>
-    </ajc-test>
+
 </suite>

weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java

@@ -77,6 +77,15 @@ public class BcelAdvice extends Advice {
     	}
 		pointcutTest = getPointcut().findResidue(shadow, exposedState);
 		
+		// these initializations won't be performed by findResidue, but need to be
+		// so that the joinpoint is primed for weaving
+		if (getKind() == AdviceKind.PerThisEntry) {
+			shadow.getThisVar();
+		} else if (getKind() == AdviceKind.PerTargetEntry) {
+			shadow.getTargetVar();
+		}
+		
+		
         // make sure thisJoinPoint parameters are initialized
         if ((getExtraParameterFlags() & ThisJoinPointStaticPart) != 0) {
         	((BcelShadow)shadow).getThisJoinPointStaticPartVar();

weaver/src/org/aspectj/weaver/bcel/BcelShadow.java

@@ -104,6 +104,9 @@ import org.aspectj.weaver.ast.Var;
  *      do have a target of sorts, just one that needs to be pushed on the stack,
  *      dupped, and not touched otherwise until the constructor runs.
  * 
+ * @author Jim Hugunin
+ * @author Erik Hilsdale
+ * 
  */
 
 public class BcelShadow extends Shadow {
@@ -1176,8 +1179,8 @@ public class BcelShadow extends Shadow {
 		 * 
 		 * It extracts the instructions of the original shadow into a method.
 		 * 
-		 * Then it inlines the instructions of the advice in its place, taking care
-		 * to treat the closure argument specially (it doesn't exist).
+		 * Then it extracts the instructions of the advice into a new method defined on
+		 * this enclosing class.  This new method can then be specialized as below.
 		 * 
 		 * Then it searches in the instructions of the advice for any call to the
 		 * proceed method.
@@ -1192,6 +1195,11 @@ public class BcelShadow extends Shadow {
 		 *
 		 * If only one call to proceed is made, we can re-inline the original shadow.
 		 * We are not doing that presently.
+		 * 
+		 * If the body of the advice can be determined to not alter the stack, or if
+		 * this shadow doesn't care about the stack, i.e. method-execution, then the
+		 * new method for the advice can also be re-lined.  We are not doing that
+		 * presently.
 		 */
 		 
 		// !!! THIS BLOCK OF CODE SHOULD BE IN A METHOD CALLED weaveAround(...);
@@ -1229,35 +1237,30 @@ public class BcelShadow extends Shadow {
 							getSignature(),
 							getEnclosingClass()) + "$advice";
         
-		List paramTypeList = new ArrayList();
 		List argVarList = new ArrayList();
 		List proceedVarList = new ArrayList();
 		int extraParamOffset = 0;
 		
-		//TODO paramTypeList not needed any more
-		
-		// start w/ stuff
+		// Create the extra parameters that are needed for passing to proceed
+		// This code is very similar to that found in makeCallToCallback and should
+		// be rationalized in the future
 		if (thisVar != null) {
-			paramTypeList.add(thisVar.getType());
 			argVarList.add(thisVar);
 			proceedVarList.add(new BcelVar(thisVar.getType(), extraParamOffset));
 			extraParamOffset += thisVar.getType().getSize();
 		}
 		
 		if (targetVar != null && targetVar != thisVar) {
-			paramTypeList.add(targetVar.getType());
 			argVarList.add(targetVar);
 			proceedVarList.add(new BcelVar(targetVar.getType(), extraParamOffset));
 			extraParamOffset += targetVar.getType().getSize();
 		}
 		for (int i = 0, len = getArgCount(); i < len; i++) {
-			paramTypeList.add(argVars[i].getType());
 			argVarList.add(argVars[i]);
 			proceedVarList.add(new BcelVar(argVars[i].getType(), extraParamOffset));
 			extraParamOffset += argVars[i].getType().getSize();
 		}
 		if (thisJoinPointVar != null) {
-			paramTypeList.add(thisJoinPointVar.getType());
 			argVarList.add(thisJoinPointVar);
 			proceedVarList.add(new BcelVar(thisJoinPointVar.getType(), extraParamOffset));
 			extraParamOffset += thisJoinPointVar.getType().getSize();
@@ -1270,10 +1273,6 @@ public class BcelShadow extends Shadow {
 		System.arraycopy(extractedMethodParameterTypes, 0, parameterTypes, parameterIndex, extractedMethodParameterTypes.length);
 		parameterIndex += extractedMethodParameterTypes.length;
 
-//        for (Iterator i = paramTypeList.iterator(); i.hasNext(); ) {
-//        	ResolvedTypeX t = (ResolvedTypeX)i.next();
-//        	parameterTypes[parameterIndex++] = BcelWorld.makeBcelType(t);
-//        }
         parameterTypes[parameterIndex++] =
         	BcelWorld.makeBcelType(adviceMethod.getEnclosingClass().getType());
 		System.arraycopy(adviceParameterTypes, 0, parameterTypes, parameterIndex, adviceParameterTypes.length);
@@ -1289,6 +1288,10 @@ public class BcelShadow extends Shadow {
     
 		getEnclosingClass().addMethodGen(localAdviceMethod);
 		
+		
+		// create a map that will move all slots in advice method forward by extraParamOffset
+		// in order to make room for the new proceed-required arguments that are added at
+		// the beginning of the parameter list
 		int nVars = adviceMethod.getMaxLocals() + extraParamOffset;
 		IntMap varMap = IntMap.idMap(nVars);
 		for (int i=extraParamOffset; i < nVars; i++) {
@@ -1350,11 +1353,7 @@ public class BcelShadow extends Shadow {
             range.append(callback);
             range.append(afterThingie);          
         }        
-        
-        // now the range contains everything we need.  We now inline the advice method.
 
-				
-        //BcelClassWeaver.inlineMethod(adviceMethod, enclosingMethod, adviceMethodInvocation);
 
         // now search through the advice, looking for a call to PROCEED.  
         // Then we replace the call to proceed with some argument setup, and a 
@@ -1391,31 +1390,7 @@ public class BcelShadow extends Shadow {
 		// we have on stack all the arguments for the ADVICE call.
 		// we have in frame somewhere all the arguments for the non-advice call.
 		
-//		List argVarList = new ArrayList();
-//		
-//		// start w/ stuff
-//		if (thisVar != null) {
-//			argVarList.add(thisVar);
-//		}
-//		
-//        if (targetVar != null && targetVar != thisVar) {
-//            argVarList.add(targetVar);
-//        }
-//		for (int i = 0, len = getArgCount(); i < len; i++) {
-//			argVarList.add(argVars[i]);
-//		}
-//		if (thisJoinPointVar != null) {
-//			argVarList.add(thisJoinPointVar);
-//		}
-		
 		BcelVar[] adviceVars = munger.getExposedStateAsBcelVars();		
-		//??? this is too easy
-//		for (int i=0; i < adviceVars.length; i++) {
-//			if (adviceVars[i] != null) 
-//				adviceVars[i].setPositionInAroundState(i);
-//		}
-
-
 		IntMap proceedMap =  makeProceedArgumentMap(adviceVars);
 
 //		System.out.println(proceedMap + " for " + this);