RFR: 8224629: Unnecessary indirection in LambdaToMethod

[Vicente Romero]

looks good,
Vicente

On 5/31/19 4:25 PM, Alan Malloy wrote:
> 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 <mailto: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 <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/20190531/a2b4730d/attachment-0001.html>