pr73507 - wip

11 files changed, 215 insertions, 38 deletions

diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/AjcMemberMaker.java b/org.aspectj.matcher/src/org/aspectj/weaver/AjcMemberMaker.java
index 8874dcd03..fa3ff3379 100644
--- a/org.aspectj.matcher/src/org/aspectj/weaver/AjcMemberMaker.java
+++ b/org.aspectj.matcher/src/org/aspectj/weaver/AjcMemberMaker.java
@@ -486,9 +486,12 @@ public class AjcMemberMaker {
 	 * This field goes on top-most implementers of the interface the field is declared onto
 	 */
 	public static ResolvedMember interFieldInterfaceField(ResolvedMember field, UnresolvedType onClass, UnresolvedType aspectType) {
-		return new ResolvedMemberImpl(Member.FIELD, onClass, makePublicNonFinal(field.getModifiers()), field.getReturnType(),
-				NameMangler.interFieldInterfaceField(aspectType, field.getDeclaringType(), field.getName()), UnresolvedType.NONE,
-				UnresolvedType.NONE);
+//		return new ResolvedMemberImpl(Member.FIELD, onClass, makePublicNonFinal(field.getModifiers()), field.getReturnType(),
+//				NameMangler.interFieldInterfaceField(aspectType, field.getDeclaringType(), field.getName()), UnresolvedType.NONE,
+//				UnresolvedType.NONE);
+
+		// TODO what about non public fields, can you have those?
+		return interFieldClassField(field,aspectType,true);
 	}
 
 	/**
diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/ResolvedType.java b/org.aspectj.matcher/src/org/aspectj/weaver/ResolvedType.java
index 7b904778f..0bba33032 100644
--- a/org.aspectj.matcher/src/org/aspectj/weaver/ResolvedType.java
+++ b/org.aspectj.matcher/src/org/aspectj/weaver/ResolvedType.java
@@ -674,8 +674,8 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 			return false;
 		}
 
-		if (m1.getKind() == Member.FIELD) {
-			return m1.getDeclaringType().equals(m2.getDeclaringType());
+		if (m1.getKind() == Member.FIELD && m1.getDeclaringType().equals(m2.getDeclaringType())) {
+			return true ;
 		} else if (m1.getKind() == Member.POINTCUT) {
 			return true;
 		}
@@ -1735,12 +1735,13 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 			}
 		}
 
+		boolean needsAdding = true;
+		boolean needsToBeAddedEarlier = false;
 		// now compare to existingMungers
 		for (Iterator<ConcreteTypeMunger> i = interTypeMungers.iterator(); i.hasNext();) {
 			ConcreteTypeMunger existingMunger = i.next();
 			if (conflictingSignature(existingMunger.getSignature(), munger.getSignature())) {
-				// System.err.println("match " + munger + " with " +
-				// existingMunger);
+				// System.err.println("match " + munger + " with " + existingMunger);
 				if (isVisible(munger.getSignature().getModifiers(), munger.getAspectType(), existingMunger.getAspectType())) {
 					// System.err.println("    is visible");
 					int c = compareMemberPrecedence(sig, existingMunger.getSignature());
@@ -1751,11 +1752,22 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 					if (c < 0) {
 						// the existing munger dominates the new munger
 						checkLegalOverride(munger.getSignature(), existingMunger.getSignature(), 0x11, null);
-						return;
+						needsAdding = false;
+//						return;
+						if (munger.getSignature().getKind()==Member.FIELD && 
+								munger.getSignature().getDeclaringType().resolve(world).isInterface()) {
+							needsAdding=true;
+						}
+						break;
 					} else if (c > 0) {
 						// the new munger dominates the existing one
 						checkLegalOverride(existingMunger.getSignature(), munger.getSignature(), 0x11, null);
-						i.remove();
+						if (munger.getSignature().getKind()==Member.FIELD && 
+								munger.getSignature().getDeclaringType().resolve(world).isInterface()) {
+							needsToBeAddedEarlier = true;
+						} else {
+							i.remove();
+						}
 						break;
 					} else {
 						interTypeConflictError(munger, existingMunger);
@@ -1769,7 +1781,13 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 		// we are adding the parameterized form of the ITD to the list of
 		// mungers. Within it, the munger knows the original declared
 		// signature for the ITD so it can be retrieved.
-		interTypeMungers.add(munger);
+		if (needsAdding) {
+			if (!needsToBeAddedEarlier) {
+				interTypeMungers.add(munger);				
+			} else {
+				interTypeMungers.add(0,munger);
+			}
+		}
 	}
 
 	/**
diff --git a/tests/bugs170/pr73507/Case1.java b/tests/bugs170/pr73507/Case1.java
new file mode 100644
index 000000000..74a645851
--- /dev/null
+++ b/tests/bugs170/pr73507/Case1.java
@@ -0,0 +1,20 @@
+import java.lang.reflect.*;
+
+interface I {
+}
+
+
+class C implements I {
+}
+
+public aspect Case1 {
+
+  public int I.i;
+
+  public static void main(String []argv) throws Exception {
+    Field f =  C.class.getField("i");
+    if (f==null) System.out.println("Couldn't find a field called i");
+    else         System.out.println("Found a field called i");
+  }
+  
+}
diff --git a/tests/bugs170/pr73507/Case2.java b/tests/bugs170/pr73507/Case2.java
new file mode 100644
index 000000000..15ba1665f
--- /dev/null
+++ b/tests/bugs170/pr73507/Case2.java
@@ -0,0 +1,19 @@
+import java.lang.reflect.*;
+
+interface I {
+}
+
+
+class C implements I {
+  public int i = 1;
+}
+
+public aspect Case2 {
+
+  public int I.i = 5;
+
+  public static void main(String []argv) {
+    System.out.println("Value of C.i is "+new C().i);
+  }
+  
+}
diff --git a/tests/bugs170/pr73507/Case3.java b/tests/bugs170/pr73507/Case3.java
new file mode 100644
index 000000000..588d97dc2
--- /dev/null
+++ b/tests/bugs170/pr73507/Case3.java
@@ -0,0 +1,27 @@
+import java.lang.reflect.*;
+
+interface I {
+}
+
+
+class C implements I {
+}
+
+public aspect Case3 {
+
+  // one order
+  public int C.i = 1;
+  public int I.i = 5;
+
+  // the other order ;)
+  public int I.j = 5;
+  public int C.j = 1;
+
+  public static void main(String []argv) {
+    System.out.println("Value of C.i is "+new C().i);
+    System.out.println("Value of C.j is "+new C().j);
+    System.out.println("Value of I.i is "+((I)new C()).i);
+    System.out.println("Value of I.j is "+((I)new C()).j);
+  }
+  
+}
diff --git a/tests/bugs170/pr73507/Case4.java b/tests/bugs170/pr73507/Case4.java
new file mode 100644
index 000000000..08912fbc1
--- /dev/null
+++ b/tests/bugs170/pr73507/Case4.java
@@ -0,0 +1,18 @@
+import java.lang.reflect.*;
+
+interface I {
+}
+
+
+class C implements I {
+  public int i = 1;
+}
+
+public aspect Case4 {
+
+  public String I.i = "hello";
+
+  public static void main(String []argv) {
+  }
+  
+}
diff --git a/tests/src/org/aspectj/systemtest/ajc170/Ajc170Tests.java b/tests/src/org/aspectj/systemtest/ajc170/Ajc170Tests.java
index bfb995d12..ebdf20c3e 100644
--- a/tests/src/org/aspectj/systemtest/ajc170/Ajc170Tests.java
+++ b/tests/src/org/aspectj/systemtest/ajc170/Ajc170Tests.java
@@ -25,6 +25,23 @@ import org.aspectj.weaver.UnresolvedType;
  */ 
 public class Ajc170Tests extends org.aspectj.testing.XMLBasedAjcTestCase {
 
+
+	public void testPublicITDFs_pr73507_1() {
+		runTest("public ITDfs - 1");
+	}
+
+	public void testPublicITDFs_pr73507_2() {
+		runTest("public ITDfs - 2");
+	}
+
+	public void testPublicITDFs_pr73507_3() {
+		runTest("public ITDfs - 3");
+	}
+
+	public void testPublicITDFs_pr73507_4() {
+		runTest("public ITDfs - 4");
+	}
+
 	public void testBCExceptionAnnoDecp_371998() {
 		runTest("BCException anno decp");
     }
diff --git a/tests/src/org/aspectj/systemtest/ajc170/ajc170.xml b/tests/src/org/aspectj/systemtest/ajc170/ajc170.xml
index b5ce9623c..58c936905 100644
--- a/tests/src/org/aspectj/systemtest/ajc170/ajc170.xml
+++ b/tests/src/org/aspectj/systemtest/ajc170/ajc170.xml
@@ -2,6 +2,41 @@
 
 <suite>
 
+ 
+    <ajc-test dir="bugs170/pr73507" title="public ITDfs - 1">
+      <compile files="Case1.java" options="-1.5"/>
+      <run class="Case1">
+        <stdout>
+          <line text="Found a field called i"/>
+        </stdout>
+      </run>
+    </ajc-test>
+    
+    <ajc-test dir="bugs170/pr73507" title="public ITDfs - 2">
+      <compile files="Case2.java" options="-1.5"/>
+      <run class="Case2">
+        <stdout>
+          <line text="Value of C.i is 1"/>
+        </stdout>
+      </run>
+    </ajc-test>
+    
+     <ajc-test dir="bugs170/pr73507" title="public ITDfs - 3">
+      <compile files="Case3.java" options="-1.5"/>
+      <run class="Case3">
+        <stdout>
+          <line text="Value of C.i is 1"/>
+          <line text="Value of C.j is 1"/>
+        </stdout>
+      </run>
+    </ajc-test>
+    
+     <ajc-test dir="bugs170/pr73507" title="public ITDfs - 4">
+      <compile files="Case4.java" options="-1.5">
+        <message kind="error" line="13" text="can't override java.lang.String I.i with int C.i return types don't match"/>
+      </compile>
+    </ajc-test>
+
    <ajc-test dir="bugs170/pr371998" title="BCException anno decp">
      <compile files="AspectTest.java" options="-1.5 -showWeaveInfo">
    	   <message kind="weave" text="Extending interface set for type 'Foo' (AspectTest.java) to include 'X' (AspectTest.java)"/>
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
index a6f2c0fe6..59fb837c0 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java
@@ -717,13 +717,8 @@ class BcelClassWeaver implements IClassWeaver {
 
 		ResolvedMember[] methods = typeToCheck.getDeclaredMethods();
 		for (int ii = 0; ii < methods.length; ii++) {
-			ResolvedMember methodThatMightBeGettingOverridden = methods[ii]; // the
-			// method
-			// we
-			// are
-			// going
-			// to
-			// check
+			// the method we are going to check
+			ResolvedMember methodThatMightBeGettingOverridden = methods[ii]; 
 			ResolvedMember isOverriding = isOverriding(typeToCheck, methodThatMightBeGettingOverridden, mname, mrettype, mmods,
 					inSamePackage, methodParamsArray);
 			if (isOverriding != null) {
diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java b/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java
index fcf61a085..4211475f4 100644
--- a/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java
+++ b/weaver/src/org/aspectj/weaver/bcel/BcelTypeMunger.java
@@ -24,6 +24,7 @@ import java.util.Set;
 
 import org.aspectj.apache.bcel.Constants;
 import org.aspectj.apache.bcel.classfile.ConstantPool;
+import org.aspectj.apache.bcel.classfile.Field;
 import org.aspectj.apache.bcel.classfile.Signature;
 import org.aspectj.apache.bcel.classfile.annotation.AnnotationGen;
 import org.aspectj.apache.bcel.generic.FieldGen;
@@ -1880,6 +1881,8 @@ public class BcelTypeMunger extends ConcreteTypeMunger {
 				LazyMethodGen mg1 = makeMethodGen(gen, AjcMemberMaker.interFieldInterfaceSetter(field, onType, aspectType));
 				gen.addMethodGen(mg1);
 			} else {
+				if (gen.fieldExists(field.getName())) return false;
+				
 				weaver.addInitializer(this);
 				ResolvedMember newField = AjcMemberMaker.interFieldClassField(field, aspectType,
 						munger.version == NewFieldTypeMunger.VersionTwo);
@@ -1909,29 +1912,33 @@ public class BcelTypeMunger extends ConcreteTypeMunger {
 			}
 			return true;
 		} else if (onInterface && gen.getType().isTopmostImplementor(onType)) {
-			// wew know that we can't be static since we don't allow statics on
-			// interfaces
+			// we know that we can't be static since we don't allow statics on interfaces
 			if (Modifier.isStatic(field.getModifiers())) {
 				throw new RuntimeException("unimplemented");
 			}
-			weaver.addInitializer(this);
-			// System.err.println("impl body on " + gen.getType() + " for " +
-			// munger);
-
+			
+			
 			Type fieldType = BcelWorld.makeBcelType(field.getType());
-
-			FieldGen fg = makeFieldGen(gen, AjcMemberMaker.interFieldInterfaceField(field, onType, aspectType));
-
-			if (annotationsOnRealMember != null) {
-				for (int i = 0; i < annotationsOnRealMember.length; i++) {
-					AnnotationAJ annotationX = annotationsOnRealMember[i];
-					AnnotationGen a = ((BcelAnnotation) annotationX).getBcelAnnotation();
-					AnnotationGen ag = new AnnotationGen(a, weaver.getLazyClassGen().getConstantPool(), true);
-					fg.addAnnotation(ag);
+			String fieldname = field.getName();
+			if (!gen.fieldExists(fieldname)) {
+				weaver.addInitializer(this);
+				// System.err.println("impl body on " + gen.getType() + " for " +
+				// munger);
+	
+	
+				FieldGen fg = makeFieldGen(gen, AjcMemberMaker.interFieldInterfaceField(field, onType, aspectType));
+	
+				if (annotationsOnRealMember != null) {
+					for (int i = 0; i < annotationsOnRealMember.length; i++) {
+						AnnotationAJ annotationX = annotationsOnRealMember[i];
+						AnnotationGen a = ((BcelAnnotation) annotationX).getBcelAnnotation();
+						AnnotationGen ag = new AnnotationGen(a, weaver.getLazyClassGen().getConstantPool(), true);
+						fg.addAnnotation(ag);
+					}
 				}
+				fieldname = fg.getName();
+				gen.addField(fg, getSourceLocation());
 			}
-
-			gen.addField(fg, getSourceLocation());
 			// this uses a shadow munger to add init method to constructors
 			// weaver.getShadowMungers().add(makeInitCallShadowMunger(initMethod)
 			// );
@@ -1941,10 +1948,10 @@ public class BcelTypeMunger extends ConcreteTypeMunger {
 			InstructionList il = new InstructionList();
 			InstructionFactory fact = gen.getFactory();
 			if (Modifier.isStatic(field.getModifiers())) {
-				il.append(fact.createFieldAccess(gen.getClassName(), fg.getName(), fieldType, Constants.GETSTATIC));
+				il.append(fact.createFieldAccess(gen.getClassName(), fieldname, fieldType, Constants.GETSTATIC));
 			} else {
 				il.append(InstructionConstants.ALOAD_0);
-				il.append(fact.createFieldAccess(gen.getClassName(), fg.getName(), fieldType, Constants.GETFIELD));
+				il.append(fact.createFieldAccess(gen.getClassName(), fieldname, fieldType, Constants.GETFIELD));
 			}
 			il.append(InstructionFactory.createReturn(fieldType));
 			mg.getBody().insert(il);
@@ -1974,11 +1981,11 @@ public class BcelTypeMunger extends ConcreteTypeMunger {
 			InstructionList il1 = new InstructionList();
 			if (Modifier.isStatic(field.getModifiers())) {
 				il1.append(InstructionFactory.createLoad(fieldType, 0));
-				il1.append(fact.createFieldAccess(gen.getClassName(), fg.getName(), fieldType, Constants.PUTSTATIC));
+				il1.append(fact.createFieldAccess(gen.getClassName(), fieldname, fieldType, Constants.PUTSTATIC));
 			} else {
 				il1.append(InstructionConstants.ALOAD_0);
 				il1.append(InstructionFactory.createLoad(fieldType, 1));
-				il1.append(fact.createFieldAccess(gen.getClassName(), fg.getName(), fieldType, Constants.PUTFIELD));
+				il1.append(fact.createFieldAccess(gen.getClassName(), fieldname, fieldType, Constants.PUTFIELD));
 			}
 			il1.append(InstructionFactory.createReturn(Type.VOID));
 			mg1.getBody().insert(il1);
diff --git a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
index 606bd2a96..d8a9c0e0e 100644
--- a/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
+++ b/weaver/src/org/aspectj/weaver/bcel/LazyClassGen.java
@@ -433,6 +433,24 @@ public final class LazyClassGen {
 	public List<BcelField> getFieldGens() {
 		return fields;
 	}
+	
+	public boolean fieldExists(String name) {
+//		Field[] allFields = myGen.getFields();
+//		if (allFields!=null) { 
+//			for (int i=0;i<allFields.length;i++) {
+//				Field f = allFields[i];
+//				if (f.getName().equals(name)) {
+//					return f;
+//				}
+//			}
+//		}
+		for (BcelField f: fields) {
+			if (f.getName().equals(name)) {
+				return true;
+			}
+		}
+		return false;
+	}
 
 	private void writeBack(BcelWorld world) {
 		if (getConstantPool().getSize() > Short.MAX_VALUE) {