RFR: 8247912: Make narrowOop a scoped enum
Roman Kennke
rkennke at openjdk.java.net
Mon Sep 21 10:01:29 UTC 2020
On Mon, 21 Sep 2020 08:32:14 GMT, Roman Kennke <rkennke 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.
>>
>> For the most part this was fairly straightforward, and the patch size is
>> relatively small. The implementation of some existing CompressedOops
>> "primitives" required adjustment. An explicit conversion to narrowOop was
>> added, with casts change to use it. There were a few places that were type
>> punning and needed explicit conversions,, mostly in platform-specific assembly
>> support.
>>
>> There are a couple of lingering problems.
>>
>> Relocation::pd_set_data_value in relocInfo_ppc.cpp is treating a narrowKlass
>> as a narrowOop. I adjusted the code to accommodate the narrowOop change, but
>> this probably ought to be done differently.
>>
>> There are a couple of `(narrowOop)` casts remaining in s390.ad. I'm not sure
>> whether these can be safely converted to CompressedOops::narrow_oop_cast.
>>
>> There might still be some casts from narrowOop to an integral type. Those are
>> hard to find in our cast-happy code base.
>>
>> Testing:
>> tier1-6 for Oracle supported platforms.
>> Build fastdebug linux-ppc64le and linux-s390x.
>
> I'm not convinced. This reduces readability of the code for little gain IMO. A typedef is the logical C++ construct for
> something like a narrowOop. An enum class is not, or at least I don't see it. How's a narrowOop an enumeration? The
> next time somebody uninitiated comes across this declaration, this is going to cause headscratching. I'd rather avoid
> that.
> _Mailing list message from [Kim Barrett](mailto:kim.barrett at oracle.com) on
> [shenandoah-dev](mailto:shenandoah-dev at openjdk.java.net):_
> > On Sep 21, 2020, at 4:34 AM, Roman Kennke <rkennke at openjdk.java.net> wrote:
> > On Mon, 21 Sep 2020 04:01:34 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
> > > [?]
> >
> >
> > I'm not convinced. This reduces readability of the code for little gain IMO. A typedef is the logical C++ construct for
> > something like a narrowOop. An enum class is not, or at least I don't see it. How's a narrowOop an enumeration? The
> > next time somebody uninitiated comes across this declaration, this is going to cause headscratching. I'd rather avoid
> > that.
>
> This is a normal and well-established use of scoped enums as type-safe
> integral-like values. Examples from the C++17 Standard include std::byte and
> std::align_val_t. I've also seen type-safe integer arithmetic done this way,
> eliminating that nasty implicit promotion and conversion stuff that can get
> one into so much trouble. I'm sure I can produce more examples if needed.
Hmm ok. Thanks for clarification.
Roman
-------------
PR: https://git.openjdk.java.net/jdk/pull/273
More information about the shenandoah-dev
mailing list