RFR: 8224629: Unnecessary indirection in LambdaToMethod

[Vicente Romero]

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 <mailto: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/59c8b409/attachment-0001.html>