RFR: 8336492: Regression in lambda serialization [v6]

[Maurizio Cimadamore]

On Mon, 29 Jul 2024 16:09:53 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Simplify translation contexts
>>  - Address review comment
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java line 347:
> 
>> 345:     public void visitLambda(JCLambda tree) {
>> 346:         LambdaTranslationContext localContext = new LambdaTranslationContext(tree);
>> 347:         if (dumpLambdaToMethodStats) {
> 
> probably in a follow-up fix, but I don't see too much value on keeping / growing this stats code. This seems to belong in a regression test that probably uses a plugin or similar to get some stats while compiling lambdas / mrefs

Actually... in an attempt to remove the translation context classes (and just keep the lambda translation context class - after all method references don't need much there), I stumbled upon a bad test failure in the lambda deduplication test. This test depends on the compiler generating the stat diagnostics for every encountered lambda... maybe we can drop that dependency and the stats, but probably better to do it elsewhere.

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java line 1188:
> 
>> 1186:                     break;
>> 1187:                 case LOCAL_VAR:
>> 1188:                     ret = new VarSymbol(sym.flags() & FINAL, sym.name, sym.type, translatedSym);
> 
> shouldn't this symbol be synthetic too? same for the param below

I suppose local variables and parameters come from "real" symbols in the source code. We could probably do either way, but for now I preferred not to touch this stuff

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20349#discussion_r1695554139
PR Review Comment: https://git.openjdk.org/jdk/pull/20349#discussion_r1695551862