From c9a60e519d73bb7aa4d8cf4615445089202bd3ad Mon Sep 17 00:00:00 2001 From: aclement Date: Mon, 27 Mar 2006 13:42:23 +0000 Subject: [PATCH] test and fix for 133307 - funky type variable bounds. --- .../compiler/lookup/AjLookupEnvironment.java | 1 + tests/bugs151/pr133307/Broken.aj | 7 + .../org/aspectj/systemtest/ajc150/ajc150.xml | 3 +- .../systemtest/ajc151/Ajc151Tests.java | 5 +- .../org/aspectj/systemtest/ajc151/ajc151.xml | 4 + .../weaver/CrosscuttingMembersSet.java | 27 ++++ .../patterns/IVerificationRequired.java | 22 +++ .../weaver/patterns/WildTypePattern.java | 136 +++++++++++++----- 8 files changed, 169 insertions(+), 36 deletions(-) create mode 100644 tests/bugs151/pr133307/Broken.aj create mode 100644 weaver/src/org/aspectj/weaver/patterns/IVerificationRequired.java diff --git a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java index df36ecebb..e562f3538 100644 --- a/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java +++ b/org.aspectj.ajdt.core/src/org/aspectj/ajdt/internal/compiler/lookup/AjLookupEnvironment.java @@ -254,6 +254,7 @@ public class AjLookupEnvironment extends LookupEnvironment implements AnonymousC stepCompleted = BUILD_FIELDS_AND_METHODS; lastCompletedUnitIndex = lastUnitIndex; AsmManager.setCompletingTypeBindings(false); + factory.getWorld().getCrosscuttingMembersSet().verify(); CompilationAndWeavingContext.leavingPhase(completeTypeBindingsToken); } diff --git a/tests/bugs151/pr133307/Broken.aj b/tests/bugs151/pr133307/Broken.aj new file mode 100644 index 000000000..30f2ef2ba --- /dev/null +++ b/tests/bugs151/pr133307/Broken.aj @@ -0,0 +1,7 @@ +interface TestIF {} + +class TestClass {} + +aspect TestAspect { + declare parents: TestClass implements TestIF; +} \ No newline at end of file diff --git a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml index 38e72cb43..bd8234389 100644 --- a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml +++ b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml @@ -4491,7 +4491,8 @@ - + + diff --git a/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java b/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java index fce100bd4..f078ae9d2 100644 --- a/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java +++ b/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java @@ -25,8 +25,9 @@ import org.aspectj.testing.XMLBasedAjcTestCase; public class Ajc151Tests extends org.aspectj.testing.XMLBasedAjcTestCase { -// public void testDeca() { runTest("doubly annotating a method with declare");} -// public void testDeca2() { runTest("doubly annotating a method with declare - 2");} + public void testCircularGenerics_pr133307() { runTest("circular generics");} + // public void testDeca() { runTest("doubly annotating a method with declare");} + // public void testDeca2() { runTest("doubly annotating a method with declare - 2");} public void testCrashingWithASM_pr132926_1() { runTest("crashing on annotation type resolving with asm - 1");} public void testCrashingWithASM_pr132926_2() { runTest("crashing on annotation type resolving with asm - 2");} public void testCrashingWithASM_pr132926_3() { runTest("crashing on annotation type resolving with asm - 3");} diff --git a/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml b/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml index 2373c27ec..c1d6610d9 100644 --- a/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml +++ b/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml @@ -19,6 +19,10 @@ + + + + diff --git a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java index e0e65bdcd..b4efd2c87 100644 --- a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java +++ b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java @@ -23,6 +23,7 @@ import java.util.Set; import org.aspectj.weaver.patterns.CflowPointcut; import org.aspectj.weaver.patterns.DeclareParents; +import org.aspectj.weaver.patterns.IVerificationRequired; /** * This holds on to all CrosscuttingMembers for a world. It handles @@ -45,6 +46,8 @@ public class CrosscuttingMembersSet { private List declareAnnotationOnMethods= null; // includes ctors private List declareDominates = null; private boolean changedSinceLastReset = false; + + private List /*IVerificationRequired*/ verificationList = null; // List of things to be verified once the type system is 'complete' public CrosscuttingMembersSet(World world) { this.world = world; @@ -261,12 +264,36 @@ public class CrosscuttingMembersSet { } public void reset() { + verificationList=null; changedSinceLastReset = false; } public boolean hasChangedSinceLastReset() { return changedSinceLastReset; } + + /** + * Record something that needs verifying when we believe the type system is complete. + * Used for things that can't be verified as we go along - for example some + * recursive type variable references (pr133307) + */ + public void recordNecessaryCheck(IVerificationRequired verification) { + if (verificationList==null) verificationList = new ArrayList(); + verificationList.add(verification); + } + /** + * Called when type bindings are complete - calls all registered verification + * objects in turn. + */ + public void verify() { + if (verificationList==null) return; + for (Iterator iter = verificationList.iterator(); iter.hasNext();) { + IVerificationRequired element = (IVerificationRequired) iter.next(); + element.verify(); + } + verificationList = null; + } + } diff --git a/weaver/src/org/aspectj/weaver/patterns/IVerificationRequired.java b/weaver/src/org/aspectj/weaver/patterns/IVerificationRequired.java new file mode 100644 index 000000000..f1ce19eff --- /dev/null +++ b/weaver/src/org/aspectj/weaver/patterns/IVerificationRequired.java @@ -0,0 +1,22 @@ +/* ******************************************************************* + * Copyright (c) 2006 Contributors + * All rights reserved. + * This program and the accompanying materials are made available + * under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Andy Clement IBM initial implementation + * ******************************************************************/ +package org.aspectj.weaver.patterns; + + +/** + * Implementors provide a 'verify()' method that is invoked at the end of type + * binding completion. + * @see WildTypePattern.VerifyBoundsForTypePattern + */ +public interface IVerificationRequired { + public void verify(); +} \ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java b/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java index 9d39c1791..fdde0e9b4 100644 --- a/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java +++ b/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java @@ -21,6 +21,7 @@ import java.util.Map; import java.util.StringTokenizer; import org.aspectj.bridge.IMessage; +import org.aspectj.bridge.ISourceLocation; import org.aspectj.bridge.Message; import org.aspectj.bridge.MessageUtil; import org.aspectj.util.FileUtil; @@ -925,46 +926,115 @@ public class WildTypePattern extends TypePattern { // now check that each typeParameter pattern, if exact, matches the bounds // of the type variable. - return checkBoundsOK(scope,genericType,requireExactType); - // return true; + // pr133307 - delay verification until type binding completion, these next few lines replace + // the call to checkBoundsOK + if (!boundscheckingoff) { + VerifyBoundsForTypePattern verification = + new VerifyBoundsForTypePattern(scope,genericType,requireExactType,typeParameters,getSourceLocation()); + scope.getWorld().getCrosscuttingMembersSet().recordNecessaryCheck(verification); + } +// return checkBoundsOK(scope,genericType,requireExactType); + + return true; } - public boolean checkBoundsOK(IScope scope,ResolvedType genericType,boolean requireExactType) { - if (boundscheckingoff) return true; - TypeVariable[] tvs = genericType.getTypeVariables(); - TypePattern[] typeParamPatterns = typeParameters.getTypePatterns(); - if (typeParameters.areAllExactWithNoSubtypesAllowed()) { - for (int i = 0; i < tvs.length; i++) { - UnresolvedType ut = typeParamPatterns[i].getExactType(); - boolean continueCheck = true; - // FIXME asc dont like this but ok temporary measure. If the type parameter - // is itself a type variable (from the generic aspect) then assume it'll be - // ok... (see pr112105) Want to break this? Run GenericAspectK test. - if (ut.isTypeVariableReference()) { - continueCheck = false; - } - - if (continueCheck && - !tvs[i].canBeBoundTo(ut.resolve(scope.getWorld()))) { - // issue message that type parameter does not meet specification - String parameterName = ut.getName(); - if (ut.isTypeVariableReference()) parameterName = ((TypeVariableReference)ut).getTypeVariable().getDisplayName(); - String msg = - WeaverMessages.format( - WeaverMessages.VIOLATES_TYPE_VARIABLE_BOUNDS, - parameterName, - new Integer(i+1), - tvs[i].getDisplayName(), - genericType.getName()); - if (requireExactType) scope.message(MessageUtil.error(msg,getSourceLocation())); - else scope.message(MessageUtil.warn(msg,getSourceLocation())); - return false; + /** + * By capturing the verification in this class, rather than performing it in verifyTypeParameters(), + * we can cope with situations where the interactions between generics and declare parents would + * otherwise cause us problems. For example, if verifying as we go along we may report a problem + * which would have been fixed by a declare parents that we haven't looked at yet. If we + * create and store a verification object, we can verify this later when the type system is + * considered 'complete' + */ + static class VerifyBoundsForTypePattern implements IVerificationRequired { + + private IScope scope; + private ResolvedType genericType; + private boolean requireExactType; + private TypePatternList typeParameters = TypePatternList.EMPTY; + private ISourceLocation sLoc; + + public VerifyBoundsForTypePattern(IScope scope, ResolvedType genericType, boolean requireExactType, + TypePatternList typeParameters, ISourceLocation sLoc) { + this.scope = scope; + this.genericType = genericType; + this.requireExactType = requireExactType; + this.typeParameters = typeParameters; + this.sLoc = sLoc; + } + + public void verify() { + TypeVariable[] tvs = genericType.getTypeVariables(); + TypePattern[] typeParamPatterns = typeParameters.getTypePatterns(); + if (typeParameters.areAllExactWithNoSubtypesAllowed()) { + for (int i = 0; i < tvs.length; i++) { + UnresolvedType ut = typeParamPatterns[i].getExactType(); + boolean continueCheck = true; + // FIXME asc dont like this but ok temporary measure. If the type parameter + // is itself a type variable (from the generic aspect) then assume it'll be + // ok... (see pr112105) Want to break this? Run GenericAspectK test. + if (ut.isTypeVariableReference()) { + continueCheck = false; + } + + System.err.println("Verifying "+ut.getName()+" meets bounds for "+tvs[i]); + if (continueCheck && + !tvs[i].canBeBoundTo(ut.resolve(scope.getWorld()))) { + // issue message that type parameter does not meet specification + String parameterName = ut.getName(); + if (ut.isTypeVariableReference()) parameterName = ((TypeVariableReference)ut).getTypeVariable().getDisplayName(); + String msg = + WeaverMessages.format( + WeaverMessages.VIOLATES_TYPE_VARIABLE_BOUNDS, + parameterName, + new Integer(i+1), + tvs[i].getDisplayName(), + genericType.getName()); + if (requireExactType) scope.message(MessageUtil.error(msg,sLoc)); + else scope.message(MessageUtil.warn(msg,sLoc)); + } } } } - return true; } + + // pr133307 - moved to verification object +// public boolean checkBoundsOK(IScope scope,ResolvedType genericType,boolean requireExactType) { +// if (boundscheckingoff) return true; +// TypeVariable[] tvs = genericType.getTypeVariables(); +// TypePattern[] typeParamPatterns = typeParameters.getTypePatterns(); +// if (typeParameters.areAllExactWithNoSubtypesAllowed()) { +// for (int i = 0; i < tvs.length; i++) { +// UnresolvedType ut = typeParamPatterns[i].getExactType(); +// boolean continueCheck = true; +// // FIXME asc dont like this but ok temporary measure. If the type parameter +// // is itself a type variable (from the generic aspect) then assume it'll be +// // ok... (see pr112105) Want to break this? Run GenericAspectK test. +// if (ut.isTypeVariableReference()) { +// continueCheck = false; +// } +// +// if (continueCheck && +// !tvs[i].canBeBoundTo(ut.resolve(scope.getWorld()))) { +// // issue message that type parameter does not meet specification +// String parameterName = ut.getName(); +// if (ut.isTypeVariableReference()) parameterName = ((TypeVariableReference)ut).getTypeVariable().getDisplayName(); +// String msg = +// WeaverMessages.format( +// WeaverMessages.VIOLATES_TYPE_VARIABLE_BOUNDS, +// parameterName, +// new Integer(i+1), +// tvs[i].getDisplayName(), +// genericType.getName()); +// if (requireExactType) scope.message(MessageUtil.error(msg,getSourceLocation())); +// else scope.message(MessageUtil.warn(msg,getSourceLocation())); +// return false; +// } +// } +// } +// return true; +// } public boolean isStar() { boolean annPatternStar = annotationPattern == AnnotationTypePattern.ANY; -- 2.39.5