On Tue, 21 Apr 2026 20:54:56 GMT, Joe Darcy <[email protected]> wrote:
>> David Beaumont has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 19 additional >> commits since the last revision: >> >> - Merge branch 'lworld' into jdk_8377101_gensrc/squashed >> - Make AP swallow all unprocessed annotations >> - Remove unwanted lint warning suppression >> - Merge branch 'lworld' into jdk_8377101_gensrc/squashed >> - Finally working >> - Latest changes (inc. feecback). >> >> * feedback updates and tidying >> * better compile macro call - still not working >> - [[UNDO FORMATTING IN SQUASH BRANCH]] >> - [[AUTOMATIC FORMATTING]] >> - better but still not working >> - Simpler temp directory >> - ... and 9 more: >> https://git.openjdk.org/valhalla/compare/3bcd5d1b...6b9608a8 > > make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java > line 83: > >> 81: * @implNote This annotation currently accepts and consumes <em>all</em> >> 82: * annotations. This is a temporary situation to avoid unwanted warnings >> about >> 83: * unrelated annotations. See JDK-8382613 for more details. > > Note as a convention, we don't typically include bug numbers in comments in > this fashion. Yeah, I know. It's temporary though. I'll change it if you want, but I see not downside to giving a proper reference for the comment (unless you know of a better way, such as the bug URL). > make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java > line 93: > >> 91: static final String OUTDIR_OPTION_KEY = "valueclasses.outdir"; >> 92: >> 93: private ProcessingEnvironment processingEnv = null; > > There is a protected processingEnv field define in the superclass you can use > instead. I didn't spot it was visible. Ta. > make/jdk/src/classes/build/tools/valhalla/valuetypes/GenValueClasses.java > line 128: > >> 126: Optional<? extends TypeElement> valueClassAnnotation = >> 127: getAnnotation(annotations, >> MIGRATED_VALUE_CLASS_ANNOTATION); >> 128: if (valueClassAnnotation.isPresent()) { > > I would expect a call to RoundEnvironment.getElementsAnnotatedWith() to be > used in this method. Is there a reason for avoiding that? > > If you need to operate on the _files_ containing the annotated types, you > could use Elements.getFileObjectOf to create a set of them. I'm not super familiar with APs, so it's probably just that. Let me have a look... ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2180#discussion_r3124416666 PR Review Comment: https://git.openjdk.org/valhalla/pull/2180#discussion_r3124431966 PR Review Comment: https://git.openjdk.org/valhalla/pull/2180#discussion_r3124421002
