RFR: 8295020: javac emits incorrect code for for-each on an intersection type.

[Srikanth Adayapalam]

On Fri, 14 Oct 2022 04:16:33 GMT, Srikanth Adayapalam <sadayapalam at openjdk.org> wrote:

> Eliminate needless casts and ensure invoked method is looked up against the appropriate receiver type

Notes for the reviewer:

The loop from the test case viz:


for (Object s : (MyIterable & Serializable) null) {}


gets transformed by Lower into:


for (Iterator i$ = ((Iterable)(Main$MyIterable & Serializable)null).iterator(); i$.hasNext(); ) {
    Object s = (Object)i$.next();
    {
    }
}


This transformation is problematic in a couple of ways:

    The cast of the expression (Main$MyIterable & Serializable)null into (Iterable) is not
    required at all. After erasure the type of the intersection cast node is actually
    Main$MyIterable which is already an Iterable. As a matter of Gen#visitTypeCast correctly
    deduces this to be a redundant cast and does not emit a checkcast in the class file.

    The iterator() method to be invoked is looked up against the JCEnhancedForLoop#expr.type
    i.e from Main$MyIterable while the type of the receiver expression for the iterator()
    method invocation is the casted type of (Iterable)(Main$MyIterable & Serializable)null).
    As a result, the method
    com.sun.tools.javac.jvm.Gen#binaryQualifier((Symbol sym, Type site)
    attempts to coalesce the method iterator() (sym) as owned by Iterator (site) by calling
    com.sun.tools.javac.code.Symbol.MethodSymbol#clone which simply changes ownership of
    the method symbol iterator() without accommodating for descriptor difference that arises
    due to covariant return type.


The fix comes in two parts: (a) Ensuring that a cast is inserted only when absolutely necessary
and not just because JCEnhancedForLoop#expr.type happens to be an intersection type (b) Ensuring
that where a type cast is inserted, the iterator() method is looked up against the modified
receiver type.

(An open question is whether MethodSymbol#clone should be fixed to not just rewire ownership, but also
account for differences in descriptor - if deemed needed this can be followed up in a separate
ticket.)

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

PR: https://git.openjdk.org/jdk/pull/10710