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