On Mon, 4 Dec 2023 21:27:57 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:
>> As part of: >> https://github.com/openjdk/jdk/pull/16505 >> >> there are new symbol files for JDK 22, and @jddarcy noted the content looks >> weird. >> >> I was investigating, and found a few problems, some introduced by >> https://github.com/openjdk/jdk/commit/fc314740e947b2338ab9e4d4fce0c4f52de56c4b, >> some pre-existing. >> >> Note that >> https://github.com/openjdk/jdk/commit/fc314740e947b2338ab9e4d4fce0c4f52de56c4b >> changed the way we generate `ct.sym` - it now contains `.sig` files also >> for the current JDK, not only for the past versions. Before this patch, >> `ct.sym` only contained a list of permitted modules for the current JDK, and >> the classfiles themselves were read from `lib/modules`. >> >> The problems (and their solution) I've attempted to tackle here: >> - since >> https://github.com/openjdk/jdk/commit/fc314740e947b2338ab9e4d4fce0c4f52de56c4b, >> the ct.sym accidentally includes all modules for the current release, >> including non-API/undocumented modules. The proposed solution is to pass the >> documented modules together with their transitive dependencies as a >> parameter when constructing ct.sym, then use them to only generate data for >> these modules. >> - there are tiny differences between the data that are in `ct.sym` and in >> the `.sym.txt` historical files, mostly around annotations. The `.sym.txt` >> files contain references to internal annotations (like `@PreviewFeature`), >> which are stripped or converted before the `.sig` file is written into >> `ct.sym`. When generating historical data for JDK N, we run `CreateSymbols` >> on JDK N, reading the JDK N classfiles, and producing `.sym.txt`. This is >> done using `--release N`, so that we read the correct span of modules. >> Sadly, since now `--release N` will always use the `.sig` files, we are >> loosing a little bit of information esp. around the annotations. The >> proposal here is to use `--release N` to read the list of modules to cover, >> and `--source N` to read the actual real classfiles from `lib/modules`. This >> should basically revert the behavior to the previous state. >> - this is an existing issue, but one that needs to be fixed. Sealed types >> are not written and read properly - writing was not storing them, and >> reading permitted subclasses was happening too late, not on the `header` >> line. Note that when fixing this, we now must store some of the non-exported >> elements, which are reachable through the permitted subclasses, so that >> casting works as expected. Also, since the historical record is incorrect >> here, I re-run the generator ... > > Jan Lahoda has updated the pull request incrementally with one additional > commit since the last revision: > > Reflecting review feedback - keeping CT_TRANSITIVE_MODULES separate from > CT_MODULES. Build changes look fine now. ------------- Marked as reviewed by ihse (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/16942#pullrequestreview-1764783110
