From 42035aea54234894721cca2858035002c7bfa9c7 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Thu, 15 Mar 2012 10:03:03 -0700 Subject: [PATCH] pr73507 - wip --- .../org/aspectj/weaver/AjcMemberMaker.java | 9 ++-- .../src/org/aspectj/weaver/ResolvedType.java | 32 +++++++++--- tests/bugs170/pr73507/Case1.java | 20 ++++++++ tests/bugs170/pr73507/Case2.java | 19 +++++++ tests/bugs170/pr73507/Case3.java | 27 ++++++++++ tests/bugs170/pr73507/Case4.java | 18 +++++++ .../systemtest/ajc170/Ajc170Tests.java | 17 +++++++ .../org/aspectj/systemtest/ajc170/ajc170.xml | 35 +++++++++++++ .../aspectj/weaver/bcel/BcelClassWeaver.java | 9 +--- .../aspectj/weaver/bcel/BcelTypeMunger.java | 49 +++++++++++-------- .../org/aspectj/weaver/bcel/LazyClassGen.java | 18 +++++++ 11 files changed, 215 insertions(+), 38 deletions(-) create mode 100644 tests/bugs170/pr73507/Case1.java create mode 100644 tests/bugs170/pr73507/Case2.java create mode 100644 tests/bugs170/pr73507/Case3.java create mode 100644 tests/bugs170/pr73507/Case4.java 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 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 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 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 getFieldGens() { return fields; } + + public boolean fieldExists(String name) { +// Field[] allFields = myGen.getFields(); +// if (allFields!=null) { +// for (int i=0;i Short.MAX_VALUE) { -- 2.39.5