On Wed, 10 Dec 2025 13:45:31 GMT, Jan Lahoda <[email protected]> wrote:
>> 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 > > In practice, as long as the `JavacFileManager` and `ClassFinder` agree on the > `JRTIndex` instance (which they should), then it should be fine checking that > the file originates in the one specific instance of JRT FS that is held by > the `JRTIndex`. If the file manager is not a JavacFileManager, `jrtIndex` > will be `null`, so custom file managers are ruled out here, I think. I added comments. Thanks for explaining, I am much more confident this is correct now. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/1761#discussion_r2622797735
