RFR 8191802: Upward projection result is A<? extends Number> instead of A<? super Integer>
Vicente Romero
vicente.romero at oracle.com
Wed Nov 29 12:58:13 UTC 2017
On 11/29/2017 07:39 AM, Maurizio Cimadamore wrote:
>
>
>
> On 29/11/17 03:25, Vicente Romero wrote:
>> nit: at TypeArgumentProjection::visitType the code could be more
>> explicit:
>>
>> Type upper = t.map(TypeProjection.this, pkind);
>> Type lower = t.map(TypeProjection.this, pkind.complement());
>>
>> ||
>> \ /
>> \/
>>
>> Type upper = t.map(TypeProjection.this, ProjectionKind.UPWARDS);
>> Type lower = t.map(TypeProjection.this, ProjectionKind.DOWNWARDS);
>
> I think you are right - since that code doesn't make sense for pkind
> = downwards (in which case we prematurely exit with the null type),
> the remainder of this method can assume that pkind = UPWARDS and
> pkind.complement() = DOWNWARDS.
>>
>> side: the spec says that the downward projection is a partial
>> function so it could be assumed that the upward projection is a total
>> function, but then at Check::checkLocalVarType an error message is
>> issued if the compiler couldn't find the upward projection for a
>> given type. If this is possible then the spec should be updated
>> saying that the upward projection is also a partial function, if the
>> upward projection is a total function and the compiler fail to find
>> it because the initializer expression is incorrect, there is a self
>> reference to the variable, etc., then IMO the error message should
>> make a reference to the incorrect initializer expression as the root
>> cause of the problem. The fact that the upward projection couldn't be
>> determined is just a consequence of that and there shouldn't be an
>> additional error message to state that.
> upper projection is a total function - downward is not. The check you
> see in checkLocalVarType is not to check that the outcome of upward
> projection is defined (it always should be) - but is to check as to
> whether the type of the initializer expression is the null type, in
> which case it should be an error. I agree that as things are now is a
> bit messy because the code does the check AFTER the projection, but
> this is relying on the fact that the projection doesn't alter those
> types. Anyway, if it makes things clearer, I can move the check for
> VOID and NULL to _before_ the projection. Would that help?
yes I think that moving the check before the projection would make more
sense
>
> Maurizio
Vicente
>>
>> Thanks,
>> 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/20171129/bf689f9b/attachment-0001.html>
More information about the compiler-dev
mailing list