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