RFR: 8290074: Remove implicit arguments for RegisterMap constructor

Axel Boldt-Christmas duke at openjdk.org
Wed Jul 27 07:53:10 UTC 2022


On Tue, 26 Jul 2022 16:25:04 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Currently the `RegisterMap` constructor uses implicit boolean arguments to configure its function. Implicit boolean arguments makes code harder to understand and reason about at the call site. Using explicit scoped enums instead makes it both clear what is being configured and the type safety makes mistakes less likely. 
>> 
>> Update `RegisterMap` constructors to use these scoped enum types instead of booleans.
>> ```C++
>> enum class UpdateMap { skip, yes };
>> enum class ProcessFrames { skip, yes };
>> enum class WalkContinuation { skip, yes };
>> 
>> 
>> Testing: tier1-3
>
> src/hotspot/share/runtime/registerMap.hpp line 75:
> 
>> 73:   enum class UpdateMap { skip, yes };
>> 74:   enum class ProcessFrames { skip, yes };
>> 75:   enum class WalkContinuation   { skip, yes };
> 
> Instead of `yes` I would recommend using like `include` (or `add') as `yes` seems relatively unspecific compared to `skip`.

`include` is probably better. I was trying to think of a good word to use, first I thought of `do` and `skip` because the enums are actions/verbs. But the reverse order (as in `UpdateMap :: do`) sounds a bit weird. Thought of it more as a question instead, `Action? :: yes/skip`. But `include` fits better with respects to `skip`, `yes`/`no` is an alternative. But `skip`/`include` is nicer. Also an excuse to fix that double space on line 75.

-------------

PR: https://git.openjdk.org/jdk/pull/9455


More information about the serviceability-dev mailing list