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

[Vicente Romero]

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.

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