On Sun, 3 Dec 2023 21:31:42 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 for JDK 17-21 (as sealed c...

looks sensible to me

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

Marked as reviewed by vromero (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16942#pullrequestreview-1763256790

Reply via email to