RFR: 8334037: Local class creation in lambda in pre-construction context crashes javac [v2]

[Jan Lahoda]

On Mon, 24 Jun 2024 14:47:22 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address review comments
>>   Erase lambda functional return and simplifify lambda visitor in `Lower`
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 3896:
> 
>> 3894:         Type prevRestype = currentRestype;
>> 3895:         try {
>> 3896:             currentRestype = types.findDescriptorType(tree.type).getReturnType();
> 
> We no longer need to mess with `variableIndex` in the new code: if the lambda is a statement lambda, this will call `visitBlock` which will update `variableIndex` accordingly

I probably miss something, but where is the return type erased?

> src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java line 1642:
> 
>> 1640:                 return;
>> 1641: 
>> 1642:             if (scanner.hasLambdas) {
> 
> This is another bit I'm not sure about - why do we need a scanner to check if there are lambdas? I think we added this because there were concerns over running "too many visitors", but today we have many more, and we don't have scanners for them, so... (again, this is something for another PR)

IIRC, this was added because the new phase slowed down performance running of very small, simple, testcase(s) without any lambda (https://bugs.openjdk.org/browse/JDK-8026936).

We also do the same for patterns/`TransPatterns`.

To what degree this is needed currently is not obvious.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19836#discussion_r1651467363
PR Review Comment: https://git.openjdk.org/jdk/pull/19836#discussion_r1651215443