]> source.dussan.org Git - aspectj.git/commitdiff
fix and better tests for
authorjhugunin <jhugunin>
Fri, 2 May 2003 20:36:06 +0000 (20:36 +0000)
committerjhugunin <jhugunin>
Fri, 2 May 2003 20:36:06 +0000 (20:36 +0000)
Bugzilla Bug 37152
   java.lang.VerifyError:

org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/ast/AccessForInlineVisitor.java
org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java
tests/ajcTests.xml
tests/bugs/inlineAround/aspect1/Base.java [new file with mode: 0644]
tests/bugs/inlineAround/aspect2/Concrete.java [new file with mode: 0644]
tests/bugs/inlineAround/p1/Main.java [new file with mode: 0644]
tests/jimTests.xml
weaver/src/org/aspectj/weaver/bcel/BcelAdvice.java
weaver/src/org/aspectj/weaver/bcel/BcelShadow.java

index aa195135e145ab64eb75d39bca6afacc7693dbd2..76acd7306f6bb3f908fc9356d73c5699ead181ad 100644 (file)
@@ -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;
        }
index c86d1c50f6db333b10521de2e6ccdd50162aa6d3..d63418ff66971f7d8f95102744243cb1c5f08902 100644 (file)
@@ -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/";
index 6a164462d379a3b8dbaf1956f9186adae2e76127..732f9b4bbd0eb796b7575c485c429d1689c199da 100644 (file)
@@ -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)
     </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"
         <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>
diff --git a/tests/bugs/inlineAround/aspect1/Base.java b/tests/bugs/inlineAround/aspect1/Base.java
new file mode 100644 (file)
index 0000000..e61f346
--- /dev/null
@@ -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;
+       }
+}
\ No newline at end of file
diff --git a/tests/bugs/inlineAround/aspect2/Concrete.java b/tests/bugs/inlineAround/aspect2/Concrete.java
new file mode 100644 (file)
index 0000000..f065fa9
--- /dev/null
@@ -0,0 +1,7 @@
+package aspect2;
+
+import aspect1.Base;
+
+aspect Concrete extends Base perthis(where()) {
+       protected pointcut where(): call(* *..C.*(..));
+}
\ No newline at end of file
diff --git a/tests/bugs/inlineAround/p1/Main.java b/tests/bugs/inlineAround/p1/Main.java
new file mode 100644 (file)
index 0000000..bf44127
--- /dev/null
@@ -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);
+       }
+}
\ No newline at end of file
index c071015d0499ace1a84d88fba622fd7ef3b96851..80a883ac870f8ded053ef659e4f54e47e43b45ad 100644 (file)
@@ -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>
\ No newline at end of file
index a62bbd927412a2d6eb1d8fc6dd6c275a9f387b01..8666df8f5d781c96f7fbb746e97b7f792a40e661 100644 (file)
@@ -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();
index 04b785dc81df44cc7fc5c1dcd1210e07486d91d9..c91731e0e7dd445fc81d441e5a5579cc82d61cac 100644 (file)
@@ -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);