RFR: 8224629: Unnecessary indirection in LambdaToMethod

[Alan Malloy]

We already have a MethodHandleSymbol, though. You're envisioning something
like this?

class FancySymbol extends MethodHandleSymbol {
    public MethodHandleSymbol asHandle() {
        return new DelegatingFancySymbol(this);
    }
}

I'd argue that if FancySymbol wants to require you to call asHandle()
before you can treat it as a MethodHandleSymbol, it should have made itself
a MethodSymbol, not a MethodHandleSymbol. Then, of course clients would be
required to use its asHandle method, invoking whatever non-obvious logic it
represents, in order to get a MethodHandleSymbol. I don't see any value in
catering to classes like this. If someone someday really does add
FancySymbol and needs all users of MethodHandleSymbol to call asHandle() on
all instances, they'll have more use sites than this one to refactor (right
now, I see LambdaToMethod.addDeserializationCase, PoolWriter.writeConstant,
and perhaps DynamicMethodSymbol will need some rethinking).

But if you are sure, then we should at least remove the meaningless upcast.

On Tue, May 28, 2019 at 9:29 AM Vicente Romero <vicente.romero at oracle.com>
wrote:

> I think that I prefer the flexibility that invoking the ::asHandle
> method brings. Suppose that a subclass wants to provide an
> implementation of the asHandle method that will return the non-obvious
> result. Not a high probability case but we have seen similar uses to the
> Symbol::baseSymbol method. I think I prefer to have options,
>
> Vicente
>
> On 5/22/19 8:50 PM, Alan Malloy wrote:
> > Hello. Please review this patch to eliminate an unnecessary upcast and
> > method call in LambdaToMethod.
> >
> > bug: https://bugs.openjdk.java.net/browse/JDK-8224629
> > webrev: http://cr.openjdk.java.net/~cushon/8224629/webrev.00/
> >
> > Testing: All of jtreg:test/langtools/tools/javac pass locally. No new
> > test added, as no behavior change is intended.
> >
> > Thanks,
> >     Alan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20190528/dd44da11/attachment.html>