On Thu, 24 Jul 2025 06:07:31 GMT, John R Rose <jr...@openjdk.org> wrote:
>> Chen Liang has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 24 commits: >> >> - Merge branch 'master' of https://github.com/openjdk/jdk into >> exp/cds-mh-anno >> - Merge branch 'master' of https://github.com/openjdk/jdk into >> exp/cds-mh-anno >> - Reviews >> - Documentation >> - Don't fail for patching tests >> - Remove design document from code >> - Some more from reviews >> - Reviewed the diff on github >> - Stage >> - Missed comment updates >> - ... and 14 more: https://git.openjdk.org/jdk/compare/a201be85...d2dd466b > > Good fix. I added some minor comments. No logic changes requested! > > I'm a little surprised to see the second annotation has survived; I'd prefer > to see it totally nuked. But I am not asking you to remove it; there is an > argument that it is helpful. I requested better documdentation and checking > that the two annotations are properly interdependent. The "runtime setup" > annotation should not be allowed except in classes marked as AOT safe. > > In a few places you removed an unrelated "non-public" comment. That is not a > good cleanup to incorporate in passing, since the comment is actually > functional. > > If somebody is arguing that such comments are somehow detrimental, let them > make a separate argument and a separate PR to purge them. @rose00 I have re-asserted that AOTRuntimeSetup requires AOTSafeCI as a prerequisite and rolled back `/*non-public/` comment changes. Can you re-review? ------------- PR Comment: https://git.openjdk.org/jdk/pull/25922#issuecomment-3129885206
