RFR: 8319813: Remove upper limit on number of compiler phases in phasetype.hpp [v3]

Daniel Lundén duke at openjdk.org
Tue Nov 21 09:48:06 UTC 2023


On Tue, 21 Nov 2023 08:46:34 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> 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.

I went with Emanuels suggestion (now changed). A topic that may be worth discussing separately: have you considered using an autoformatter for HotSpot? I assume this has been discussed before, and there are probably (too) many pitfalls given the size and age of the project.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16729#discussion_r1400293340


More information about the hotspot-compiler-dev mailing list