[lworld] RFR: JDK 8251107 [lworld] test lworld-values/TopInterfaceNegativeTest.java needs scrubbing

[Srikanth Adayapalam]

On Wed, 5 Aug 2020 05:13:07 GMT, Srikanth Adayapalam <sadayapalam at openjdk.org> wrote:

>> I think the test still using the withdrawn InlineObject is a red herring. The golden files account for this already and
>> capture suitable errors for those uses of the non-existent interface.
>> The real trouble is due to an error NOT being emitted in line 33 of the TopInterfaceNegativeTest.java:
>> 
>> Given:
>> 
>> static inline class V2 implements InlineObject {}
>> 
>> void foo(V2 v) {
>>         if (v instanceof IdentityObject)  // line 33
>>             throw new AssertionError("Expected inline object but found identity object");
>>     }
>> 
>> v instanceof IdentityObject is patently false and can be ascertained at compile time to be patently false thereby
>> resulting an error message:
>> TopInterfaceNegativeTest.java:33:13: compiler.err.prob.found.req: (compiler.misc.inconvertible.types:
>> TopInterfaceNegativeTest.V2, java.lang.IdentityObject)
>> Given 'v' is type V2 and that is a final class, we known all the interfaces implemented by v and so can evaluate at
>> compile time whether the instanceof is patently bogus -
>> Now with the change for sealing of projections, this error is not emitted anymore and that is the reason for the
>> failure.
>> The error is valid and should be restored.
>
> Here is a smaller test case: without the change for sealing this elicits an error
> 
> public inline class X {
>     boolean b = new X() instanceof IdentityObject;
> }

Here is some analysis:

Earlier on, the reference projection type was incorrectly being marked as final. The fix for JDK- 8244315 correctly
removes the final modifier from V$ref.class.

While compiling the expression new X() instanceof IdentityObject we ask if X is castable to IdentityObject - if not
reject the instanceof as being implausible.

In isCastable TypeRelation visitor, in visitClassType visitor method circa line 1754 in Types.java is this crucial
block of code:

                 if (isValue(t)) {
                        // (s) Value ? == (s) Value.ref
                        t = t.referenceProjection();
                    }
This basically says whether a value instance is castable to type S is to be answered by asking whether its reference
projection is castable to S.  This switch is needed because down below we are going to be checking subtyping
relationships and inlines types are islands - they will answer false to any subtyping question - inline types are not
subtypes of j.l.O; they are not even subtypes of the interfaces they implement !!!

Reference: http://cr.openjdk.java.net/~briangoetz/valhalla/sov/02-object-model.html
nterfaces
Historically, for a class to implement an interface meant several things:

Conformance. The class has, as members, all the members of the interface.
Transitivity. Any subclasses of this class also implement the interface.
Subtyping. The class type is a subtype of the interface type.
We need to refine this last bullet, subtyping, in a small way to support inline classes; we say that the reference
projection of the inline class is a subtype of the interface type. (Identity class types are their own reference
projection, so this statement holds for all classes.) Similarly, if an inline class extends an abstract class, this
means that the reference projection is a subtype of the abstract class.
 
Now we have effectively transformed the question of whether X implements IdentityObject into whether X$ref implements
IdentityObject

And X$ref being final earlier (incorrectly) vs being non-final class now makes a material difference down below in this
block, circa  line 1818 in Types.java:
// Sidecast
                    if (s.hasTag(CLASS)) {
                        if ((s.tsym.flags() & INTERFACE) != 0) {
                            return ((t.tsym.flags() & FINAL) == 0)
                                ? sideCast(t, s, warnStack.head)
                                : sideCastFinal(t, s, warnStack.head);
                        } else if ((t.tsym.flags() & INTERFACE) != 0) {
                            return ((s.tsym.flags() & FINAL) == 0)
                                ? sideCast(t, s, warnStack.head)
                                : sideCastFinal(t, s, warnStack.head);
                        } else {
                            // unrelated class types
                            return false;
                        }
                    }


What used to be checked with sideCastFinal now gets checked with sideCast yielding a different result.

The key to the fix may be to take advantage of the fact that the set of interfaces implemented by the reference
projection is identical to that of the value projection.

-------------

PR: https://git.openjdk.java.net/valhalla/pull/135