RFR: 8319813: Remove upper limit on number of compiler phases in phasetype.hpp [v3]
Christian Hagedorn
chagedorn at openjdk.org
Tue Nov 21 08:50:11 UTC 2023
On Tue, 21 Nov 2023 07:47:41 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
>> Daniel Lundén has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Inline single-use function for empty check
>
> src/hotspot/share/compiler/compilerDirectives.cpp line 300:
>
>> 298: }
>> 299:
>> 300: DirectiveSet::DirectiveSet(CompilerDirectives* d) :_inlinematchers(nullptr), _directive(d), _ideal_phase_name_set(PHASE_NUM_TYPES, mtCompiler) {
>
> Needs a space after the colon `:`. But even better would be to put the initializations on a new line each, with 2 spaces indentation, and the opening bracket on a new line by itself.
I agree that we should wrap this line. But unfortunately, there does not seem to be a rule to follow in our style guide about how to format a wrapped initializer list - maybe that's something to think about to add at some point. The Google C++ style guide [suggests](https://google.github.io/styleguide/cppguide.html#Constructor_Initializer_Lists) the following (which I often adhere to but I always put each member on a new line for clarity if it's too much to put on one line):
Constructor initializer lists can be all on one line or with subsequent lines indented four spaces.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16729#discussion_r1400215762
More information about the hotspot-compiler-dev
mailing list