RFR JDK-8206142: type inference: javac is incorrectly applying capture conversion during incorporation

[Maurizio Cimadamore]

On 30/07/18 19:00, Vicente Romero wrote:
>
>
> On 07/30/2018 07:25 AM, Maurizio Cimadamore wrote:
>> Hi Vicente,
>> while I agree that the program in JDK-8206142 exposes an issues with 
>> a spurious capture being performed by javac, and, while I agree that, 
>> in that case, we can do without the capyure (both the source and the 
>> target of the test are some B<...>), in the more general case where 
>> source and target are at different heights, you need to recurse to 
>> supertype and, hence, you need capture.
>>
>> So, I'm afraid that with this we will potentially introduce 
>> compatibility issues, w/o fully resolving the underlying issue, which 
>> is described by this spec issue which Dan filed while ago:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8016196
>>
>> Maybe we should wait on this one?
>
> not sure here, it could be a good idea to wait but usually these spec 
> issues take some time to be sorted out, so we could pause now and push 
> this fix later if more users complain about this issue.
Right. But the point I was trying to make is that, if you never capture 
(as in this patch), when you have a subtyping test like this:

A<? super T> <: B<M>

where

A<X> <: B<C<? extends X>>

you end up with the following, non-sensical subtyping assertion:

B<C<? extends ? super T> <: B<M>

So, javac always applies capture conversion (even at risk of 
incompatibilities like the one in this issue) in order to remain on a 
safe path, from a type-wellformedness perspective.

Maurizio
>
>>
>> Maurizio
>
> Vicente
>
>>
>>
>>
>> On 23/07/18 14:22, Vicente Romero wrote:
>>>
>>>
>>> On 07/21/2018 09:49 AM, B. Blaser wrote:
>>>> Hi Vicente,
>>>>
>>>> On 20 July 2018 at 22:27, Vicente Romero 
>>>> <vicente.romero at oracle.com> wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review the patch to fix [1] at [2]. The fix is putting 
>>>>> javac in sync
>>>>> with the spec by not applying capture conversion during 
>>>>> incorporation.
>>>>>
>>>>> Thanks,
>>>>> Vicente
>>>>>
>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8206142
>>>>> [2] 
>>>>> http://cr.openjdk.java.net/~vromero/8206142/webrev.00/jdk.dev.patch
>>>> It's hard to measure the potential side-effects of this fix,
>>>
>>> yes, I'm also worried about the possible side effects. I agree that 
>>> more discussion will be needed,
>>>
>>>>   but it
>>>> seems good to me even if you'll probably need another review.
>>>> Maybe, I'd rewrite it as next to avoid creating a new method?
>>>>
>>>> Thanks,
>>>> Bernard
>>>
>>> Thanks,
>>> Vicente
>>>
>>>>
>>>>
>>>> diff -r b0fcf59be391
>>>> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
>>>> --- 
>>>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
>>>>         Fri Jul 20 14:48:41 2018 -0700
>>>> +++ 
>>>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java
>>>>         Sat Jul 21 14:33:05 2018 +0200
>>>> @@ -1003,24 +1003,24 @@
>>>>        * Is t an unchecked subtype of s?
>>>>        */
>>>>       public boolean isSubtypeUnchecked(Type t, Type s, Warner warn) {
>>>> -        boolean result = isSubtypeUncheckedInternal(t, s, true, 
>>>> warn);
>>>> +        boolean result = isSubtypeUnchecked(t, s, true, warn);
>>>>           if (result) {
>>>>               checkUnsafeVarargsConversion(t, s, warn);
>>>>           }
>>>>           return result;
>>>>       }
>>>>       //where
>>>> -        private boolean isSubtypeUncheckedInternal(Type t, Type s,
>>>> boolean capture, Warner warn) {
>>>> +        public boolean isSubtypeUnchecked(Type t, Type s, boolean
>>>> capture, Warner warn) {
>>>>               if (t.hasTag(ARRAY) && s.hasTag(ARRAY)) {
>>>>                   if (((ArrayType)t).elemtype.isPrimitive()) {
>>>>                       return isSameType(elemtype(t), elemtype(s));
>>>>                   } else {
>>>> -                    return isSubtypeUncheckedInternal(elemtype(t),
>>>> elemtype(s), false, warn);
>>>> +                    return isSubtypeUnchecked(elemtype(t),
>>>> elemtype(s), false, warn);
>>>>                   }
>>>>               } else if (isSubtype(t, s, capture)) {
>>>>                   return true;
>>>>               } else if (t.hasTag(TYPEVAR)) {
>>>> -                return isSubtypeUncheckedInternal(t.getUpperBound(),
>>>> s, false, warn);
>>>> +                return isSubtypeUnchecked(t.getUpperBound(), s, 
>>>> false, warn);
>>>>               } else if (!s.isRaw()) {
>>>>                   Type t2 = asSuper(t, s.tsym);
>>>>                   if (t2 != null && t2.isRaw()) {
>>>> diff -r b0fcf59be391
>>>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java
>>>> --- 
>>>> a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java
>>>>         Fri Jul 20 14:48:41 2018 -0700
>>>> +++ 
>>>> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Infer.java
>>>>         Sat Jul 21 14:33:05 2018 +0200
>>>> @@ -1180,7 +1180,7 @@
>>>>           IS_SUBTYPE() {
>>>>               @Override
>>>>               boolean apply(Type op1, Type op2, Warner warn, Types 
>>>> types) {
>>>> -                return types.isSubtypeUnchecked(op1, op2, warn);
>>>> +                return types.isSubtypeUnchecked(op1, op2, false, 
>>>> warn);
>>>>               }
>>>>           },
>>>>           IS_SAME_TYPE() {
>>>
>>
>