Community
Participate
Working Groups
I20080528-2000 For a fix for bug 211037, I tried to use BindingKey.createWilcardTypeBindingKey("Ljava/lang/Object;", Signature.C_EXTENDS). The result was "+Ljava/lang/Object;". Later, I called 'new BindingKey("+Ljava/lang/Object;").toSignature()'. This returned ";". Expected: "+Ljava.lang.Object;" In the AST, the binding key for '? extends Object' inside 'Class<? extends Object>' is "Ljava/lang/Class;+Ljava/lang/Object;". I think the parameterized type should not show up in the key for the wildcard type, e.g. in this example, the two type bindings for '? extends Object' should be identical, with the same key: Class<? extends Object> clazz = null; ArrayList<? extends Object> list = null;
It looks like 2 problems are reported in comment 0. The first one is new BindingKey("+Ljava/lang/Object;").toSignature() failing to return the correct signature. I will keep this bug for this problem. Please open a separate bug for the second issue.
Sorry, I didn't spell out the fundamental issue that has to be resolved first, before we can decide whether we need multiple bug reports (and what independent problems they should address): What is the correct binding key string for a wildcard type binding? Once this is clear, one or more of these methods have to be fixed: - BindingKey#createWilcardTypeBindingKey(..) - BindingKey#toSignature() - ITypeBinding#getKey() I think, for '? extends Object', the key should be "+Ljava.lang.Object;". For this, BindingKey#toSignature() and ITypeBinding#getKey() would need to be fixed (thereby making sure that all contracts related to getKey() are fulfilled).
In fact, at the compiler level, to create a wildcard binding we need: 1. a generic type (a wildcard is always the wildcard of a generic type) 2. a kind 3. a bound type (optional) 4. a rank (case of 2 wildcards appearing as type arguments, e.g. X<?, ?>) All this information is in the binding key. However, BindingKey.createWilcardTypeBindingKey(...) allows to pass in only 2 and 3. So it looks like this API is not enough to recreate a wildcard type. We need to introduce a new createWilcardTypeBindingKey(...) API that takes all 4 arguments. And deprecate the existing createWilcardTypeBindingKey(...). We also need to add getGenericType() and getRank() on ITypeBinding.
Srikanth, please investigate the addition of the new API (see comment 3) and the corresponding implementation.
Created attachment 122674 [details] Proposed patch + test As described in comment #3, the existing API is inadequate to recreate the wildcard binding from a binding key. Here is an API proposal that addresses that limitation by requiring an extra argument (generic type binding key) to be passed. (The 'rank' argument is not required since it will be synthesized by the BindingKeyParser) Javadoc for the new API: /** * Creates a new wildcard type binding key from the given generic type binding key, the given wildcard * kind (one of {@link Signature#C_STAR}, {@link Signature#C_SUPER}, or {@link Signature#C_EXTENDS} and * the given bound type binding key. If the wildcard kind is {@link Signature#C_STAR}, the given bound * type binding key is ignored. * <p> * For example: * <pre> * <code> * createWildcardTypeBindingKey("Ljava/util/ArrayList;", Signature.C_STAR, null) -> "Ljava/util/ArrayList;*" * createWildcardTypeBindingKey("Ljava/util/ArrayList;", Signature.C_SUPER, "Ljava/lang/String;") -> "Ljava/util/ArrayList;-Ljava/lang/String;" * createWildcardTypeBindingKey("Ljava/util/ArrayList;", Signature.C_EXTENDS, "Ljava/lang/String;") -> * "Ljava/util/ArrayList;+Ljava/lang/String;" * </code> * </pre> * </p> * * @param genericTypeKey the binding key of the generic type * @param boundKind one of {@link Signature#C_STAR}, {@link Signature#C_SUPER}, or {@link Signature#C_EXTENDS} * @param boundTypeKey the binding key of the bounding type. * @return a new wildcard type binding key */ public static String createWildcardTypeBindingKey(String genericTypeKey, char boundKind, String boundTypeKey) { ... }
Thinking more about the 'rank', it seems that synthesizing the 'rank' in the BindingKeyParser is not the right thing to do. Indeed in the following case: "HashMap<? extends Integer, ? extends Integer>" if we take the 2 wildcard bindings and ask for their key, they will have the same key even though they are not equals. As a result, if one tries to resolve these wildcard bindings from their key (e.g. using ASTParser.createASTs(...)), this will result in the first wildcard binding to be returned for the first wildcard key as well as for the second wildcard key. So synthesizing the 'rank' in the BindingKeyParser works only if parsing the key for "HashMap<? extends Integer, ? extends Integer>", not if parsing the key for the second "? extends Integer". I'm afraid this code has to be rewritten, so that the 'rank' is always included in the key and synthesizing the 'rank' will no longer be needed.
(In reply to comment #6) > Thinking more about the 'rank', it seems that synthesizing the 'rank' in the > BindingKeyParser is not the right thing to do. > Indeed in the following case: "HashMap<? extends Integer, ? extends Integer>" > if we take the 2 wildcard bindings and ask for their key, they will have the > same key even though they are not equals. > As a result, if one tries to resolve these wildcard bindings from their key > (e.g. using ASTParser.createASTs(...)), this will result in the first wildcard > binding to be returned for the first wildcard key as well as for the second > wildcard key. > So synthesizing the 'rank' in the BindingKeyParser works only if parsing the > key for "HashMap<? extends Integer, ? extends Integer>", not if parsing the key > for the second "? extends Integer". > I'm afraid this code has to be rewritten, so that the 'rank' is always included > in the key and synthesizing the 'rank' will no longer be needed. OK, I think this makes sense and I can organize this. The fix seems to be spread across the following pieces: 1. BindingKey.createWildcardTypeBindingKey is ignoring the generic type and rank. It should not. 2. WildcardBinding.computeUniqueKey is ignoring rank, but takes into consideration the generic type. It should use both. 3. BindingKeyResolver should not attempt to track & synthesize the rank but must instead recover the rank encoded/embedded in the binding key by methods (1) and (2) above. 4. Provide new methods getRank and getGenericType on the ITypeBinding. 5. Fix broken tests and add new tests. Now what is your thought on what KeyToSignature should be doing ? It is dropping both the generic type and the rank and as a result the signature comes as as shown below: Ljava/util/HashMap;+Ljava/lang/Integer;0; ==> +Ljava.lang.Integer; Is this intentional ?
(In reply to comment #7) > OK, I think this makes sense and I can organize this. The fix seems to be > spread across the following pieces: > > 1. BindingKey.createWildcardTypeBindingKey is ignoring the generic type and > rank. It should not. > > 2. WildcardBinding.computeUniqueKey is ignoring rank, but takes into > consideration the generic type. It should use both. > > 3. BindingKeyResolver should not attempt to track & synthesize the rank but > must instead recover the rank encoded/embedded in the binding key by methods > (1) and (2) above. > > 4. Provide new methods getRank and getGenericType on the ITypeBinding. > > 5. Fix broken tests and add new tests. > That makes sense to me. > Now what is your thought on what KeyToSignature should be doing ? It is > dropping both the generic type and the rank and as a result the signature > comes as as shown below: > > Ljava/util/HashMap;+Ljava/lang/Integer;0; ==> +Ljava.lang.Integer; > > Is this intentional ? > Yes, I believe this is intentional. The only usage of such signature I can think of is Signature.toString(String). In this case, the generic type and the rank are not needed.
(In reply to comment #7) Sounds good, except: > 4. Provide new methods getRank and getGenericType on the ITypeBinding. I don't like the name of the proposed method ITypeBinding#getGenericType(). We already have isGenericType(), but getGenericType() would NOT return the corresponding generic type for the receiver (getTypeDeclaration() does that). The method should in fact return the parameterized (not generic) type binding that contains the wildcard (the "parent" in ASTNode-speak). For any wildcard type binding wtb, the array wtb.getGenericType().getTypeArguments() should contain wtb. Better names would be getContainerType() or maybe getParameterizedType().
(In reply to comment #9) > (In reply to comment #7) > Sounds good, except: > > 4. Provide new methods getRank and getGenericType on the ITypeBinding. > I don't like the name of the proposed method ITypeBinding#getGenericType(). We > already have isGenericType(), but getGenericType() would NOT return the > corresponding generic type for the receiver (getTypeDeclaration() does that). Sorry, I don't follow your train of thoughts. What do you mean (the new API) getGenericType() "would NOT" return the generic type ? Are you saying it is less useful that way ? And that it makes more sense from a client p.o.v to have an API return the Parametrized type binding ? Naturally, the name should follow what we want the new API to expose (which itself is best decided by client needs/ expected usage model (of which I don't have a fully clear picture - so your input is useful and welcome)) I see that getTypeDeclaration() does return the generic type if the receiver is a paramerized type binding and getTypeArguments exposes the wild card type bindings of a parametrized type. So if we did have a facilty to go from wild card binding to parametrized type binding, it would complete the circle - is this what you are implying ??
Sorry, I mixed 2 objections: a) The name of the method should make clear that it does not return another view of the receiver binding. getContainerType() or getContainingType() would be less ambiguous. b) Although I currently don't have a concrete use case, I think it could be interesting for clients to get not only the generic type of the container, but also the parameterized type (which would "complete the circle", similar to how you can go from a type variable tv to the declaring type and then find tv in the array tv.getDeclaringClass().getTypeParameters()). However, I now had a quick look at the compiler bindings, and it seems like the the containing parameterized binding is not easily available from the wildcard binding. => (b) is not too important, so it's no problem if you just return the generic type (but please call the method getContainerType()).
(In reply to comment #11) > Sorry, I mixed 2 objections: > a) The name of the method should make clear that it does not return another > view of the receiver binding. getContainerType() or getContainingType() would > be less ambiguous. > => (b) is not too important, so it's no problem if you just return the generic > type (but please call the method getContainerType()). I am eagar to oblige, but am having some issues with semantics - What would the javadoc say ? It would be odd if the method exposes the generic type and is documented to do so too, but is named getContainerType() - wouldn't it ? - This is perhaps splitting hairs, but the word "Container", if it is used in the sense of a collection of objects, is too limiting as you can have parametrized types that aren't exactly containers. OTOH, If it is used in the sense of "containing" the wildcard (or otherwise) argument types, that is not quite true too - the argument type parameterize and select one concrete type instance from a family of types, rather than being contained. > b) Although I currently don't have a concrete use case, I think it could be > interesting for clients to get not only the generic type of the container, but > also the parameterized type (which would "complete the circle", similar to how > you can go from a type variable tv to the declaring type and then find tv in > the array tv.getDeclaringClass().getTypeParameters()). > However, I now had a quick look at the compiler bindings, and it seems like the > the containing parameterized binding is not easily available from the wildcard > binding. Yes, this was my concern too. I think we can wait for a use case requirement to emerge and if and when it does, have a separate enhancement request.
Created attachment 123066 [details] Patch with review comments addressed This patch contains the changes outlines in comment#7 I'll recreate the final patch if needed after we decide on the new API name and also to address any remaining nits. Javadoc for the new APIs: /** * Creates a new wildcard type binding key from the given generic type binding key, the given wildcard * kind (one of {@link Signature#C_STAR}, {@link Signature#C_SUPER}, or {@link Signature#C_EXTENDS} * the given bound type binding key and the given rank. If the wildcard kind is {@link Signature#C_STAR}, * the given bound type binding key is ignored. * <p> * For example: * <pre> * <code> * createWildcardTypeBindingKey("Ljava/util/ArrayList;", Signature.C_STAR, null, 0) -> "Ljava/util/ArrayList;{0}*" * createWildcardTypeBindingKey("Ljava/util/ArrayList;", Signature.C_SUPER, "Ljava/lang/String;", 0) -> "Ljava/util/ArrayList;{0}-Ljava/lang/String;" * createWildcardTypeBindingKey("Ljava/util/HashMap;", Signature.C_EXTENDS, "Ljava/lang/String;", 1) -> * "Ljava/util/HashMap;{1}+Ljava/lang/String;" * </code> * </pre> * </p> * * @param genericTypeKey the binding key of the generic type * @param boundKind one of {@link Signature#C_STAR}, {@link Signature#C_SUPER}, or {@link Signature#C_EXTENDS} * @param boundTypeKey the binding key of the bounding type. * @param rank the relative position of this wild card type in the parameterization of the generic type. * @return a new wildcard type binding key */ public static String createWildcardTypeBindingKey(String genericTypeKey, char boundKind, String boundTypeKey, int rank) { } 2. On ITypeBinding: /** * Returns the generic type associated with this wildcard type, if it has one. * Returns <code>null</code> if this is not a wildcard type. * * @return the generic type associated with this wildcard type, or <code>null</code> if none * @see #isWildcardType() * @since 3.5 */ public ITypeBinding getGenericType(); /** * Returns the rank associated with this wildcard type. The rank of this wild card type is the relative * position of the wild card type in the parameterization of the associated generic type. * Returns <code>-1</code> if this is not a wildcard type. * * @return the rank associated with this wildcard type, or <code>-1</code> if none * @see #isWildcardType() * @since 3.5 */ public int getRank();
> It would be odd if the method exposes the generic type and is documented to > do so too, but is named getContainerType() - wouldn't it ? If we had a hierarchy of ITypeBindings and getGenericType() would only be declared on IWildcardTypeBinding, then I would agree. But ITypeBinding#getGenericType() will be available on all type bindings, including parameterized type bindings. I find it confusing to have getGenericType() on a parameterized type binding, but the method returns null, and not the generic type. I have a method with the perfect name for what I want, but it doesn't work. I have to use getDeclaringType() instead. I agree that getContainerType() or getContainingType() are not 100% accurate, but at least they don't collide with something else. Another possibility would be getParameterizedGenericType(), which would be a bit longer, but would cause less confusion and still be correct (it returns the generic type which got parameterized by the wildcard type)
(In reply to comment #14) [...] > I find it confusing to have getGenericType() on a parameterized type binding, > but the method returns null, and not the generic type. I have a method with the > perfect name for what I want, but it doesn't work. I have to use > getDeclaringType() instead. Yes, indeed. > Another possibility would > be getParameterizedGenericType(), which would be a bit longer, but would cause > less confusion and still be correct (it returns the generic type which got > parameterized by the wildcard type) This has the same problem as above, don't you think ? i.e if it is invoked on a parametrized type binding and if it returns null, it would be confusing. Jerome, what are your thoughts on this ? Alternately, does it make sense to call the new API getGenericType and have it return the generic type for both wild card type binding and parametrized type binding ?
What about getWildcardTypeGenericType() ? This makes it clear that it applies to a wildcard binding only (isWildcardType() returns true) and that the returned value is a generic type (isGenericType() returns true). Otherwise the patch looks good. Just don't forget the "@since 3.5" on the new createWilcardTypeBindingKey(...)
(In reply to comment #16) > What about getWildcardTypeGenericType() ? Thanks, that is clearer. Unless there are any major objections, I'll improve it a bit more by twisting it to be getGenericTypeOfWildcardType()
Created attachment 123244 [details] Patch with API name finalized
Comment on attachment 123244 [details] Patch with API name finalized Patch looks good
Patch released for 3.5M5
Verified for 3.5M5 using I20090126-1300
