On Wed, 9 Sep 2026 19:22:27 GMT, Ioi Lam <[email protected]> wrote: > > I think that there's a relative easy change that can be made to keep > > FieldClosure "clean", and I have a version of that here: > > Diff: > > [stefank/jdk@pull/32565...stefank:jdk:alt_32565](https://github.com/stefank/jdk/compare/pull/32565...stefank:jdk:alt_32565) > > Branch: > > [master...stefank:jdk:alt_32565](https://github.com/openjdk/jdk/compare/master...stefank:jdk:alt_32565) > > The patch moves the fields added to FieldClosure out to its own struct, > > that I call `ValuePayloadContext`. Whenever the code is descending into a > > flat field/element, it sets up a new `ValuePayloadContext` to describe the > > current value payload that is being iterated. > > Hi Stefan, this does look a lot cleaner. I've integrated your patch into this > PR. > > Also, as you fixed the comment about `this->field_holder()` in > `fieldDescriptor::field_offset_in_obj()`, I realized that the only place we > use `vpc->klass()` outside of asserts is in `field_offset_in_obj()`, but we > already know what this class should be. So I put `vpc->klass()` inside > `DEBUG_ONLY`. > > I think it's still worth having `vpc->klass()` to make the code more > readable. In the future, if `ValuePayloadContext` is used in other places > without a `fieldDescriptor`, we can remove the `DEBUG_ONLY` macros. > > BTW, I removed your comment `(in other cases this could be an abstract value > class)`. As far as I know, a flattened field cannot be of an abstract value > type. E.g., this field will not be flattened.
Unfortunately, this doesn't work. I actually tried this first, passing the offset only instead of the vpc and relying on the field_holder(), but that is wrong. The code could be visiting a field inside an abstract value class and when that happens field_holder() doesn't match vpc.klass(). It is easy to trigger the assert by writing a test that creates an abstract value class with a field, then a concrete value class that inherits from it, put it in a holder object, and then route the holder into the print fields code. ------------- PR Comment: https://git.openjdk.org/jdk/pull/32565#issuecomment-5616035680
