more bug fixes

10 files changed, 145 insertions, 56 deletions

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 66e004d00..26bf18d8b 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
@@ -59,7 +59,7 @@ public class AspectDeclaration extends MemberTypeDeclaration {
 	}
 	
 	public void resolve() {
-		if (binding == null || ignoreFurtherInvestigation) {
+		if (binding == null) {
 			ignoreFurtherInvestigation = true;
 			return;
 		}
diff --git a/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java b/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java
index 6b0cc3785..b70be133f 100644
--- a/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java
+++ b/org.aspectj.ajdt.core/testsrc/org/aspectj/ajdt/internal/compiler/batch/WorkingTestMain.java
@@ -35,6 +35,7 @@ public class WorkingTestMain {
 		List args = new ArrayList();
 		args.add("-verbose");
 		
+		//args.add("-1.3");
 		args.add("-d");
 		args.add("out");
 		
@@ -48,8 +49,8 @@ public class WorkingTestMain {
 		//args.add("-aspectpath");
 		//args.add("../weaver/testdata/megatrace.jar");
 		
-		//args.add("testdata/src1/AroundA1.java");
-		args.add("../tests/new/StrictFPAdvice.java");
+		args.add("testdata/src1/AroundA1.java");
+		//args.add("../tests/new/AroundInnerCalls.java");
 		//args.add("-Xlint:error");
 		//args.add("testdata/src1/InterType.java");
 		//args.add("@" + examplesDir + "tjp/files.lst");
@@ -58,11 +59,12 @@ public class WorkingTestMain {
 		CommandTestCase.runCompiler(args, CommandTestCase.NO_ERRORS);
 		//CommandTestCase.runCompiler(args, new int[] {11, 14, 18, 32, 43});
 		
-		CommandTestCase.printGenerated("../out", "StrictFPAdvice");
-		CommandTestCase.printGenerated("../out", "A");
+//		CommandTestCase.printGenerated("../out", "AdviceOnInheritedMethod");
+//		CommandTestCase.printGenerated("../out", "SuperC");
+//		CommandTestCase.printGenerated("../out", "SubC");
 
 		//TestUtil.runMain("out;../bcweaver/testdata/megatrace.jar", "Privileged");
-		TestUtil.runMain("out;../lib/test/testing-client.jar", "StrictFPAdvice");
+		//TestUtil.runMain("out;../lib/test/testing-client.jar", "AroundInnerCalls");
 	}
 	
 	private static String examplesDir = "c:/aspectj/examples/";
diff --git a/weaver/src/org/aspectj/weaver/World.java b/weaver/src/org/aspectj/weaver/World.java
index c12acf985..99f4c6c18 100644
--- a/weaver/src/org/aspectj/weaver/World.java
+++ b/weaver/src/org/aspectj/weaver/World.java
@@ -144,7 +144,9 @@ public abstract class World {
     }
 
     protected int getModifiers(Member member) {
-        return resolve(member).getModifiers();
+    	ResolvedMember r = resolve(member);
+    	if (r == null) throw new BCException("bad resolve of " + member);
+        return r.getModifiers();
     }
 
     protected String[] getParameterNames(Member member) {
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java b/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java
index c398cec63..5e3ef4665 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelCflowStackFieldAdder.java
@@ -36,7 +36,7 @@ public class BcelCflowStackFieldAdder extends BcelTypeMunger {
     		gen.getConstantPoolGen()).getField();
     	gen.addField(f);
 
-		LazyMethodGen clinit = gen.getStaticInitializer();
+		LazyMethodGen clinit = gen.getAjcClinit(); //StaticInitializer();
 		InstructionList setup = new InstructionList();
 		InstructionFactory fact = gen.getFactory();
 
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
index 432acd17b..acc8f96f1 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelShadow.java
@@ -309,6 +309,13 @@ public class BcelShadow extends Shadow {
             LazyMethodGen enclosingMethod) 
     {
         InstructionList body = enclosingMethod.getBody();
+        InstructionHandle ih = body.getStart();
+        if (ih.getInstruction() instanceof InvokeInstruction) {
+        	InvokeInstruction ii = (InvokeInstruction)ih.getInstruction();
+        	if (ii.getName(enclosingMethod.getEnclosingClass().getConstantPoolGen()).equals(NameMangler.AJC_CLINIT_NAME)) {
+        		ih = ih.getNext();
+        	}
+        }
         BcelShadow s =
             new BcelShadow(
                 world,
@@ -319,7 +326,7 @@ public class BcelShadow extends Shadow {
         ShadowRange r = new ShadowRange(body);
         r.associateWithShadow(s);
         r.associateWithTargets(
-            Range.genStart(body),
+            Range.genStart(body, ih),
             Range.genEnd(body));
         return s;
     }
@@ -654,7 +661,7 @@ public class BcelShadow extends Shadow {
         //???return TypeX.forName(getEnclosingClass().getClassName());      
     }
 
-    public boolean hasRealTarget() {
+    public boolean isTargetDifferentFromThis() {
         return hasTarget() && isExpressionKind();
     }
 
@@ -690,9 +697,9 @@ public class BcelShadow extends Shadow {
         if (!hasTarget()) {
             throw new IllegalStateException("no target");
         }
-        initializeTargetVar();
-        return targetVar;
-    }
+	    initializeTargetVar();
+	    return targetVar;
+        }
     public Var getArgVar(int i) {
         initializeArgVars();
         return argVars[i];
@@ -700,8 +707,8 @@ public class BcelShadow extends Shadow {
 
     // reflective thisJoinPoint support
     private BcelVar thisJoinPointVar = null;
-    private BcelVar thisJoinPointStaticPartVar = null;  //XXX should be field
-    private BcelVar thisEnclosingJoinPointStaticPartVar = null;  //XXX should be field
+    private BcelVar thisJoinPointStaticPartVar = null;  
+    private BcelVar thisEnclosingJoinPointStaticPartVar = null;  
     
 	public final Var getThisJoinPointVar() {
 		return getThisJoinPointBcelVar();
@@ -800,6 +807,7 @@ public class BcelShadow extends Shadow {
     private void initializeThisVar() {
         if (thisVar != null) return;
         thisVar = new BcelVar(getThisType().resolve(world), 0);
+        thisVar.setPositionInAroundState(0);
     }
     public void initializeTargetVar() {
     	InstructionFactory fact = getFactory();    	
@@ -812,14 +820,15 @@ public class BcelShadow extends Shadow {
             TypeX type = getTargetType();
             targetVar = genTempVar(type, "ajc$target");
             range.insert(targetVar.createStore(fact), Range.OutsideBefore); 
+	        targetVar.setPositionInAroundState(hasThis() ? 1 : 0);            
         }
-        targetVar.setPositionInAroundState(0);
     }
     public void initializeArgVars() {
         if (argVars != null) return;
     	InstructionFactory fact = getFactory();
         int len = getArgCount();
         argVars = new BcelVar[len];
+        int positionOffset = (hasTarget() ? 0 : 1) + (hasThis() ? 0 : 1);
                      
         if (getKind().argsOnStack()) {
             // we move backwards because we're popping off the stack
@@ -828,20 +837,21 @@ public class BcelShadow extends Shadow {
                 BcelVar tmp = genTempVar(type, "ajc$arg" + i);
                 range.insert(tmp.createStore(getFactory()), Range.OutsideBefore);
                 int position = i;
-                if (hasTarget()) position += 1;
+                if (hasTarget()) position += positionOffset;
                 tmp.setPositionInAroundState(position);
                 argVars[i] = tmp;
             }
         } else {
             int index = 0;
             if (hasThis()) index++;
+            
             for (int i = 0; i < len; i++) {
                 TypeX type = getArgType(i); 
                 BcelVar tmp = genTempVar(type, "ajc$arg" + i);
                 range.insert(tmp.createCopyFrom(fact, index), Range.OutsideBefore);
                 argVars[i] = tmp;
                 int position = i;
-                if (hasTarget()) position += 1;
+                if (hasTarget()) position += positionOffset;
                 tmp.setPositionInAroundState(position);
                 index += type.getSize();
             }       
@@ -850,6 +860,7 @@ public class BcelShadow extends Shadow {
     public void initializeForAroundClosure() {
         initializeArgVars();
         if (hasTarget()) initializeTargetVar();
+        if (hasThis()) initializeThisVar();
     }
 
             
@@ -925,6 +936,10 @@ public class BcelShadow extends Shadow {
     }
     
     public void weaveAfterThrowing(BcelAdvice munger, TypeX catchType) {
+    	// a good optimization would be not to generate anything here
+    	// if the shadow is GUARANTEED empty (i.e., there's NOTHING, not even
+    	// a shadow, inside me).
+    	if (getRange().getStart().getNext() == getRange().getEnd()) return;
         InstructionFactory fact = getFactory();        
         InstructionList handler = new InstructionList();        
         BcelVar exceptionVar = genTempVar(catchType);
@@ -1352,14 +1367,21 @@ public class BcelShadow extends Shadow {
         }
     }
     
-    // XXX only used for testing
+    // exposed for testing
     InstructionList makeCallToCallback(LazyMethodGen callbackMethod) {
     	InstructionFactory fact = getFactory();
         InstructionList callback = new InstructionList();
-        if (targetVar != null) {
+        if (thisVar != null) {
+        	callback.append(fact.ALOAD_0); 
+        }
+        if (targetVar != null && targetVar != thisVar) {
             callback.append(BcelRenderer.renderExpr(fact, world, targetVar));
         }
         callback.append(BcelRenderer.renderExprs(fact, world, argVars));
+        // remember to render tjps
+        if (thisJoinPointVar != null) {
+        	callback.append(BcelRenderer.renderExpr(fact, world, thisJoinPointVar));
+        }
         callback.append(Utility.createInvoke(fact, callbackMethod));
         return callback;
     }
@@ -1372,13 +1394,20 @@ public class BcelShadow extends Shadow {
         BcelVar arrayVar = genTempVar(TypeX.OBJECTARRAY);
         //final Type objectArrayType = new ArrayType(Type.OBJECT, 1);
         final InstructionList il = new InstructionList();
-        int alen = getArgCount() + (targetVar == null ? 0 : 1) + (thisJoinPointVar == null ? 0 : 1);
+        int alen = getArgCount() + (thisVar == null ? 0 : 1) + 
+        			((targetVar != null && targetVar != thisVar) ? 1 : 0) + 
+        			(thisJoinPointVar == null ? 0 : 1);
         il.append(Utility.createConstant(fact, alen));
         il.append((Instruction)fact.createNewArray(Type.OBJECT, (short)1));
         arrayVar.appendStore(il, fact);
 
         int stateIndex = 0;     
-        if (targetVar != null) {
+        if (thisVar != null) {
+            arrayVar.appendConvertableArrayStore(il, fact, stateIndex, thisVar);
+			thisVar.setPositionInAroundState(stateIndex);
+            stateIndex++;
+        }       	
+        if (targetVar != null && targetVar != thisVar) {
             arrayVar.appendConvertableArrayStore(il, fact, stateIndex, targetVar);
 			targetVar.setPositionInAroundState(stateIndex);
             stateIndex++;
@@ -1599,7 +1628,10 @@ public class BcelShadow extends Shadow {
     private IntMap makeRemap() {
         IntMap ret = new IntMap(5);
         int reti = 0;
-        if (targetVar != null) {
+		if (thisVar != null) {
+			ret.put(0, reti++);  // thisVar guaranteed to be 0
+		}
+        if (targetVar != null && targetVar != thisVar) {
             ret.put(targetVar.getSlot(), reti++);
         }            
         for (int i = 0, len = argVars.length; i < len; i++) {
@@ -1613,21 +1645,28 @@ public class BcelShadow extends Shadow {
         // aliases, which we so helpfully put into temps at the beginning of this join 
         // point.
         if (! getKind().argsOnStack()) {
-		    int index = 0;
-	        if (hasThis()) { ret.put(0, 0); index++; }
+        	int oldi = 0;
+        	int newi = 0;
+        	// if we're passing in a this and we're not argsOnStack we're always 
+        	// passing in a target too
+	        if (hasThis()) { ret.put(0, 0); oldi++; newi+=1; }
+	        //assert targetVar == thisVar
 	        for (int i = 0; i < getArgCount(); i++) {
 	            TypeX type = getArgType(i); 
-				ret.put(index, index);
-	            index += type.getSize();
+				ret.put(oldi, newi);
+	            oldi += type.getSize();
+	            newi += type.getSize();
 	        }   
         }      
         return ret;
     }
 
     /**
-     * The new method is nonStatic iff we're from nonExpression advice
-     * with an enclosing non-static method.
-     * Otherwise, it's static
+     * 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 createMethodGen(String newMethodName) {
         Type[] parameterTypes = world.makeBcelTypes(getSignature().getParameterTypes());
@@ -1638,10 +1677,23 @@ public class BcelShadow extends Shadow {
 //            modifiers |= Modifier.STRICT;
 //        }
         modifiers |= Modifier.STATIC;
-        if (hasTarget()) {
+        if (targetVar != null && targetVar != thisVar) {
             TypeX targetType = getTargetType();
+            ResolvedMember resolvedMember = getSignature().resolve(world);
+            if (resolvedMember != null && Modifier.isProtected(resolvedMember.getModifiers()) && 
+            	!samePackage(targetType.getPackageName(), getThisType().getPackageName()))
+            {
+            	if (!targetType.isAssignableFrom(getThisType(), world)) {
+            		throw new BCException("bad bytecode");
+            	}
+            	targetType = getThisType();
+            }
             parameterTypes = addType(world.makeBcelType(targetType), parameterTypes);
         }
+        if (thisVar != null) {
+        	TypeX thisType = getThisType();
+        	parameterTypes = addType(world.makeBcelType(thisType), parameterTypes);
+        }
         
         // We always want to pass down thisJoinPoint in case we have already woven
         // some advice in here.  If we only have a single piece of around advice on a
@@ -1669,6 +1721,13 @@ public class BcelShadow extends Shadow {
 //                TypeX.getNames(getSignature().getExceptions(world)),
                 getEnclosingClass());
     }
+
+	private boolean samePackage(String p1, String p2) {
+		if (p1 == null) return p2 == null;
+		if (p2 == null) return false;
+		return p1.equals(p2);
+	}
+
    
     private Type[] addType(Type type, Type[] types) {
         int len = types.length;
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
index 0a1861b47..b3b4cced8 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
@@ -318,6 +318,25 @@ public final class LazyClassGen {
         return clinit;
     }
     
+    public LazyMethodGen getAjcClinit() {
+        for (Iterator i = methodGens.iterator(); i.hasNext();) {
+            LazyMethodGen gen = (LazyMethodGen) i.next();
+			if (gen.getName().equals(NameMangler.AJC_CLINIT_NAME)) return gen;
+        } 
+        throw new RuntimeException("unimplemented");
+//        LazyMethodGen clinit = new LazyMethodGen(
+//        	Modifier.STATIC,
+//        	Type.VOID,
+//        	"<clinit>",
+//        	new Type[0],
+//        	CollectionUtil.NO_STRINGS,
+//        	this);
+//       	clinit.getBody().insert(getFactory().RETURN);
+//        methodGens.add(clinit);
+//        return clinit;
+    }
+    
+    
     
     // reflective thisJoinPoint support
     Map/*BcelShadow, Field*/ tjpFields = new HashMap();
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
index 63d105e19..35648dc5f 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyMethodGen.java
@@ -700,8 +700,11 @@ public final class LazyMethodGen {
                 if (r instanceof ExceptionRange) {
                     ExceptionRange er = (ExceptionRange) r;
                     if (er.getStart() == ih) {
-                        // order is important, insert handlers in order of start
-                        insertHandler(er, exnList);
+                    	//System.err.println("er " + er);
+                    	if (!er.isEmpty()){
+                        	// order is important, insert handlers in order of start
+                        	insertHandler(er, exnList);
+                    	}
                     }
                 } else {
                     // we must be a shadow range or something equally useless, 
diff --git a/weaver/src/org/aspectj/weaver/bcel/Range.java b/weaver/src/org/aspectj/weaver/bcel/Range.java
index 0d6eb21b6..49df80897 100644
--- a/weaver/src/org/aspectj/weaver/bcel/Range.java
+++ b/weaver/src/org/aspectj/weaver/bcel/Range.java
@@ -50,7 +50,9 @@ abstract class Range implements InstructionTargeter {
 
     boolean isEmpty() {
         InstructionHandle ih = start;
+        //System.err.println("  looking for " + end);
         while (ih != end) {
+        	//System.err.println("    ih " + ih);
             if (! Range.isRangeHandle(ih)) return false;
             ih = ih.getNext();
         }
diff --git a/weaver/testdata/AroundArgsDynamicHelloWorld.txt b/weaver/testdata/AroundArgsDynamicHelloWorld.txt
index 5ca21c9b5..0c2881392 100644
--- a/weaver/testdata/AroundArgsDynamicHelloWorld.txt
+++ b/weaver/testdata/AroundArgsDynamicHelloWorld.txt
@@ -68,15 +68,19 @@ public class DynamicHelloWorld extends java.lang.Object implements java.io.Seria
     | |             IFEQ L0
     | |             ALOAD 4
     | |             CHECKCAST java.util.ArrayList
-    | |             BIPUSH 2
+    | |             BIPUSH 3
     | |             ANEWARRAY java.lang.Object
     | |             ASTORE 5
     | |             ALOAD 5
     | |             BIPUSH 0
-    | |             ALOAD 4
+    | |             ALOAD_0
     | |             AASTORE
     | |             ALOAD 5
     | |             BIPUSH 1
+    | |             ALOAD 4
+    | |             AASTORE
+    | |             ALOAD 5
+    | |             BIPUSH 2
     | |             ALOAD_3
     | |             AASTORE
     | |             NEW DynamicHelloWorld$AjcClosure1
@@ -85,9 +89,10 @@ public class DynamicHelloWorld extends java.lang.Object implements java.io.Seria
     | |             INVOKESPECIAL DynamicHelloWorld$AjcClosure1.<init> ([Ljava/lang/Object;)V
     | |             INVOKESTATIC Aspect.ajc_around0 (Ljava/util/ArrayList;Lorg/aspectj/runtime/internal/AroundClosure;)Z
     | |             GOTO L1
-    | |         L0: ALOAD 4
+    | |         L0: ALOAD_0
+    | |             ALOAD 4
     | |             ALOAD_3
-    | |             INVOKESTATIC DynamicHelloWorld.add_aroundBody0 (Ljava/util/List;Ljava/lang/Object;)Z
+    | |             INVOKESTATIC DynamicHelloWorld.add_aroundBody0 (LDynamicHelloWorld;Ljava/util/List;Ljava/lang/Object;)Z
     | |         L1: NOP
     | method-call(boolean java.util.List.add(java.lang.Object))
     |               POP
@@ -99,12 +104,12 @@ public class DynamicHelloWorld extends java.lang.Object implements java.io.Seria
     method-execution(java.lang.String DynamicHelloWorld.doit(java.lang.String, java.util.List))
   end String doit(String, java.util.List)
 
-  static final boolean add_aroundBody0(java.util.List, Object):
-                    ALOAD_0
+  static final boolean add_aroundBody0(DynamicHelloWorld, java.util.List, Object):
                     ALOAD_1
+                    ALOAD_2
                     INVOKEINTERFACE java.util.List.add (Ljava/lang/Object;)Z   (line 21)
                     IRETURN
-  end static final boolean add_aroundBody0(java.util.List, Object)
+  end static final boolean add_aroundBody0(DynamicHelloWorld, java.util.List, Object)
 end public class DynamicHelloWorld
 
 public class DynamicHelloWorld$AjcClosure1 extends org.aspectj.runtime.internal.AroundClosure:
@@ -119,14 +124,18 @@ public class DynamicHelloWorld$AjcClosure1 extends org.aspectj.runtime.internal.
                     ALOAD_0
                     GETFIELD org.aspectj.runtime.internal.AroundClosure.state [Ljava/lang/Object;
                     ASTORE_2
+                    ALOAD_2
+                    BIPUSH 0
+                    AALOAD
+                    CHECKCAST DynamicHelloWorld
                     ALOAD_1
                     BIPUSH 0
                     AALOAD
                     CHECKCAST java.util.List
                     ALOAD_2
-                    BIPUSH 1
+                    BIPUSH 2
                     AALOAD
-                    INVOKESTATIC DynamicHelloWorld.add_aroundBody0 (Ljava/util/List;Ljava/lang/Object;)Z
+                    INVOKESTATIC DynamicHelloWorld.add_aroundBody0 (LDynamicHelloWorld;Ljava/util/List;Ljava/lang/Object;)Z
                     INVOKESTATIC org.aspectj.runtime.internal.Conversions.booleanObject (Z)Ljava/lang/Object;
                     ARETURN
   end public Object run(Object[])
diff --git a/weaver/testsrc/org/aspectj/weaver/patterns/BindingTestCase.java b/weaver/testsrc/org/aspectj/weaver/patterns/BindingTestCase.java
index d1d2b3923..d7b598f38 100644
--- a/weaver/testsrc/org/aspectj/weaver/patterns/BindingTestCase.java
+++ b/weaver/testsrc/org/aspectj/weaver/patterns/BindingTestCase.java
@@ -23,20 +23,9 @@ import junit.framework.TestCase;
 import org.aspectj.bridge.AbortException;
 import org.aspectj.weaver.*;
 
-/**
- * @author hugunin
- *
- * To change this generated comment edit the template variable "typecomment":
- * Window>Preferences>Java>Templates.
- * To enable and disable the creation of type comments go to
- * Window>Preferences>Java>Code Generation.
- */
 public class BindingTestCase extends TestCase {
 
-	/**
-	 * Constructor for ParserTestCase.
-	 * @param arg0
-	 */
+
 	public BindingTestCase(String arg0) {
 		super(arg0);
 	}
@@ -58,8 +47,12 @@ public class BindingTestCase extends TestCase {
 		checkBindings("this(*)", none);
 		checkBindings("this(a)", a);
 		
-		checkBindings("args(.., a,..,b)", all);
-		checkBindings("args(a,..,b, ..)", all);
+		try {checkBindings("args(.., a,..,b)", all);
+			//checkBindings("args(a,..,b, ..)", all);
+			fail("shouldn't be implemented yet");
+		} catch (AbortException ae) {
+			// not implemented yet
+		}
 		
 		checkBindings("args(a,..,b)", all);
 		checkBindings("args(b)", b);