On Fri, 25 Jul 2025 14:38:18 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> wrote:
>> Jan Lahoda has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - Fixing toString value for flags that have ACC_ overloaded flags. >> - Review feedback: marking ACC flags with the @Use annotation, forces split >> of the TYPE target into CLASS/MODULE/PACKAGE/TYPE_VARIABLE. >> - Reflecting review feedback: >> - when conflict is detected, the generator fails >> - adding runtime checks >> - using constants for number of bits > > There's a lot to like in this patch. Now flags cannot erroneously overlap, > which will give us all javac developers more confidence that we're not > stepping into each other toes. Also, by making the mechanism more formally > correct, it's easier to see which "holes" we have available (and there's > probably quite a lot left). > > The only thing I'm unsure of, as pointed out in the review, is the dynamic > checks. I believe that, as far as this PR is concerned, this feels like > "scope creep". I think it would be cleaner if this PR only concerned about > introducing the machinery. Then maybe we can brainstorm separately on what > would be the best way to add some of these checks (but I'm skeptical that we > can find anything that doesn't feel like a compromise). > > For instance, another way to do this could be to do another combo test like > the tree test, where we compile all the existing tests, and check the > validity of all the flags on all the symbols. That's an offline way to make > sure that what we do in javac is "sort of" correct, but one that doesn't leak > into the code. > > The real fix for this, as we know (and as @archiecobbs commented) is to move > away from longs, but that's not what this PR is about. Thanks @mcimadamore for the review. I think the build changes are now ready to be reviewed. Thanks! ------------- PR Comment: https://git.openjdk.org/jdk/pull/26452#issuecomment-3127824873