On Tue, 26 Jul 2022 16:25:04 GMT, Thomas Schatzl <[email protected]> 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
