On Wed, 10 Dec 2025 10:51:06 GMT, Maurizio Cimadamore <[email protected]>
wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove note about StableValue (not possible)
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java line 1:
>
>> 1: /*
>
> Most of the changes in this files are stylistic. I'd strongly recommend (in
> the future) to separate those out, so that the "meat" of the PR is more
> self-evident. After all, filing multiple PRs is cheap :-)
Oh, for some reason it's got the reformatted version of the file I use for
editing, not the "minimal diff" version I normally push. I can undo that. Sorry.
> src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java line
> 151:
>
>> 149:
>> 150: private void addCloseable(Closeable c) {
>> 151: synchronized (closeables) {
>
> Here and elsewhere -- I'm not sure about `synchronized`. In general, javac
> operates under the assumption of "single threaded-ness" -- but I see around
> the file manager code that `synchronized` seems to be used. I'll leave this
> to @lahodaj
I think in the compiler you can't assume single threaded-ness, and unless this
adds a deadlock risk (which I am pretty sure it doesn't) it's definitely my
preference to have correctness and avoid weird bugs/failures.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1761#discussion_r2619986996
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1761#discussion_r2619992479
