LUB computation involving NULL

[B. Blaser]

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