RFR: 8355753: @SuppressWarnings("this-escape") not respected for indirect leak via field [v2]

[Archie Cobbs]

On Thu, 29 May 2025 17:50:37 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> Archie Cobbs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Add additional tests for warnings generated from fields.
>>  - Merge branch 'master' into JDK-8355753
>>  - Refactor ThisEscapeAnalyzer to correct bug in suppression logic.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java line 387:
> 
>> 385:         // Sort warnings so redundant warnings immediately follow whatever they are redundant for, then remove them
>> 386:         warningList.sort(Warning::sortByStackFrames);
>> 387:         AtomicReference<Warning> previousRef = new AtomicReference<>();
> 
> just curious: why do we need to deal with AtomicReference here?

Hi @vicente-romero-oracle, thanks for taking a look.

The reason for the `AtomicReference` is that this lamba needs to be stateful (it needs to remember the most recent previous `Warning` that was also retained, if any), so if it wants to remain a lambda (i.e., and not a more verbose inner class) that state needs to be declared outside the lamba, and therefore it must be effectively final, which means there must be an extra level of indirection, etc. I guess it's a verbosity vs. clarity trade-off.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24932#discussion_r2114770332