[compiler-rt] Move to cxx17.

[hctim]

Summary

This patch prefers cxx17 to cxx14, if it's available.

Please let me know if there's a good reason to stay at cxx14-only. The
rest of llvm seems to be using cxx17 without problems, and I'd like to
use the 'static inline' members feature of cxx17 to solve a GWP-ASan
dynamic initialization problem in Scudo standalone.

[hctim]

@srhines, @mcgrathr, @phosek, @echristo - please raise your hand if you foresee any problems with bumping compiler-rt to c++17. The rest of LLVM seems to use it just fine.

[phosek]

I'd slightly prefer setting CXX_STANDARD 17 on individual targets (you might also need CXX_STANDARD_REQUIRED YES if you want to rely on C++17 features) which is a more idiomatic CMake, but either way it's fine with us.

[hctim]

In D101473#2723490, @phosek wrote:

I'd slightly prefer setting CXX_STANDARD 17 on individual targets (you might also need CXX_STANDARD_REQUIRED YES if you want to rely on C++17 features) which is a more idiomatic CMake, but either way it's fine with us.

You're right, the patch should be:

-append_string_if(COMPILER_RT_HAS_STD_CXX14_FLAG -std=c++14 CMAKE_CXX_FLAGS)
+set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD_REQUIRED TRUE)

I'd still like to hear the reception for upping the build requirements to cxx17. We need to at least do this for scudo/standalone.

[phosek]

In D101473#2724259, @hctim wrote:

In D101473#2723490, @phosek wrote:

I'd slightly prefer setting CXX_STANDARD 17 on individual targets (you might also need CXX_STANDARD_REQUIRED YES if you want to rely on C++17 features) which is a more idiomatic CMake, but either way it's fine with us.

You're right, the patch should be:

-append_string_if(COMPILER_RT_HAS_STD_CXX14_FLAG -std=c++14 CMAKE_CXX_FLAGS)
+set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD_REQUIRED TRUE)

I'd still like to hear the reception for upping the build requirements to cxx17. We need to at least do this for scudo/standalone.

Modifying global variables is highly discouraged in modern CMake. Ideally we would set that as a property on the target like we do for example in libcxx. Since all compiler-rt runtimes are created via add_compiler_rt_runtime function, it should be possible to just set that property there.

[srhines]

In D101473#2723435, @hctim wrote:

@srhines, @mcgrathr, @phosek, @echristo - please raise your hand if you foresee any problems with bumping compiler-rt to c++17. The rest of LLVM seems to use it just fine.

+1 ok from me

@llozano might also care about this, but I suspect it's ok.

[mcgrathr]

In D101473#2723435, @hctim wrote:

@srhines, @mcgrathr, @phosek, @echristo - please raise your hand if you foresee any problems with bumping compiler-rt to c++17. The rest of LLVM seems to use it just fine.

+1 Fuchsia has been using 17 for a long time now. The only concern we have is that public headers (e.g. <sanitizer/asan_interface.h> et al) remain compatible with C++14 (since our public-facing headers must too, though our internal code uses 17 freely). Since most of them are compatible with C and very minimal C++ already, that doesn't seem like a problem in practice. It would be ideal for the public facing APIs to be tested against users in multiple -std modes by their own regression tests.

(I'll defer to @phosek on the cmake issues.)

[vitalybuka]

I see a lot of inline variables are a C++17 extension [-Wc++17-extensions]
we probably need to add -Wno-c++17-extensions

[vitalybuka]

compiler-rt/CMakeLists.txt

273

Actually I see c++14 still there

Why we don't use?
set (CMAKE_CXX_STANDARD 17)

compiler-rt/CMakeLists.txt

 else()
   append_string_if(COMPILER_RT_HAS_WALL_FLAG -Wall CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 endif()
 if(COMPILER_RT_ENABLE_WERROR)
   append_string_if(COMPILER_RT_HAS_WERROR_FLAG -Werror CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
   append_string_if(COMPILER_RT_HAS_WX_FLAG /WX CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 endif()
 
-append_string_if(COMPILER_RT_HAS_STD_CXX14_FLAG -std=c++14 CMAKE_CXX_FLAGS)
+if (COMPILER_RT_HAS_STD_CXX17_FLAG)
+  string(APPEND CMAKE_CXX_FLAGS " -std=c++17")
+elseif(COMPILER_RT_HAS_STD_CXX14_FLAG)
+  string(APPEND CMAKE_CXX_FLAGS " -std=c++14")
+endif()
 
 # Emulate C99 and C++11's __func__ for MSVC prior to 2013 CTP.
 if(NOT COMPILER_RT_HAS_FUNC_SYMBOL)
   add_definitions(-D__func__=__FUNCTION__)
 endif()
 
 # Provide some common commmandline flags for Sanitizer runtimes.
 if("${ANDROID_API_LEVEL}" GREATER_EQUAL 28)

compiler-rt/cmake/config-ix.cmake

 check_cxx_compiler_flag(-funwind-tables      COMPILER_RT_HAS_FUNWIND_TABLES_FLAG)
 check_cxx_compiler_flag(-fno-stack-protector COMPILER_RT_HAS_FNO_STACK_PROTECTOR_FLAG)
 check_cxx_compiler_flag(-fno-sanitize=safe-stack COMPILER_RT_HAS_FNO_SANITIZE_SAFE_STACK_FLAG)
 check_cxx_compiler_flag(-fvisibility=hidden  COMPILER_RT_HAS_FVISIBILITY_HIDDEN_FLAG)
 check_cxx_compiler_flag(-frtti               COMPILER_RT_HAS_FRTTI_FLAG)
 check_cxx_compiler_flag(-fno-rtti            COMPILER_RT_HAS_FNO_RTTI_FLAG)
 check_cxx_compiler_flag("-Werror -fno-function-sections" COMPILER_RT_HAS_FNO_FUNCTION_SECTIONS_FLAG)
 check_cxx_compiler_flag(-std=c++14           COMPILER_RT_HAS_STD_CXX14_FLAG)
+check_cxx_compiler_flag(-std=c++17           COMPILER_RT_HAS_STD_CXX17_FLAG) # C++17 takes precedence.
 check_cxx_compiler_flag(-ftls-model=initial-exec COMPILER_RT_HAS_FTLS_MODEL_INITIAL_EXEC)
 check_cxx_compiler_flag(-fno-lto             COMPILER_RT_HAS_FNO_LTO_FLAG)
 check_cxx_compiler_flag(-fno-profile-generate COMPILER_RT_HAS_FNO_PROFILE_GENERATE_FLAG)
 check_cxx_compiler_flag(-fno-profile-instr-generate COMPILER_RT_HAS_FNO_PROFILE_INSTR_GENERATE_FLAG)
 check_cxx_compiler_flag(-fno-profile-instr-use COMPILER_RT_HAS_FNO_PROFILE_INSTR_USE_FLAG)
 check_cxx_compiler_flag("-Werror -msse3" COMPILER_RT_HAS_MSSE3_FLAG)
 check_cxx_compiler_flag("-Werror -msse4.2"   COMPILER_RT_HAS_MSSE4_2_FLAG)
 check_cxx_compiler_flag(--sysroot=.          COMPILER_RT_HAS_SYSROOT_FLAG)

 set(VERS_COMPAT_OPTION "-Wl,-z,gnu-version-script-compat")
 check_linker_flag("${VERS_COMPAT_OPTION}" COMPILER_RT_HAS_GNU_VERSION_SCRIPT_COMPAT)
 
 set(DUMMY_VERS ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/dummy.vers)
 file(WRITE ${DUMMY_VERS} "{};")
 set(VERS_OPTION "-Wl,--version-script,${DUMMY_VERS}")
 if(COMPILER_RT_HAS_GNU_VERSION_SCRIPT_COMPAT)
   # Solaris 11.4 ld only supports --version-script with
-  # -z gnu-version-script-compat. 
+  # -z gnu-version-script-compat.
   string(APPEND VERS_OPTION " ${VERS_COMPAT_OPTION}")
 endif()
 check_linker_flag("${VERS_OPTION}" COMPILER_RT_HAS_VERSION_SCRIPT)
 
 if(ANDROID)
   check_linker_flag("-Wl,-z,global" COMPILER_RT_HAS_Z_GLOBAL)
   check_library_exists(log __android_log_write "" COMPILER_RT_HAS_LIBLOG)
 endif()