LUB computation involving NULL

[Maurizio Cimadamore]

I agree that the lub implementation should at least check for absence of 
null types (since that's not how this function is supposed to be used).

Maurizio

On 12/10/2020 13:09, B. Blaser wrote:
> Hi Maurizio,
>
> On Mon, 12 Oct 2020 at 12:32, Maurizio Cimadamore
> <maurizio.cimadamore at oracle.com> wrote:
>>
>> On 10/10/2020 15:43, B. Blaser wrote:
>>> Hi,
>>>
>>> This one is interesting.
>>>
>>> Unless I read JLS15 too quickly, neither §15.28.1 mandates explicitly
>>> to skip null types before computing the LUB:
>>>
>>> "Otherwise, boxing conversion (§5.1.7) is applied to each result
>>> expression that has a primitive type, after which the type of the
>>> switch expression is the result of applying capture conversion
>>> (§5.1.10) to the least upper bound (§4.10.4) of the types of the
>>> result expressions."
>>>
>>> nor §4.10.4 explicitly specifies how to handle them.
>>>
>>> So, ignoring null types not to interfere with this computation seems
>>> implicitly deduced from the fact that they can match any reference
>>> type and Jan's pull request [1] looks therefore reasonable.
>>>
>>> However, compilation currently succeeds with the first arm being null
>>> (as initially reported), suggesting probably to fix this more directly
>>> in Types::lub as here under (langtools:tier1 being OK on jdk14u).
>> For method type inference, we already filter types before applying lub.
>> I think we should do the same here. Of course we could also fix deeper,
>> but I think it's better to start simple.
>>
>>   From a spec perspective, I think the filtering is implicit in 18.2.
>>
>> ```
>> A constraint formula of the form ‹S <: T› is reduced as follows:
>>
>>       ...
>>
>>       Otherwise, if S is the null type, the constraint reduces to true.
>> ```
>>
>> In other words, if the actual type (S) has the null type, we never even
>> generate a bound for lub to contemplate - that subtyping constraint is
>> instead implicitly satisfied. Which seems to suggest a pre-filtering
>> step (which is what the compiler does).
>>
>> Maurizio
> For method type inference, I agree that §18.2 is clear enough not to
> involve 'null' in the LUB computation but §15.28.1 might be more
> explicit, at my mind. Another option on spec side would be to
> factorize this logic directly in §4.10.4 (LUB computation).
>
> On an implementation perspective and without any precise requirement
> from the spec, I believe this would be more robust to have the 'null'
> handling directly in Types::lub or at least a comment to warn about
> that.
>
> But for now on, I agree that Jan's solution would be suitable enough,
> although I strongly believe some refactoring would be necessary in
> this area since the following lines seem still obscure to me, as said
> previously:
>
> @@ -3970,33 +3970,29 @@
>
>           case CLASS_BOUND:
>               // calculate lub(A, B)
> -            int startIdx = 0;
> -            for (int i = 0; i < ts.length ; i++) {
> -                Type t = ts[i];
> -                if (t.hasTag(CLASS) || t.hasTag(TYPEVAR)) {
> -                    break;
> -                } else {
> -                    startIdx++;
> -                }
> -            }
> -            Assert.check(startIdx < ts.length);
>
> Should I file a JBS issue to keep track of this along with the patch
> I'm suggesting?
>
> Bernard