RFR: 8247912: Make narrowOop a scoped enum
Kim Barrett
kim.barrett at oracle.com
Tue Sep 22 13:14:58 UTC 2020
> On Sep 21, 2020, at 5:32 PM, Ioi Lam <iklam at openjdk.java.net> wrote:
>
> On Mon, 21 Sep 2020 04:01:34 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>
>> Please review this change to the type narrowOop from a typedef for juint to a
>> scoped enum with uint32_t as the representation type. This provides stronger
>> type checking when using this type.
>> […]
>
> This looks good overall to me.
>
> src/hotspot/share/oops/compressedOops.hpp line 142:
>
>> 140: // Shift by 32 is UB if size in bits of i is 32, e.g. on 32bit platform.
>> 141: assert(((i >> 16) >> 16) == 0, "narrowOop overflow");
>> 142: return static_cast<narrowOop>(static_cast<NarrowType>(i));
>
> Should we have an assert that further restricts the range according to max heap size?
I’m updating this, in part in light of your question below.
> Also, should we allow only unsigned types for T? I think using signed types here is prone to error and should be
> flagged.
It turns out we can’t restrict it to unsigned types, at least not very easily. For example,
CDS calls this with an intptr_t value. I suppose CDS could do conversion itself before
calling this, but that seems a little odd. And I don’t know how many more there are after
changing that one.
> Changes requested by iklam (Reviewer).
>
> PR: https://git.openjdk.java.net/jdk/pull/273
More information about the shenandoah-dev
mailing list