test and fix for 133307 - funky type variable bounds.

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 df36ecebbd6a878b73ce490ccd5d6e5a8dd5b9ca..e562f353830b877d0b3e59d0c47fbc9aa4d3b53a 100644 (file)
--- 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 (file)
index 0000000..30f2ef2
--- /dev/null
+++ b/tests/bugs151/pr133307/Broken.aj
@@ -0,0 +1,7 @@
+interface TestIF<T extends TestIF> {}
+
+class TestClass {}
+
+aspect TestAspect {
+   declare parents: TestClass implements TestIF<TestClass>;
+}
\ 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 38e72cb43b06431400f030e5df549da14384efdf..bd82343892ed4275ba89c0a3f0022ad83774334d 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
+++ b/tests/src/org/aspectj/systemtest/ajc150/ajc150.xml
@@ -4491,7 +4491,8 @@
    <ajc-test dir="java5/generics/genericaspects/" title="uberaspects - H">
      <compile files="GenericAspectH.aj" options="-1.5">
        <message kind="error" line="7" text="Type java.lang.String does not meet the specification for type parameter 1 (N extends java.lang.Number) in generic type GenericAspect$SimpleI"/>
-       <message kind="error" line="16" text="The method m4(String) is undefined for the type Base"/>
+          <!-- see pr133307, shame about this -->
+       <!--message kind="error" line="16" text="The method m4(String) is undefined for the type Base"/-->
      </compile>
    </ajc-test>
    <ajc-test dir="java5/generics/genericaspects/" title="uberaspects - I">

diff --git a/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java b/tests/src/org/aspectj/systemtest/ajc151/Ajc151Tests.java
index fce100bd4fe57745e913758d0edd826ffadd8bda..f078ae9d2c0440cb7258667984a1f6cd24cefea0 100644 (file)
--- 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 2373c27eca6b90db9caa7da0c19f7cfb7346d491..c1d6610d90a65034625591ca898495f75fac6ea9 100644 (file)
--- a/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml
+++ b/tests/src/org/aspectj/systemtest/ajc151/ajc151.xml
@@ -19,6 +19,10 @@
      </compile>
     </ajc-test>
     
+    <ajc-test dir="bugs151/pr133307" title="circular generics">
+      <compile files="Broken.aj" options="-1.5"/>
+    </ajc-test>  
+    
     <ajc-test dir="bugs151/pr123553" title="generic advice parameters">
         <compile files="A.java" options="-1.5"/>
         <run class="A"/>

diff --git a/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java b/weaver/src/org/aspectj/weaver/CrosscuttingMembersSet.java
index e0e65bdcd8d4d233ea01b3b4d34c8b450c15ff58..b4efd2c87108bdd2cc67390593563df35293adad 100644 (file)
--- 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 (file)
index 0000000..f1ce19e
--- /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 9d39c17916050e16a4638cfd94cf8f7ffe326fb8..fdde0e9b4f929ce550c2bdf076095105f415459f 100644 (file)
--- 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;