RFR 8191802: Upward projection result is A<? extends Number> instead of A<? super Integer>
Vicente Romero
vicente.romero at oracle.com
Wed Nov 29 02:22:41 UTC 2017
On 11/28/2017 06:14 PM, Vicente Romero wrote:
> Hi,
>
> This one is a tough one to review, so I think that we will probably
> need a couple of iterations. As mentioned earlier today in an offline
> chat, some comments at test UpperBounds seems to don't match with the
> type expected in the annotation.
>
> It seems to me that the code could be simplified, more self
> documented, etc, if TypeProjection::visitTypeVar was not handling type
> variables and captured type variables at the same time, it doesn't
> feel natural either.
never mind about the two below, I was checking the code again and I
think that the case I was thinking about can't happen. I missed that
visitor TypeProjection is invoked from TypeArgumentProjection
>
> At TypeArgumentProjection::visitWildcardType shouldn't we consider
> returning the same type if the wildcard doesn't contain restricted
> variables? In other words: can we guarantee that every wildcard
> visited will contain restricted variables? I'm thinking about nested
> cases.
>
> kind of similar comment for TypeArgumentProjection::visitType, can we
> guarantee that every time we get to this method, we are dealing with a
> type that contains restricted variables? again I'm considering nested
> cases. We have to be more careful in this case as this could imply
> issuing an error for a case for which a projection exists.
>
> Thanks,
> Vicente
Vicente
>
> On 11/27/2017 10:09 AM, Maurizio Cimadamore wrote:
>>
>> This is an updated version of the webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/8191802-v2/
>>
>> I realized that there were further discrepancies w.r.t. the spec
>> text, so I handled them all at once:
>>
>> * the spec has different treatment for when a type argument is a
>> wildcard and for when it's a regular type - the implementation was
>> treating both wildcards and regular types in the same way, which led
>> to inconsistencies (see JDK-8191893). I now defined separate visitors
>> for the outer projection and type argument projection parts, so that
>> the code should be easier to follow.
>>
>> * not all fresh types created during a projection overrode the
>> 'needStripping' method, which could lead to issue with type
>> annotation processing
>>
>> * the fact that TypeProjection extended from StructuralMapping had a
>> subtle issue - on the one hand, StructuralMapping has the required
>> logic to handle arrays (e.g. map component type and return new array
>> if needed) - on the other hand, with projections we need to be
>> careful - if the element type has no projection, then projection of
>> the array is also undefined. The impl was returning an array of
>> <null> which was then causing crashes inside the compiler. This is
>> now called out explicitly.
>>
>> Maurizio
>>
>>
>> On 24/11/17 17:39, Maurizio Cimadamore wrote:
>>>
>>> Hi,
>>> please review the fix for JDK-8191802:
>>>
>>> http://cr.openjdk.java.net/~mcimadamore/8191802/
>>>
>>> This is a conformance issue with local variable type inference - the
>>> specification text for upper projection says this (section 4.10.5):
>>>
>>> "The upward projection of a type T with respect to a set of
>>> restricted type variables is defined as follows:
>>>
>>> [...]
>>>
>>> If T is a parameterized class type or a parameterized interface
>>> type, G<A1, ..., An>, then the result is G<A1', ..., An'>, where,
>>> for 1 ≤ i ≤ n, Ai' is derived from Ai as follows:
>>>
>>> [...]
>>> If Ai is a type that mentions a restricted type variable,
>>> then Ai' is a wildcard. Let U be the upward projection of Ai. There
>>> are three cases:
>>> *If U is not Object, and if either the declared bound of the ith
>>> parameter of G, Bi, mentions a type parameter of G, or Bi is not a
>>> subtype of U, then Ai' is an upper-bounded wildcard, ? extends U.*
>>> Otherwise, if the downward projection of Ai is L, then
>>> Ai' is a lower-bounded wildcard, ? super L.
>>> Otherwise, the downward projection of Ai is undefined
>>> and Ai' is an unbounded wildcard, ?."
>>>
>>> The text in bold is not implemented accurately by javac. What javac
>>> used to do was simply to throw away the upper bound and favor the
>>> lower bound if the upper was Object.
>>>
>>> The spec text is much more subtle and precise, allowing javac to
>>> throw away upper bounds that do not add any extra information w.r.t.
>>> declared bounds.
>>>
>>> As a result of this change, there are few places where the compiler
>>> used to infer A<? extend B> (where B was same type as declared
>>> bound) and now it infers A<?> as per spec - this caused few changes
>>> in the jshell test TypeNameTest.
>>>
>>> I've added a lvti harness test to verify the assertions in the above
>>> paragraph.
>>>
>>> Cheers
>>> Maurizio
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20171128/d9fec177/attachment-0001.html>
More information about the compiler-dev
mailing list