genericitds: 2 big changes here: I've modifed the super/extends stuff so its only in one place (UnresolvedType) - making it available through the type hierarchy. I've modified the TypeMap in the World to avoid putting entries in the type map which might confuse us later. This fix stops us putting parameterized types that are parameterized by type variables from a generic member into the typemap, since we may look them up later for another member that happened to use the same variable name but had different bounds specified. I'm sure its not perfect yet, but it is definetly improved. things like Enum<E> still go in the typemap since E is not a generic member type variable.

7 files changed, 159 insertions, 30 deletions

diff --git a/weaver/src/org/aspectj/weaver/ResolvedType.java b/weaver/src/org/aspectj/weaver/ResolvedType.java
index a8b942c94..185c1d9fe 100644
--- a/weaver/src/org/aspectj/weaver/ResolvedType.java
+++ b/weaver/src/org/aspectj/weaver/ResolvedType.java
@@ -18,7 +18,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -29,6 +28,7 @@ import org.aspectj.bridge.IMessage;
 import org.aspectj.bridge.ISourceLocation;
 import org.aspectj.bridge.Message;
 import org.aspectj.bridge.MessageUtil;
+import org.aspectj.util.FuzzyBoolean;
 import org.aspectj.weaver.patterns.Declare;
 import org.aspectj.weaver.patterns.PerClause;
 
@@ -1609,6 +1609,69 @@ public abstract class ResolvedType extends UnresolvedType implements AnnotatedEl
 	public String getSignatureForAttribute() {
 		throw new RuntimeException("Cannot ask this type "+this+" for a generic sig attribute");
 	}
-
+	
+	private FuzzyBoolean parameterizedWithAMemberTypeVariable = FuzzyBoolean.MAYBE;
+	
+	/**
+	 * return true if the parameterization of this type includes a member type variable.  Member
+	 * type variables occur in generic methods/ctors.
+	 */
+	public boolean isParameterizedWithAMemberTypeVariable() {
+		// MAYBE means we haven't worked it out yet...
+		if (parameterizedWithAMemberTypeVariable==FuzzyBoolean.MAYBE) {
+			
+			// if there are no type parameters then we cant be...
+			if (typeParameters==null || typeParameters.length==0) {
+				parameterizedWithAMemberTypeVariable = FuzzyBoolean.NO;
+				return false;
+			}
+			
+			for (int i = 0; i < typeParameters.length; i++) {
+				UnresolvedType aType = (ResolvedType)typeParameters[i];
+				if (aType.isTypeVariableReference()  && ((TypeVariableReference)aType).getTypeVariable().getDeclaringElementKind()==TypeVariable.METHOD) {
+					parameterizedWithAMemberTypeVariable = FuzzyBoolean.YES;
+					return true;
+				}
+				if (aType.isParameterizedType()) {
+					boolean b = aType.isParameterizedWithAMemberTypeVariable();
+					if (b) {
+						parameterizedWithAMemberTypeVariable = FuzzyBoolean.YES;
+						return true;
+					}
+				}
+				if (aType.isGenericWildcard()) {
+					if (aType.isExtends()) {
+						boolean b = false;
+						UnresolvedType upperBound = aType.getUpperBound();
+						if (upperBound.isParameterizedType()) {
+							b = upperBound.isParameterizedWithAMemberTypeVariable();
+						} else if (upperBound.isTypeVariableReference() && ((TypeVariableReference)upperBound).getTypeVariable().getDeclaringElementKind()==TypeVariable.METHOD) {
+							b = true;
+						}
+						if (b) {
+							parameterizedWithAMemberTypeVariable = FuzzyBoolean.YES;
+							return true;
+						}
+						// FIXME asc need to check additional interface bounds
+					}
+					if (aType.isSuper()) {
+						boolean b = false;
+						UnresolvedType lowerBound = aType.getLowerBound();
+						if (lowerBound.isParameterizedType()) {
+							b = lowerBound.isParameterizedWithAMemberTypeVariable();
+						} else if (lowerBound.isTypeVariableReference() && ((TypeVariableReference)lowerBound).getTypeVariable().getDeclaringElementKind()==TypeVariable.METHOD) {
+							b = true;
+						}
+						if (b) {
+							parameterizedWithAMemberTypeVariable = FuzzyBoolean.YES;
+							return true;
+						}
+					}
+				}
+			}
+			parameterizedWithAMemberTypeVariable=FuzzyBoolean.NO;
+		}
+		return parameterizedWithAMemberTypeVariable.alwaysTrue();
+	}
 	    
 }
diff --git a/weaver/src/org/aspectj/weaver/TypeVariable.java b/weaver/src/org/aspectj/weaver/TypeVariable.java
index 35623d7bd..f450d8b76 100644
--- a/weaver/src/org/aspectj/weaver/TypeVariable.java
+++ b/weaver/src/org/aspectj/weaver/TypeVariable.java
@@ -34,7 +34,17 @@ public class TypeVariable {
 	private String name;
 	
 	private int rank;
-	
+
+    // It would be nice to push this field onto the TypeVariableDeclaringElement
+    // interface (a getKind()) but at the moment we don't always guarantee
+    // to set the declaring element (eclipse seems to utilise the knowledge of
+    // what declared the type variable, but we dont yet...)
+	/**
+	 * What kind of element declared this type variable?
+	 */
+	private int declaringElementKind = TYPE;
+	public static final int METHOD  = 1;
+	public static final int TYPE    = 2;
 	private TypeVariableDeclaringElement declaringElement;
 	
 	/**
@@ -254,4 +264,13 @@ public class TypeVariable {
 		return declaringElement;
 	}
 	
+	public void setDeclaringElementKind(int kind) {
+		this.declaringElementKind = kind;
+	}
+	
+	public int getDeclaringElementKind() {
+//		if (declaringElementKind==UNKNOWN) throw new RuntimeException("Dont know declarer of this tvar : "+this);
+		return declaringElementKind;
+	}
+	
 }
diff --git a/weaver/src/org/aspectj/weaver/TypeVariableReferenceType.java b/weaver/src/org/aspectj/weaver/TypeVariableReferenceType.java
index 9c47db1aa..17941318a 100644
--- a/weaver/src/org/aspectj/weaver/TypeVariableReferenceType.java
+++ b/weaver/src/org/aspectj/weaver/TypeVariableReferenceType.java
@@ -56,7 +56,7 @@ public class TypeVariableReferenceType extends BoundedReferenceType implements T
 		return true;
 	}
 	
-//	public ResolvedType resolve(World world) {
+    //public ResolvedType resolve(World world) {
 	//	return super.resolve(world);
 	//}
 	
diff --git a/weaver/src/org/aspectj/weaver/UnresolvedType.java b/weaver/src/org/aspectj/weaver/UnresolvedType.java
index 320d93e5c..5efa8c423 100644
--- a/weaver/src/org/aspectj/weaver/UnresolvedType.java
+++ b/weaver/src/org/aspectj/weaver/UnresolvedType.java
@@ -156,6 +156,12 @@ public class UnresolvedType implements TypeVariableDeclaringElement {
 	 */
 	private UnresolvedType lowerBound;
 	
+	/**
+	 * for wildcards '? extends' or for type variables 'T extends'
+	 */
+	private boolean isSuper   = false;
+	private boolean isExtends = false;
+	
    /**
      * Determines if this represents a primitive type.  A primitive type
      * is one of nine predefined resolved types.
@@ -172,16 +178,16 @@ public class UnresolvedType implements TypeVariableDeclaringElement {
      * @see     ResolvedType#Double
      * @see     ResolvedType#Void
      */   
-    public boolean isPrimitiveType() { return typeKind == TypeKind.PRIMITIVE; }
-    public boolean isSimpleType() { return typeKind == TypeKind.SIMPLE; }
-    public boolean isRawType() { return typeKind == TypeKind.RAW; }
-    public boolean isGenericType() { return typeKind == TypeKind.GENERIC; }
-    public boolean isParameterizedType() { return typeKind == TypeKind.PARAMETERIZED; }
-    public boolean isTypeVariableReference() { return typeKind == TypeKind.TYPE_VARIABLE; }
-    public boolean isGenericWildcard() { return typeKind == TypeKind.WILDCARD; }
-    public boolean isGenericWildcardExtends() { return isGenericWildcard() && upperBound != null; }
-    public boolean isGenericWildcardSuper() { return isGenericWildcard() && lowerBound != null; }
-
+    public boolean isPrimitiveType()          { return typeKind == TypeKind.PRIMITIVE; }
+    public boolean isSimpleType()             { return typeKind == TypeKind.SIMPLE; }
+    public boolean isRawType()                { return typeKind == TypeKind.RAW; }
+    public boolean isGenericType()            { return typeKind == TypeKind.GENERIC; }
+    public boolean isParameterizedType()      { return typeKind == TypeKind.PARAMETERIZED; }
+    public boolean isTypeVariableReference()  { return typeKind == TypeKind.TYPE_VARIABLE; }
+    public boolean isGenericWildcard()        { return typeKind == TypeKind.WILDCARD; }
+    public boolean isExtends() { return isExtends;}
+    public boolean isSuper()   { return isSuper;  }
+    
     // for any reference type, we can get some extra information...
     public final boolean isArray() {  return signature.startsWith("["); } 
 
@@ -241,14 +247,17 @@ public class UnresolvedType implements TypeVariableDeclaringElement {
 	 */
     protected UnresolvedType(String signature) {
         super();
- //       if (signature.indexOf('<') != -1) throw new IllegalStateException("Shouldn't be calling simple signature based type constructor with generic info in signature");
         this.signature = signature;
         this.signatureErasure = signature;
+        if (signature.charAt(0)=='-') isSuper   = true;
+        if (signature.charAt(0)=='+') isExtends = true;
     }
     
     protected UnresolvedType(String signature, String signatureErasure) {
     	this.signature = signature;
     	this.signatureErasure = signatureErasure;
+        if (signature.charAt(0)=='-') isSuper   = true;
+        if (signature.charAt(0)=='+') isExtends = true;
     }
     
     // called from TypeFactory
@@ -825,7 +834,7 @@ public class UnresolvedType implements TypeVariableDeclaringElement {
 	
 		public String toString() {
 			return type;
-}
+		}
 
 		private TypeKind(String type) {
 			this.type = type;
@@ -834,5 +843,14 @@ public class UnresolvedType implements TypeVariableDeclaringElement {
 		private final String type;
 	}
 	
+	/**
+	 * Will return true if the type being represented is parameterized with a type variable
+	 * from a generic method/ctor rather than a type variable from a generic type.  
+	 * Only subclasses know the answer...
+	 */
+	public boolean isParameterizedWithAMemberTypeVariable() {
+		throw new RuntimeException("I dont know - you should ask a resolved version of me: "+this);
+	}
+	
 }
 
diff --git a/weaver/src/org/aspectj/weaver/World.java b/weaver/src/org/aspectj/weaver/World.java
index 053a3c10d..c94090ebb 100644
--- a/weaver/src/org/aspectj/weaver/World.java
+++ b/weaver/src/org/aspectj/weaver/World.java
@@ -340,18 +340,16 @@ public abstract class World implements Dump.INode {
      */
     private ReferenceType resolveGenericWildcardFor(UnresolvedType aType) {
     	BoundedReferenceType ret = null;
-    	// FIXME asc isExtends? isGenericWildcardExtends?  I dont like having two
     	// FIXME asc doesnt take account of additional interface bounds (e.g. ? super R & Serializable - can you do that?)
-    	if (aType.isGenericWildcardExtends()) {
+    	if (aType.isExtends()) {
     		ReferenceType upperBound = (ReferenceType)resolve(aType.getUpperBound());
     		ret = new BoundedReferenceType(upperBound,true,this);
-       	} else {
+       	} else if (aType.isSuper()) {
     		ReferenceType lowerBound = (ReferenceType) resolve(aType.getLowerBound());
     		ret = new BoundedReferenceType(lowerBound,false,this);
+    	} else {
+    		// must be ? on its own!
     	}
-    	// FIXME asc verify: I don't think these go in the typemap, it makes it potentially impossible to differentiate different uses of 'T',
-    	// for example '? super T' where T is representing different things in two places would have the same sig (-TT;)
-    	// typeMap.put(aType.getSignature(),ret);
     	return ret;
     }
     
@@ -593,9 +591,42 @@ public abstract class World implements Dump.INode {
 		private Map tMap = new HashMap();
 		/** Map of types that may be ejected from the cache if we need space */
 		private Map expendableMap = new WeakHashMap();
-					
-		/** Add a new type into the map, the key is the type signature */
+		
+		private static final boolean debug = false;
+		/** 
+		 * Add a new type into the map, the key is the type signature.
+		 * Some types do *not* go in the map, these are ones involving
+		 * *member* type variables.  The reason is that when all you have is the
+		 * signature which gives you a type variable name, you cannot 
+		 * guarantee you are using the type variable in the same way 
+		 * as someone previously working with a similarly
+		 * named type variable.  So, these do not go into the map:
+		 * - TypeVariableReferenceType.
+		 * - ParameterizedType where a member type variable is involved.
+		 * - BoundedReferenceType when one of the bounds is a type variable.
+		 * 
+		 * definition: "member type variables" - a tvar declared on a generic 
+		 * method/ctor as opposed to those you see declared on a generic type.
+		 */
 		public ResolvedType put(String key, ResolvedType type) { 
+			if (type.isParameterizedType() && type.isParameterizedWithAMemberTypeVariable()) {
+				if (debug) 
+					System.err.println("Not putting a parameterized type that utilises member declared type variables into the typemap: key="+key+" type="+type);
+				return type;
+			}
+			if (type.isTypeVariableReference()) {
+				if (debug) 
+					System.err.println("Not putting a type variable reference type into the typemap: key="+key+" type="+type);
+				return type;
+			}
+			// this test should be improved - only avoid putting them in if one of the
+			// bounds is a member type variable
+			if (type instanceof BoundedReferenceType) {
+				if (debug) 
+					System.err.println("Not putting a bounded reference type into the typemap: key="+key+" type="+type);
+				return type;
+			}
+						
 			if (isExpendable(type))  {
 				return (ResolvedType) expendableMap.put(key,type);
 			} else {
diff --git a/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java b/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java
index 834168bfb..01983f3f7 100644
--- a/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java
+++ b/weaver/src/org/aspectj/weaver/patterns/WildTypePattern.java
@@ -253,13 +253,13 @@ public class WildTypePattern extends TypePattern {
 		if (lowerBound == null && aType.getLowerBound() != null) return false;
 		if (upperBound != null) {
 			// match ? extends
-			if (aType.isGenericWildcardSuper()) return false;
+			if (aType.isGenericWildcard() && aType.isSuper()) return false;
 			if (aType.getUpperBound() == null) return false;
 			return upperBound.matches((ResolvedType)aType.getUpperBound(),staticOrDynamic).alwaysTrue();
 		}
 		if (lowerBound != null) {
 			// match ? super
-			if (!aType.isGenericWildcardSuper()) return false;
+			if (!(aType.isGenericWildcard() && aType.isSuper())) return false;
 			return lowerBound.matches((ResolvedType)aType.getLowerBound(),staticOrDynamic).alwaysTrue();
 		}
 		return true;
diff --git a/weaver/testsrc/org/aspectj/weaver/bcel/BcelGenericSignatureToTypeXTestCase.java b/weaver/testsrc/org/aspectj/weaver/bcel/BcelGenericSignatureToTypeXTestCase.java
index 35ea06360..a51ffdc2c 100644
--- a/weaver/testsrc/org/aspectj/weaver/bcel/BcelGenericSignatureToTypeXTestCase.java
+++ b/weaver/testsrc/org/aspectj/weaver/bcel/BcelGenericSignatureToTypeXTestCase.java
@@ -11,16 +11,14 @@
  * ******************************************************************/
 package org.aspectj.weaver.bcel;
 
+import junit.framework.TestCase;
+
 import org.aspectj.apache.bcel.Repository;
 import org.aspectj.apache.bcel.classfile.GenericSignatureParser;
 import org.aspectj.apache.bcel.classfile.JavaClass;
 import org.aspectj.apache.bcel.classfile.Signature;
-import org.aspectj.weaver.ResolvedType;
-import org.aspectj.weaver.TypeVariable;
 import org.aspectj.weaver.UnresolvedType;
 
-import junit.framework.TestCase;
-
 /**
  * @author colyer
  *