From 7218c96f13f61055bf2952778fc0b44848e43663 Mon Sep 17 00:00:00 2001 From: aclement Date: Fri, 11 Nov 2005 13:39:57 +0000 Subject: [PATCH] YES! Bridge method code all in !! hurrah!! See pr108101 --- .../systemtest/ajc150/GenericsTests.java | 49 +++-- .../aspectj/weaver/bcel/BcelClassWeaver.java | 206 ++++++++++++------ .../org/aspectj/weaver/bcel/BcelWeaver.java | 31 ++- 3 files changed, 193 insertions(+), 93 deletions(-) diff --git a/tests/src/org/aspectj/systemtest/ajc150/GenericsTests.java b/tests/src/org/aspectj/systemtest/ajc150/GenericsTests.java index 0942cfcfe..8e10c9e17 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/GenericsTests.java +++ b/tests/src/org/aspectj/systemtest/ajc150/GenericsTests.java @@ -458,12 +458,12 @@ public class GenericsTests extends XMLBasedAjcTestCase { "java.lang.Object Sub3.m() [BridgeMethod]"}); } // Now the subclass ITD is done with binary weaving - the weaver should create the necessary bridge method -// public void testGenericITDsBridgeMethods3binary() { -// runTest("bridge methods - 3 - binary"); -// checkMethodsExist("Sub3",new String[]{ -// "java.lang.Integer Sub3.m()", -// "java.lang.Object Sub3.m() [BridgeMethod]"}); -// } + public void testGenericITDsBridgeMethods3binary() { + runTest("bridge methods - 3 - binary"); + checkMethodsExist("Sub3",new String[]{ + "java.lang.Integer Sub3.m()", + "java.lang.Object Sub3.m() [BridgeMethod]"}); + } // Now the two types are disconnected until the aspect supplies a declare parents relationship - // the bridge method should still be created in the subtype public void testGenericITDSBridgeMethods4() { @@ -473,14 +473,35 @@ public class GenericsTests extends XMLBasedAjcTestCase { "java.lang.Object Sub4.m() [BridgeMethod]"}); } // now the aspect doing the decp between the types is applied via binary weaving - weaver should create the bridge method -// public void testGenericITDSBridgeMethods4binary() { -// runTest("bridge methods - 4 - binary"); -// checkMethodsExist("Sub4",new String[]{ -// "java.lang.Integer Sub4.m()", -// "java.lang.Object Sub4.m() [BridgeMethod]"}); -// } + public void testGenericITDSBridgeMethods4binary() { + runTest("bridge methods - 4 - binary"); + checkMethodsExist("Sub4",new String[]{ + "java.lang.Integer Sub4.m()", + "java.lang.Object Sub4.m() [BridgeMethod]"}); + } + + public void testBinaryBridgeMethodsOne() { + runTest("binary bridge methods - one"); + checkMethodsExist("OneB",new String[]{ + "java.lang.Number OneB.firstMethod() [BridgeMethod]", + "java.lang.Integer OneB.firstMethod()", + "void OneB.secondMethod(java.lang.Number) [BridgeMethod]", + "void OneB.secondMethod(java.lang.Integer)", + "void OneB.thirdMethod(java.lang.Number,java.lang.Number) [BridgeMethod]", + "void OneB.thirdMethod(java.lang.Integer,java.lang.Integer)", + "void OneB.fourthMethod(java.util.List)", + "java.lang.Number OneB.fifthMethod(java.lang.Number,java.util.List) [BridgeMethod]", + "java.lang.Integer OneB.fifthMethod(java.lang.Integer,java.util.List)" + }); + } + public void testBinaryBridgeMethodsTwo() { + runTest("binary bridge methods - two"); + checkMethodsExist("TwoB",new String[]{ + "java.lang.Number TwoB.firstMethod(java.io.Serializable) [BridgeMethod]", + "java.lang.Integer TwoB.firstMethod(java.lang.String)" + }); + } - // FIXME asc need a testcase for when a decp wires two classes together and causes the subtype to need bridge methods public void testGenericITDsBridgeMethodsPR91381() {runTest("abstract intertype methods and covariant returns");} public void testGenericITDsBridgeMethodsPR91381_2() {runTest("abstract intertype methods and covariant returns - error");} @@ -836,7 +857,7 @@ public class GenericsTests extends XMLBasedAjcTestCase { ms[i].getName()+"("+stringify(ms[i].getParameterTypes())+")"+ (isBridge(ms[i])?" [BridgeMethod]":""); methodsFound.add(methodString); - debugString.append("[").append(methodString).append("]"); + debugString.append("\n[").append(methodString).append("]"); } } } catch (ClassNotFoundException e) { diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java index 4166322e6..4003a80b3 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelClassWeaver.java @@ -163,7 +163,8 @@ class BcelClassWeaver implements IClassWeaver { initializeSuperInitializerMap(ty.getResolvedTypeX()); } - + + private List[] perKindShadowMungers; private boolean canMatchBodyShadows = false; private boolean canMatchInitialization = false; @@ -323,7 +324,7 @@ class BcelClassWeaver implements IClassWeaver { } - protected LazyMethodGen makeBridgeMethod(LazyClassGen gen, ResolvedMember member) { + protected static LazyMethodGen makeBridgeMethod(LazyClassGen gen, ResolvedMember member) { // remove abstract modifier int mods = member.getModifiers(); @@ -343,11 +344,10 @@ class BcelClassWeaver implements IClassWeaver { } - // FIXME asc doesnt cope with parameter variations (see commented out code below) /** * Create a single bridge method called 'theBridgeMethod' that bridges to 'whatToBridgeTo' */ - private void createBridgeMethod(BcelWorld world, ResolvedMember whatToBridgeTo, LazyClassGen clazz,ResolvedMember theBridgeMethod) { + private static void createBridgeMethod(BcelWorld world, ResolvedMember whatToBridgeTo, LazyClassGen clazz,ResolvedMember theBridgeMethod) { InstructionList body; InstructionFactory fact; int pos = 0; @@ -357,6 +357,7 @@ class BcelClassWeaver implements IClassWeaver { UnresolvedType[] newParams = whatToBridgeTo.getParameterTypes(); Type returnType = BcelWorld.makeBcelType(theBridgeMethod.getReturnType()); Type[] paramTypes = BcelWorld.makeBcelTypes(theBridgeMethod.getParameterTypes()); + Type[] newParamTypes=BcelWorld.makeBcelTypes(newParams); body = bridgeMethod.getBody(); fact = clazz.getFactory(); @@ -367,10 +368,10 @@ class BcelClassWeaver implements IClassWeaver { for (int i = 0, len = paramTypes.length; i < len; i++) { Type paramType = paramTypes[i]; body.append(InstructionFactory.createLoad(paramType, pos)); -// if (!bridgingSetter.getParameterTypes()[i].getErasureSignature().equals(unMangledInterMethod.getParameterTypes()[i].getErasureSignature())) { -// System.err.println("Putting in cast from "+paramType+" to "+bridgingToParms[i]); -// body.append(fact.createCast(paramType,bridgingToParms[i])); -// } + if (!newParamTypes[i].equals(paramTypes[i])) { + if (debug) System.err.println("Cast "+newParamTypes[i]+" from "+paramTypes[i]); + body.append(fact.createCast(paramTypes[i],newParamTypes[i])); + } pos+=paramType.getSize(); } @@ -413,8 +414,6 @@ class BcelClassWeaver implements IClassWeaver { } } - // Assumption: at this point we've done all necessary type munging - calculateAnyRequiredBridgeMethods(); // Weave special half type/half shadow mungers... @@ -505,13 +504,60 @@ class BcelClassWeaver implements IClassWeaver { } - // FIXME asc no doubt incomplete... + + + // **************************** start of bridge method creation code ***************** + + // debug flag for bridge method creation + private static boolean debug=false; + + + // FIXME asc tidy this lot up !! + + // FIXME asc refactor into ResolvedType or even ResolvedMember? + /** + * Check if a particular method is overriding another - refactored into this helper so it + * can be used from multiple places. + */ + private static ResolvedMember isOverriding(ResolvedType typeToCheck,ResolvedMember methodThatMightBeGettingOverridden,String mname,String mrettype,int mmods,boolean inSamePackage,UnresolvedType[] methodParamsArray) { + // Check if we can be an override... + if (methodThatMightBeGettingOverridden.isStatic()) return null; // we can't be overriding a static method + if (methodThatMightBeGettingOverridden.isPrivate()) return null; // we can't be overriding a private method + if (!methodThatMightBeGettingOverridden.getName().equals(mname)) return null; // names dont match (this will also skip and too) + if (methodThatMightBeGettingOverridden.getParameterTypes().length!=methodParamsArray.length) return null; // check same number of parameters + if (!isVisibilityOverride(mmods,methodThatMightBeGettingOverridden,inSamePackage)) return null; + + if (debug) System.err.println(" Seriously considering this might be getting overridden "+methodThatMightBeGettingOverridden); + + // Look at erasures of parameters (List erased is List) + boolean sameParams = true; + for (int p = 0;p lt, T t)' and the parameterized type here + // is I then the method we are looking at will be 'void m(List lt, String t)' which when erased + // is 'void m(List lt,String t)' - so if the parameters *do* match then there is a generic method we are + // overriding + + if (sameParams) { + // check for covariance + if (typeToCheck.isParameterizedType()) { + return methodThatMightBeGettingOverridden.getBackingGenericMember(); + } else if (!methodThatMightBeGettingOverridden.getReturnType().getErasureSignature().equals(mrettype)) { + return methodThatMightBeGettingOverridden; // covariance + } + } + return null; + } + /** * Looks at the visibility modifiers between two methods, and knows whether they are from classes in * the same package, and decides whether one overrides the other. * @return true if there is an overrides rather than a 'hides' relationship */ - boolean isVisibilityOverride(int methodMods, ResolvedMember inheritedMethod,boolean inSamePackage) { + static boolean isVisibilityOverride(int methodMods, ResolvedMember inheritedMethod,boolean inSamePackage) { if (inheritedMethod.isStatic()) return false; if (methodMods == inheritedMethod.getModifiers()) return true; @@ -529,93 +575,101 @@ class BcelClassWeaver implements IClassWeaver { * * @return the method being overridden or null if none is found */ - public ResolvedMember checkForOverride(ResolvedType type,String methodname,String methodparams,String methodreturntype,int methodmodifiers,String methodpackage) { - // check this type, then check its supertypes - if (type==null) return null; - if (type instanceof MissingResolvedTypeWithKnownSignature) return null; // we just can't tell ! + public static ResolvedMember checkForOverride(ResolvedType typeToCheck,String mname,String mparams,String mrettype,int mmods,String mpkg,UnresolvedType[] methodParamsArray) { - String packageName = type.getPackageName(); + if (typeToCheck==null) return null; + if (typeToCheck instanceof MissingResolvedTypeWithKnownSignature) return null; // we just can't tell ! + + if (debug) System.err.println(" Checking for override of "+mname+" in "+typeToCheck); + + String packageName = typeToCheck.getPackageName(); if (packageName==null) packageName=""; - boolean inSamePackage = packageName.equals(methodpackage); // used when looking at visibility rules + boolean inSamePackage = packageName.equals(mpkg); // used when looking at visibility rules - for (Iterator iter = type.getMethods();iter.hasNext();) { - ResolvedMember aMethod = (ResolvedMember) iter.next(); - if (aMethod.isStatic()) continue; - if (!aMethod.isPrivate() && aMethod.getName().equals(methodname) && aMethod.getParameterSignature().equals(methodparams)) { - // visibility checks - if (isVisibilityOverride(methodmodifiers,aMethod,inSamePackage)) { - // check the return types, if they are different we need a bridging method. - if (!aMethod.getReturnType().getErasureSignature().equals(methodreturntype) && !Modifier.isPrivate(aMethod.getModifiers())) { - return aMethod; - } - } + ResolvedMember [] methods = typeToCheck.getDeclaredMethods(); + for (int ii=0;ii")) continue; // Skip constructors and static initializers + if (bridgeToCandidate.isStatic()) continue; // ignore static methods + if (name.endsWith("init>")) continue; // Skip constructors and static initializers - if (debug) System.err.println("Do we need to bridge to "+localMethodName+""+localMethod.getSignature()); - if (debug) System.err.println("Checking supertype "+clazz.getSuperClassname()); + if (debug) System.err.println("Determining if we have to bridge to "+clazz.getName()+"."+name+""+bridgeToCandidate.getSignature()); - // Check superclass - ResolvedType theSuperclass= world.resolve(clazz.getSuperClassname()); - ResolvedMember overriddenMethod = checkForOverride(theSuperclass,localMethodName,localParameterSig,localReturnTypeSig,localMethod.getAccessFlags(),clazz.getPackageName()); + // Let's take a look at the superclass + ResolvedType theSuperclass= clazz.getSuperClass(); + if (debug) System.err.println("Checking supertype "+theSuperclass); + String pkgName = clazz.getPackageName(); + UnresolvedType[] bm = BcelWorld.fromBcel(bridgeToCandidate.getArgumentTypes()); + ResolvedMember overriddenMethod = checkForOverride(theSuperclass,name,psig,rsig,bridgeToCandidate.getAccessFlags(),pkgName,bm); if (overriddenMethod!=null) { - boolean alreadyHaveABridgeMethod = localMethodsSet.contains(overriddenMethod.getName()+overriddenMethod.getSignature()); + boolean alreadyHaveABridgeMethod = methodsSet.contains(overriddenMethod.getName()+overriddenMethod.getSignature()); if (!alreadyHaveABridgeMethod) { - createBridgeMethod(world, localMethod.getMemberView(), clazz, overriddenMethod); if (debug) System.err.println("Bridging to "+overriddenMethod); - continue; + createBridgeMethod(world, bridgeToCandidate.getMemberView(), clazz, overriddenMethod); + didSomething = true; + continue; // look at the next method } } @@ -624,19 +678,24 @@ class BcelClassWeaver implements IClassWeaver { for (int j = 0; j < interfaces.length; j++) { if (debug) System.err.println("Checking superinterface "+interfaces[j]); ResolvedType interfaceType = world.resolve(interfaces[j]); - overriddenMethod = checkForOverride(interfaceType,localMethodName,localParameterSig,localReturnTypeSig,localMethod.getAccessFlags(),clazz.getPackageName()); + overriddenMethod = checkForOverride(interfaceType,name,psig,rsig,bridgeToCandidate.getAccessFlags(),clazz.getPackageName(),bm); if (overriddenMethod!=null) { - boolean alreadyHaveABridgeMethod = localMethodsSet.contains(overriddenMethod.getName()+overriddenMethod.getSignature()); + boolean alreadyHaveABridgeMethod = methodsSet.contains(overriddenMethod.getName()+overriddenMethod.getSignature()); if (!alreadyHaveABridgeMethod) { - createBridgeMethod(world, localMethod.getMemberView(), clazz, overriddenMethod); + createBridgeMethod(world, bridgeToCandidate.getMemberView(), clazz, overriddenMethod); + didSomething=true; if (debug) System.err.println("Bridging to "+overriddenMethod); - continue; + continue; // look at the next method } } } } + + return didSomething; } - + + // **************************** end of bridge method creation code ***************** + /** * Weave any declare @method/@ctor statements into the members of the supplied class */ @@ -2098,5 +2157,10 @@ class BcelClassWeaver implements IClassWeaver { public static boolean getReweavableMode() { return inReweavableMode; } + + public String toString() { + return "BcelClassWeaver instance for : "+clazz; + } + } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java index 8d730932c..d85743bb0 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelWeaver.java @@ -1379,7 +1379,7 @@ public class BcelWeaver implements IWeaver { // check the annotation is suitable for the target boolean problemReported = verifyTargetIsOK(decA, onType, annoX,reportProblems); - + if (!problemReported) { AsmRelationshipProvider.getDefault().addDeclareAnnotationRelationship(decA.getSourceLocation(),onType.getSourceLocation()); // TAG: WeavingMessage @@ -1488,26 +1488,41 @@ public class BcelWeaver implements IWeaver { return ret; } - private LazyClassGen weave(UnwovenClassFile classFile, BcelObjectType classType, boolean dump) throws IOException { - if (classType.isSynthetic()) { + if (classType.isSynthetic()) { // Don't touch synthetic classes if (dump) dumpUnchanged(classFile); return null; } -// JavaClass javaClass = classType.getJavaClass(); List shadowMungers = fastMatch(shadowMungerList, classType.getResolvedTypeX()); - List typeMungers = classType.getResolvedTypeX().getInterTypeMungers(); + List typeMungers = classType.getResolvedTypeX().getInterTypeMungers(); classType.getResolvedTypeX().checkInterTypeMungers(); + // Decide if we need to do actual weaving for this class + boolean mightNeedToWeave = + shadowMungers.size() > 0 || + typeMungers.size() > 0 || + classType.isAspect() || + world.getDeclareAnnotationOnMethods().size()>0 || + world.getDeclareAnnotationOnFields().size()>0; + + // May need bridge methods if on 1.5 and something in our hierarchy is affected by ITDs + boolean mightNeedBridgeMethods = world.isInJava5Mode() && classType.getResolvedTypeX().getInterTypeMungersIncludingSupers().size()>0; + LazyClassGen clazz = null; - if (shadowMungers.size() > 0 || typeMungers.size() > 0 || classType.isAspect() || - world.getDeclareAnnotationOnMethods().size()>0 || world.getDeclareAnnotationOnFields().size()>0 ) { + if (mightNeedToWeave || mightNeedBridgeMethods) { clazz = classType.getLazyClassGen(); //System.err.println("got lazy gen: " + clazz + ", " + clazz.getWeaverState()); try { - boolean isChanged = BcelClassWeaver.weave(world, clazz, shadowMungers, typeMungers, lateTypeMungerList); + boolean isChanged = false; + + if (mightNeedToWeave) + isChanged = BcelClassWeaver.weave(world, clazz, shadowMungers, typeMungers, lateTypeMungerList); + + if (mightNeedBridgeMethods) + isChanged = BcelClassWeaver.calculateAnyRequiredBridgeMethods(world,clazz) || isChanged; + if (isChanged) { if (dump) dump(classFile, clazz); return clazz; -- 2.39.5