On Wed, 5 Aug 2026 14:38:18 GMT, Chen Liang <[email protected]> wrote:
>> Reopened Valhalla PR which did not go in before the code freeze >> (openjdk/valhalla#2405). >> >> The original PR was review by @jsikstro and @johan-sjolen. >>> There are few places which uses the fully qualified name for the >>> AsValueClass annotation. As a result the plugging does not modify these >>> classes when compiling, so they are still identity classes. >>> >>> I propose improving the robustness of this plugin. We need to do this >>> during parsing so we cannot actually check 100% that it will resolve to the >>> correct annotation. However we can do a best effort, which handles same >>> package, fully qualified, imported and rejects other annotations with the >>> same class name. >> >> This only adapts the current ValueClassPlugin to be more robust, there might >> be room for improving how we do this Value class transformation in some >> other way. >> >> * Testing >> * Verified that enable preview testing classes are transformed, including >> `gc/stress/gcbasher` which was missed before this change. >> * Testing tests with annotation with and without enable preview >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > test/jtreg_value_class_plugin/plugin/jdk/test/valueclass/ValueClassPlugin.java > line 81: > >> 79: public void visitClassDef(JCClassDecl tree) { >> 80: boolean hasAnnotation = >> tree.mods.annotations.stream() >> 81: .anyMatch(a -> >> a.annotationType.toString() > > I think maybe you can check `a.annotationType.type.toString()`? That should > be the fully-qualified class name of `AsValueClass` and you should be able to > drop the complex checks with imports and everything. This plugin runs during parsing when the type has not yet been resolved and set. I am not fully aware of all the reasons that we decided to do this during parsing. But it seems like `javac` consumes the information we are modifying before it does its analysis which figures out the type and populates the type field. I think if we want to be able to do this later we would have to either change javac, or mimic what javac does here and not only fix-up what we already do, but also repair any derived properties. I think doing it like this is a pragmatic albite hacky solution. It would be nice if there was a more elegant solution here, but that is not a solution I can currently see. (But I am very much a newcomer to the javac code and tooling) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32214#discussion_r3726688040
