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

[Francesco Andreuzzi]

On Wed, 20 Aug 2025 02:52:50 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> A simpler and better solution would be to make phase_descriptions and phase_names static
data members of CompilerPhaseTypeHelper, with just a declaration in the header, and the
definition in a new .cpp file. (Note that with C++17 they could be declared inline and the .cpp
file isn't needed.)

Got it, applied this suggestion in 44aabc0a8c3115cf5f0559ee2ecc6ca1d42b2464

> I'm not sure why the change from const to constexpr is being made here.

I saw both arrays can be constructed at compile time and figured `phase_names` and `phase_descriptions` and thought it may be desirable to do so, since `constexpr` is accepted in the style guide. It's not relevant for this fix though, I reverted the change.

> Doesn't this have a problem that each translation unit including this header gets it's own
private copy of these arrays? And doesn't that introduce an ODR violation for the referring
code in CompilerPhaseTypeHelper? (Maybe the constexpr change has something to
do with that? But I'm not sure how.)

Yes, it seems there's an ODR violation here: `const` (which is implied by `constexpr`, so no difference there) gives the symbol [internal linkage](https://en.cppreference.com/w/cpp/language/storage_duration.html). It looks to me that `static` qualifier was redundant in the first place.

The ODR violation should be fixed by your solution.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26851#discussion_r2287454983