On Thu, 5 Jun 2025 21:06:44 GMT, Radim Vansa <rva...@openjdk.org> wrote:
>> src/hotspot/share/oops/fieldInfo.hpp line 238:
>>
>>> 236:
>>> 237: private:
>>> 238: uint32_t next_uint() { return _r.next_uint(); }
>>
>> Why did you make this change and have the callers expose _r ?
>
> AFAIU `_r` is not exposed, it's private. My change removes `next_uint()`
> because it's at wrong level of abstraction: `FieldInfoReader` should expose
> things java/injected fields counts (and field info itself), not just some
> 'uint's.
> The encapsulation is imperfect as the methods have to be called anyway only
> in the correct order but to me it seemed as a way forward.
Okay I see why you made this change. Removing the friends was good because it
removes other callers that shouldn't call next_uint(), but the callers within
FieldInfoStream now have to call _r.next_uint() which is extra exposure to the
name _r, that could be a lot more descriptive. I don't see what's wrong with
FieldInfoStream calling next_uint() a private method that has access to the
Unsigned5 stream.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24847#discussion_r2132424315
