This is an archive of the discontinued LLVM Phabricator instance.

Differential D155561

[libcxx] Respect CMAKE_MSVC_RUNTIME_LIBRARY wrt whether to use the debug CRT
ClosedPublic

Authored by mstorsjo on Jul 18 2023, 1:02 AM.

Details

Reviewers
jyknight
andrewng
Mordante
Group Reviewers
Restricted Project
Commits
rG9b02e8d4a0ab: [libcxx] Respect CMAKE_MSVC_RUNTIME_LIBRARY wrt whether to use the debug CRT
Summary

If CMAKE_MSVC_RUNTIME_LIBRARY isn't set, then CMake defaults to
the debug CRT, if CMAKE_BUILD_TYPE is set to Debug. If
CMAKE_MSVC_RUNTIME_LIBRARY is set though, that overrides any
implicit defaults.

Match this in libcxx's own manual linking logic. This allows
decoupling the debug CRT from the CMAKE_BUILD_TYPE and allows users
to configure their builds exactly how they want.

Diff Detail

Event Timeline

mstorsjo created this revision.Jul 18 2023, 1:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2023, 1:02 AM
mstorsjo requested review of this revision.Jul 18 2023, 1:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2023, 1:02 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
mstorsjo added a parent revision: D155560: [libcxx] Allow linking against the MSVC static CRT.Jul 18 2023, 1:03 AM
mstorsjo added a child revision: D155562: [libcxx] [test] Use -fms-runtime-lib= for picking the CRT to use.Jul 18 2023, 1:07 AM
mstorsjo mentioned this in D144734: [libcxx] Enable support for static and debug Windows runtimes.Jul 18 2023, 1:13 AM
Harbormaster completed remote builds in B246114: Diff 541377.Jul 18 2023, 10:20 AM
andrewng mentioned this in D155560: [libcxx] Allow linking against the MSVC static CRT.Jul 19 2023, 11:15 AM
andrewng added a comment.Jul 19 2023, 11:17 AM

LGTM, except for the amount of duplication in these test configs, also mentioned on D155560.

andrewng accepted this revision.Jul 24 2023, 11:20 AM

On the basis that any de-duplication will happen separately.

mstorsjo mentioned this in D156573: [libcxx] [test] Deduplicate setting parameters for clangcl test configs. NFC..Jul 28 2023, 2:17 PM
Mordante accepted this revision.Jul 29 2023, 6:13 AM

I too would like to see less duplication. LGTM!

This revision is now accepted and ready to land.Jul 29 2023, 6:13 AM
mstorsjo updated this revision to Diff 545407.Jul 29 2023, 11:55 PM

Rebased

mstorsjo edited parent revisions, added: D156573: [libcxx] [test] Deduplicate setting parameters for clangcl test configs. NFC.; removed: D155560: [libcxx] Allow linking against the MSVC static CRT.Aug 2 2023, 3:42 AM
mstorsjo updated this revision to Diff 546392.Aug 2 2023, 3:42 AM

Rebased to place this on top of the deduplication.

Harbormaster completed remote builds in B249720: Diff 546392.Aug 2 2023, 6:07 AM
mstorsjo updated this revision to Diff 546598.Aug 2 2023, 1:54 PM

Rebase

Harbormaster completed remote builds in B249877: Diff 546598.Aug 2 2023, 4:08 PM
mstorsjo updated this revision to Diff 546707.Aug 2 2023, 11:09 PM

Rebase

Harbormaster completed remote builds in B249957: Diff 546707.Aug 3 2023, 1:46 AM
This revision was landed with ongoing or failed builds.Aug 4 2023, 5:55 AM
Closed by commit rG9b02e8d4a0ab: [libcxx] Respect CMAKE_MSVC_RUNTIME_LIBRARY wrt whether to use the debug CRT (authored by mstorsjo). · Explain Why
This revision was automatically updated to reflect the committed changes.
mstorsjo added a commit: rG9b02e8d4a0ab: [libcxx] Respect CMAKE_MSVC_RUNTIME_LIBRARY wrt whether to use the debug CRT.

Revision Contents

Diff 545407

libcxx/CMakeLists.txt

Show First 20 Lines • Show All 676 Lines • ▼ Show 20 Lines if (LIBCXX_HAS_ATOMIC_LIB)
target_link_libraries(${target} PRIVATE atomic) target_link_libraries(${target} PRIVATE atomic)
endif() endif()
if (MINGW) if (MINGW)
target_link_libraries(${target} PRIVATE "${MINGW_LIBRARIES}") target_link_libraries(${target} PRIVATE "${MINGW_LIBRARIES}")
endif() endif()
if (MSVC) if (MSVC)
if (uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG") if ((NOT CMAKE_MSVC_RUNTIME_LIBRARY AND uppercase_CMAKE_BUILD_TYPE STREQUAL "DEBUG")
OR (CMAKE_MSVC_RUNTIME_LIBRARY MATCHES "Debug"))
set(LIB_SUFFIX "d") set(LIB_SUFFIX "d")
else() else()
set(LIB_SUFFIX "") set(LIB_SUFFIX "")
endif() endif()
if (NOT CMAKE_MSVC_RUNTIME_LIBRARY OR CMAKE_MSVC_RUNTIME_LIBRARY MATCHES "DLL$") if (NOT CMAKE_MSVC_RUNTIME_LIBRARY OR CMAKE_MSVC_RUNTIME_LIBRARY MATCHES "DLL$")
set(CRT_LIB "msvcrt") set(CRT_LIB "msvcrt")
set(CXX_LIB "msvcprt") set(CXX_LIB "msvcprt")
▲ Show 20 Lines • Show All 171 Lines • Show Last 20 Lines

libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in

# This testing configuration handles running the test suite against LLVM's libc++ # This testing configuration handles running the test suite against LLVM's libc++
# using a DLL, with Clang-cl on Windows. # using a DLL, with Clang-cl on Windows.
lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg') lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
dbg_include = '' dbg_include = ''
runtime_library = '@CMAKE_MSVC_RUNTIME_LIBRARY@' runtime_library = '@CMAKE_MSVC_RUNTIME_LIBRARY@'
if runtime_library == '' or runtime_library.endswith('DLL'): if runtime_library == '' or runtime_library.endswith('DLL'):
crt_lib = 'msvcrt' crt_lib = 'msvcrt'
cxx_lib = 'msvcprt' cxx_lib = 'msvcprt'
else: else:
crt_lib = 'libcmt' crt_lib = 'libcmt'
cxx_lib = 'libcpmt' cxx_lib = 'libcpmt'
if '@uppercase_CMAKE_BUILD_TYPE@' == 'DEBUG': if (runtime_library == '' and '@uppercase_CMAKE_BUILD_TYPE@' == 'DEBUG') or 'Debug' in runtime_library:
dbg_include = ' -D_DEBUG -include set_windows_crt_report_mode.h' dbg_include = ' -D_DEBUG -include set_windows_crt_report_mode.h'
crt_lib += 'd' crt_lib += 'd'
cxx_lib += 'd' cxx_lib += 'd'
config.substitutions.append(('%{flags}', '--driver-mode=g++')) config.substitutions.append(('%{flags}', '--driver-mode=g++'))
config.substitutions.append(('%{compile_flags}', config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX' + dbg_include '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX' + dbg_include
)) ))
Show All 16 Lines

libcxx/test/configs/llvm-libc++-shared-no-vcruntime-clangcl.cfg.in

# This testing configuration handles running the test suite against LLVM's libc++ # This testing configuration handles running the test suite against LLVM's libc++
# using a DLL, with Clang-cl on Windows. This variant sets _HAS_EXCEPTIONS = 0 # using a DLL, with Clang-cl on Windows. This variant sets _HAS_EXCEPTIONS = 0
# which removes exception class definitions from the vcruntime. # which removes exception class definitions from the vcruntime.
lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg') lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
dbg_include = '' dbg_include = ''
runtime_library = '@CMAKE_MSVC_RUNTIME_LIBRARY@' runtime_library = '@CMAKE_MSVC_RUNTIME_LIBRARY@'
if runtime_library == '' or runtime_library.endswith('DLL'): if runtime_library == '' or runtime_library.endswith('DLL'):
crt_lib = 'msvcrt' crt_lib = 'msvcrt'
cxx_lib = 'msvcprt' cxx_lib = 'msvcprt'
else: else:
crt_lib = 'libcmt' crt_lib = 'libcmt'
cxx_lib = 'libcpmt' cxx_lib = 'libcpmt'
if '@uppercase_CMAKE_BUILD_TYPE@' == 'DEBUG': if (runtime_library == '' and '@uppercase_CMAKE_BUILD_TYPE@' == 'DEBUG') or 'Debug' in runtime_library:
dbg_include = ' -D_DEBUG -include set_windows_crt_report_mode.h' dbg_include = ' -D_DEBUG -include set_windows_crt_report_mode.h'
crt_lib += 'd' crt_lib += 'd'
cxx_lib += 'd' cxx_lib += 'd'
config.substitutions.append(('%{flags}', '--driver-mode=g++')) config.substitutions.append(('%{flags}', '--driver-mode=g++'))
config.substitutions.append(('%{compile_flags}', config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX -D_HAS_EXCEPTIONS=0' + dbg_include '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX -D_HAS_EXCEPTIONS=0' + dbg_include
)) ))
Show All 16 Lines

libcxx/test/configs/llvm-libc++-static-clangcl.cfg.in

# This testing configuration handles running the test suite against LLVM's libc++ # This testing configuration handles running the test suite against LLVM's libc++
# using a static library, with Clang-cl on Windows. # using a static library, with Clang-cl on Windows.
lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg') lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
dbg_include = '' dbg_include = ''
runtime_library = '@CMAKE_MSVC_RUNTIME_LIBRARY@' runtime_library = '@CMAKE_MSVC_RUNTIME_LIBRARY@'
if runtime_library == '' or runtime_library.endswith('DLL'): if runtime_library == '' or runtime_library.endswith('DLL'):
crt_lib = 'msvcrt' crt_lib = 'msvcrt'
cxx_lib = 'msvcprt' cxx_lib = 'msvcprt'
else: else:
crt_lib = 'libcmt' crt_lib = 'libcmt'
cxx_lib = 'libcpmt' cxx_lib = 'libcpmt'
if '@uppercase_CMAKE_BUILD_TYPE@' == 'DEBUG': if (runtime_library == '' and '@uppercase_CMAKE_BUILD_TYPE@' == 'DEBUG') or 'Debug' in runtime_library:
dbg_include = ' -D_DEBUG -include set_windows_crt_report_mode.h' dbg_include = ' -D_DEBUG -include set_windows_crt_report_mode.h'
crt_lib += 'd' crt_lib += 'd'
cxx_lib += 'd' cxx_lib += 'd'
config.substitutions.append(('%{flags}', '--driver-mode=g++')) config.substitutions.append(('%{flags}', '--driver-mode=g++'))
config.substitutions.append(('%{compile_flags}', config.substitutions.append(('%{compile_flags}',
'-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX' + dbg_include '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS -DNOMINMAX' + dbg_include
)) ))
Show All 16 Lines