RFR: 8365829: Multiple definitions of static 'phase_names' [v2]

[Kim Barrett]

On Wed, 20 Aug 2025 08:21:57 GMT, Francesco Andreuzzi <duke at openjdk.org> wrote:

>> - `opto/phasetype.hpp` defines `static const char* phase_names[]`
>> - `compiler/compilerEvent.cpp` defines `static GrowableArray<const char*>* phase_names`
>> 
>> This is not a problem when the two files are compiled as different translation units, but it causes a build failure if any of them is pulled in by a precompiled header:
>> 
>> 
>> /jdk/src/hotspot/share/compiler/compilerEvent.cpp:59:36: error: redefinition of 'phase_names' with a different type: 'GrowableArray<const char *> *' vs 'const char *[100]'
>>    59 | static GrowableArray<const char*>* phase_names = nullptr;
>>       | ^
>> /jdk/src/hotspot/share/opto/phasetype.hpp:147:20: note: previous definition is here
>>   147 | static const char* phase_names[] = {
>>       | ^
>> /jdk/src/hotspot/share/compiler/compilerEvent.cpp:67:39: error: member reference base type 'const char *' is not a structure or union
>>    67 | const u4 nof_entries = phase_names->length();
>>       | ~~~~~~~~~~~^ ~~~~~~
>> /jdk/src/hotspot/share/compiler/compilerEvent.cpp:71:31: error: member reference base type 'const char *' is not a structure or union
>>    71 | writer.write(phase_names->at(i));
>>       | ~~~~~~~~~~~^ ~~
>> /jdk/src/hotspot/share/compiler/compilerEvent.cpp:77:34: error: member reference base type 'const char *' is not a structure or union
>>    77 | for (int i = 0; i < phase_names->length(); i++) {
>>       | ~~~~~~~~~~~^ ~~~~~~
>> /jdk/src/hotspot/share/compiler/compilerEvent.cpp:78:35: error: member reference base type 'const char *' is not a structure or union
>>    78 | const char* name = phase_names->at(i);
>>       | ~~~~~~~~~~~^ ~~
>> /jdk/src/hotspot/share/compiler/compilerEvent.cpp:91:9: error: comparison of array 'phase_names' equal to a null pointer is always false [-Werror,-Wtautological-pointer-compare]
>>    91 | if (phase_names == nullptr) {
>>       | ^~~~~~~~~~~ ~~~~~~~
>> /jdk/src/hotspot/share/compiler/compilerEvent.cpp:92:19: error: array type 'const char *[100]' is not assignable
>>    92 | phase_names = new (mtInternal) GrowableArray<const char*>(100, mtCompiler);
>>       | ~~~~~~~~~~~ ^
>> /jdk/src/hotspot/share/compiler/compilerEvent.cpp:103:24: error: member reference base type 'const char *' is not a structure or union
>>   103 | index = phase_names->length();
>>       | ~~~~~~~~~~~^ ~~~~~~
>> /jdk/src/hotspot/share/compiler/compilerEvent.cpp:104:16: error: member reference base type 'const char *' is not a structure or union
>>   104 | phase_names->append(use_strdup ? os::strdup(phase_name) : phase_name...
>
> Francesco Andreuzzi has updated the pull request incrementally with seven additional commits since the last revision:
> 
>  - static
>  - nn
>  - indent
>  - review sugg
>  - revert
>  - Merge branch 'resolved-default-cctor' into JDK-8365829
>  - use copy ctor

Changes requested by kbarrett (Reviewer).

src/hotspot/share/opto/phasetype.cpp line 2:

> 1: /*
> 2:  * Copyright (c) 2017, 2025, Oracle and/or its affiliates. All rights reserved.

This is a new file, so I think "2017," shouldn't be here.

src/hotspot/share/opto/phasetype.hpp line 143:

> 141: class CompilerPhaseTypeHelper {
> 142:   public:
> 143:   static const char* phase_descriptions[];

Make these `static const char* const` ? Both here and for the definitions of course. I don't
think there's any reason for the arrays to be mutable.

src/hotspot/share/opto/phasetype.hpp line 144:

> 142:   public:
> 143:   static const char* phase_descriptions[];
> 144:   static const char* phase_names[];

Is there a reason for these to be public rather than private?
Also, we "always" prefix data member names with a leading underscore; see Style Guide.

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

PR Review: https://git.openjdk.org/jdk/pull/26851#pullrequestreview-3136564495
PR Review Comment: https://git.openjdk.org/jdk/pull/26851#discussion_r2288143012
PR Review Comment: https://git.openjdk.org/jdk/pull/26851#discussion_r2288148994
PR Review Comment: https://git.openjdk.org/jdk/pull/26851#discussion_r2288140835