RFR: 8224629: Unnecessary indirection in LambdaToMethod

[Alan Malloy]

New patch removes the cast but leaves the method call. Please take a look.

http://cr.openjdk.java.net/~cushon/8224629/webrev.01/

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

>
>
> On 5/28/19 12:50 PM, Alan Malloy wrote:
>
> We already have a MethodHandleSymbol, though. You're envisioning something
> like this?
>
> class FancySymbol extends MethodHandleSymbol {
>     public MethodHandleSymbol asHandle() {
>         return new DelegatingFancySymbol(this);
>     }
> }
>
>
> it would be more common to create an anonymous inner class, but yes the
> idea is the same.
>
>
> 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.
>
>
> yep I agree on removing the upcast,
>
> Vicente
>
>
> 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/20190531/d3cedd5e/attachment.html>