[crac] RFR: Compile all the INCLUDE_LD_SO_LIST_DIAGNOSTICS code unconditionally

Radim Vansa rvansa at openjdk.org
Tue Oct 31 07:03:28 UTC 2023


On Tue, 31 Oct 2023 06:28:04 GMT, Jan Kratochvil <jkratochvil at openjdk.org> wrote:

> Make INCLUDE_LD_SO_LIST_DIAGNOSTICS an `if` conditional instead of a `#if` one.
> Fix compilation failures.

While I like the certainty that the code is always compilable, I wonder if handling optional features like this (using macros in conditions) is a pattern that is used elsewhere in the JDK?

src/hotspot/cpu/x86/vm_version_x86.cpp line 856:

> 854: }
> 855: 
> 856: //#if INCLUDE_LD_SO_LIST_DIAGNOSTICS

Please remove commented out code.

src/hotspot/cpu/x86/vm_version_x86.cpp line 944:

> 942:   const char *env = getenv(TUNABLES_NAME);
> 943:   if (env && strcmp(env, env_val) == 0) {
> 944:     if (!INCLUDE_CPU_FEATURE_ACTIVE && !INCLUDE_LD_SO_LIST_DIAGNOSTICS) {

Have you tested compilation with these disabled? Looks to me like the compiler could complain about unreachable code when the condition is always false...

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

PR Review: https://git.openjdk.org/crac/pull/136#pullrequestreview-1705554187
PR Review Comment: https://git.openjdk.org/crac/pull/136#discussion_r1377123401
PR Review Comment: https://git.openjdk.org/crac/pull/136#discussion_r1377126065


More information about the crac-dev mailing list