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

[Vicente Romero]

On 07/30/2018 01:20 PM, Maurizio Cimadamore wrote:
>
>
> 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.

got it, then I guess we have no option but waiting for the spec issue to 
be fixed

>
> Maurizio

Vicente

>>
>>>
>>> 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() {
>>>>
>>>
>>
>