[compiler] fix the wrongly appended warning flags on gcc compiler.

[georgeliao]

At global scope:
cc1plus: note: unrecognized command-line option ‘-Wno-gnu’ may have been intended to silence earlier diagnostics
cc1plus: note: unrecognized command-line option ‘-Wno-nested-anon-types’ may have been intended to silence earlier diagnostics

The original implementation uses check_cxx_compiler_flag, and the cmake function does not seem to return the expected SUPPORTED value. As a result, -Wno-nested-anon-types -Wno-gnu two warnings flags that are clang-only can pass gcc check and get appended to gcc. To verify this, make clean && make cert in the linux + gcc environment will show the message above. The fix is simple as well, append the flags explicitly to respective compilers.

https://warthogs.atlassian.net/browse/MULTI-1766

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.07%. Comparing base (637e431) to head (ea9dc2d).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3888      +/-   ##
==========================================
- Coverage   89.08%   89.07%   -0.01%     
==========================================
  Files         254      254              
  Lines       14598    14598              
==========================================
- Hits        13004    13003       -1     
- Misses       1594     1595       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[xmkg]

So the main issue here is check_cxx_compiler_flag sets the given var (SUPPORTED) as a cache variable, meaning that the result will be persisted across calls for the same variable, thus SUPPORTED should not be reused. In order to be able to re-use it though, we can remove it from cache after the check by:

unset(SUPPORTED CACHE)

then everything should work as expected.

[xmkg]

... alternatively, we can use the flag itself as a variable name, which is already unique :

  string(MAKE_C_IDENTIFIER ${flag} VAR_NAME)
  check_cxx_compiler_flag(${flag} ${VAR_NAME})
  if(${VAR_NAME})
  # ...
  endif()

This produces better output when performing the checks:

-- Performing Test _Wno_nested_anon_types
-- Performing Test _Wno_nested_anon_types - Success
-- Performing Test _Wno_gnu
-- Performing Test _Wno_gnu - Success
-- Performing Test _Wno_pedantic
-- Performing Test _Wno_pedantic - Success
-- Performing Test _Wno_ignored_qualifiers
-- Performing Test _Wno_ignored_qualifiers - Success
-- Performing Test _Weverything
-- Performing Test _Weverything - Failed
-- Performing Test _Wall
-- Performing Test _Wall - Success

[georgeliao]

So the main issue here is check_cxx_compiler_flag sets the given var (SUPPORTED) as a cache variable, meaning that the result will be persisted across calls for the same variable, thus SUPPORTED should not be reused. In order to be able to re-use it though, we can remove it from cache after the check by:

It seems to make sense. However, the first flag -Wno-nested-anon-types should fail on gcc which result in the SUPPORTED value evaluated to be false first and persists to be false throughout the 4 iterations. However, the output of SUPPORTED is 4 true in a row.

About the possible fix

unset(SUPPORTED CACHE)

Do you mean the code snippet below?

foreach(flag -Wno-nested-anon-types -Wno-gnu -Wno-pedantic -Wno-ignored-qualifiers)
  check_cxx_compiler_flag(${flag} SUPPORTED)
  if(SUPPORTED)
    target_compile_options(cert PRIVATE ${flag})
  endif()
  unset(SUPPORTED CACHE)
endforeach()

That did not seem to change the outcome.

[xmkg]

I see. I did more testing and found that it's because CMake fails to recognize the diagnostic message emitted by the compiler when a warning suppression flag is not recognized (i.e., -Wno-*). In the suppression case, the compiler would emit a warning but succeed:

-Wno-nested-anon-types

cc1plus: note: unrecognized command-line option '-Wno-nested-anon-types' may have been intended to silence earlier diagnostics
Execution build compiler returned: 0

But it's a hard failure when the "no-" is omitted, and check_cxx_compiler_flag works as expected:

g++: error: unrecognized command-line option '-Wnested-anon-types'
Compiler returned: 1

AFAICT, GCC does not have an option to make this diagnostic error a hard failure. Hence, I'm in favor of the initial solution you've proposed.

[georgeliao]

But it's a hard failure when the "no-" is omitted, and check_cxx_compiler_flag works as expected:

Exactly, It is really an error or warning thing. CMake creates a temporary source file (a simple, valid C++ program) and attempts to compile it using the provided flag. check_cxx_compiler_flag returns false only if it is an error.

[xmkg]

Since the -Wno-pedantic -Wno-ignored-qualifiers part is common between two, we can simplify this as:

target_compile_options(cert PRIVATE
    -Wno-pedantic -Wno-ignored-qualifiers
    $<$<CXX_COMPILER_ID:Clang>:-Wno-nested-anon-types -Wno-gnu>
)

[xmkg]

Otherwise, LGTM!

[georgeliao]

There is a 3rd compiler in the Multipass ecosystem and that is msvc, I think the desired behavior is that gcc gets two flags, clang gets four flags and msvc gets none. I think the conditional appending flags will be more complicated in this pattern. Probably something like this.

target_compile_options(cert PRIVATE
    $<$<OR:$<CXX_COMPILER_ID:Clang>,$<CXX_COMPILER_ID:GNU>>:-Wno-pedantic -Wno-ignored-qualifiers>
    $<$<CXX_COMPILER_ID:Clang>:-Wno-nested-anon-types -Wno-gnu>
)

[xmkg]

Yeah, this makes sense. Thanks for clarifying!