[lworld] RFR: 8372515: [lworld] Plumb in javac flags for compiling with preview mode [v2]

Maurizio Cimadamore mcimadamore at openjdk.org
Wed Dec 10 11:23:51 UTC 2025


On Tue, 25 Nov 2025 17:03:57 GMT, David Beaumont <duke at openjdk.org> 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/JRTIndex.java line 341:
> 
>> 339: 
>> 340:     public boolean isInJRT(FileObject fo) {
>> 341:         if (fo instanceof PathFileObject pathFileObject) {
> 
> This is surprisingly subtle and needs someone much more familiar with javac, FileObject and other things to look at. This method is used exactly once in ClassFinder to see if the path of a FileObject "came from" the JRT index.
> 
> Previously any FileObject created with a path that come from the global jrt file system would cause this to return "true". Now, it might not, because preview mode and non-preview mode no longer share a file system instance.
> 
> This feels correct in the sense that "the two classes could have different content in different modes", but it's bad if anyone wants flags for a class that wasn't changed by preview mode. In that case, the code will now fail to do anything (whereas before it would):
> 
> --------
> 
>     long getSupplementaryFlags(ClassSymbol c) {
>         if (jrtIndex == null || !jrtIndex.isInJRT(c.classfile) || c.name == names.module_info) {
>             return 0;
>         }
>         ...
>     }
> 
> --------
> 
> So it's not at all clear to me if this is an issue or not. If ClassSymbols for classes in the JRT image which get passed to getSupplementaryFlags() always come from **this index instance**, then it should be okay, but otherwise it's subtly broken. It depends how people can define ClassSymbol instances, and what control they have over the paths of the file objects they use.

My feeling here is that this shouldn't matter.

First, I think the method seems consistent -- e.g. `isInJRT` is an instance method on a JRTIndex, and returns true if the provided file is defined in the file system backing _that specific_ JRTIndex. Which seems correct.

Second, in reality there's only one ClassFinder per javac instantiation. Which means the class finder will either run in preview mode or not -- effectively only seeing one type of file system. In other words, I don't think it's possible for javac to witness different classfiles for the same symbol.

This might be possible in cases like combo tests, where we try to reuse the existing compilation context for multiple compilation rounds (which allows us to run thousands of javac compilation in seconds). But in that test framework, whenever a "sensitive" option is detected, javac dutifully marks its context as "not good for reuse", and so it throws it away and creates a new one (and a new classfinder).

But, @lahodaj  please chime in too

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2606254293


More information about the valhalla-dev mailing list