This is an archive of the discontinued LLVM Phabricator instance.

Differential D32320

[libc++] Default to vcruntime when targeting MSVC
ClosedPublic

Authored by smeenai on Apr 20 2017, 4:02 PM.

Details

Reviewers
mclow.lists
compnerd
EricWF
Commits
rG8048b44a8cc3: [libc++] Default to vcruntime when targeting MSVC
rCXX300921: [libc++] Default to vcruntime when targeting MSVC
rL300921: [libc++] Default to vcruntime when targeting MSVC
Summary

libc++abi is never the right option for LIBCXX_TARGETING_MSVC, since it
targets the Itanium ABI, whereas MSVC uses the Microsoft ABI. Make the
default ABI be vcruntime when targeting MSVC even if libc++abi is
present in the tree.

Diff Detail

Event Timeline

smeenai created this revision.Apr 20 2017, 4:02 PM
Herald edited edge metadata. · View Herald TranscriptApr 20 2017, 4:02 PM
Herald added a subscriber: mgorny. · View Herald Transcript
EricWF accepted this revision.Apr 20 2017, 4:30 PM

LGTM.

CMakeLists.txt
134–135

Nit: I would rather see this be

elseif ((NOT LIBCXX_STANDALONE_BUILD OR HAVE_LIBCXXABI) AND
        IS_DIRECTORY "${LIBCXX_LIBCXXABI_INCLUDES_INTERNAL}")

That way we can avoid a level of nesting.

This revision is now accepted and ready to land.Apr 20 2017, 4:30 PM
smeenai added inline comments.Apr 20 2017, 4:32 PM
CMakeLists.txt
134–135

Sounds good; will change before committing.

smeenai updated this revision to Diff 96055.Apr 20 2017, 4:40 PM

Reduce nesting (address comments)

Closed by commit rL300921: [libc++] Default to vcruntime when targeting MSVC (authored by smeenai). · Explain WhyApr 20 2017, 4:46 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
20 lines

Diff 96055

CMakeLists.txt

Show First 20 Lines • Show All 114 Lines • ▼ Show 20 Lines
if (LIBCXX_CXX_ABI STREQUAL "default") if (LIBCXX_CXX_ABI STREQUAL "default")
find_path( find_path(
LIBCXX_LIBCXXABI_INCLUDES_INTERNAL LIBCXX_LIBCXXABI_INCLUDES_INTERNAL
cxxabi.h cxxabi.h
PATHS ${LLVM_MAIN_SRC_DIR}/projects/libcxxabi/include PATHS ${LLVM_MAIN_SRC_DIR}/projects/libcxxabi/include
${LLVM_MAIN_SRC_DIR}/runtimes/libcxxabi/include ${LLVM_MAIN_SRC_DIR}/runtimes/libcxxabi/include
NO_DEFAULT_PATH NO_DEFAULT_PATH
) )
if ((NOT LIBCXX_STANDALONE_BUILD OR HAVE_LIBCXXABI) AND if (LIBCXX_TARGETING_MSVC)
# FIXME: Figure out how to configure the ABI library on Windows.
set(LIBCXX_CXX_ABI_LIBNAME "vcruntime")
elseif ((NOT LIBCXX_STANDALONE_BUILD OR HAVE_LIBCXXABI) AND
IS_DIRECTORY "${LIBCXX_LIBCXXABI_INCLUDES_INTERNAL}") IS_DIRECTORY "${LIBCXX_LIBCXXABI_INCLUDES_INTERNAL}")
set(LIBCXX_CXX_ABI_LIBNAME "libcxxabi") set(LIBCXX_CXX_ABI_LIBNAME "libcxxabi")
set(LIBCXX_CXX_ABI_INCLUDE_PATHS "${LIBCXX_LIBCXXABI_INCLUDES_INTERNAL}") set(LIBCXX_CXX_ABI_INCLUDE_PATHS "${LIBCXX_LIBCXXABI_INCLUDES_INTERNAL}")
set(LIBCXX_CXX_ABI_INTREE 1) set(LIBCXX_CXX_ABI_INTREE 1)
else()
if (LIBCXX_TARGETING_MSVC)
# FIXME: Figure out how to configure the ABI library on Windows.
set(LIBCXX_CXX_ABI_LIBNAME "vcruntime")
elseif(APPLE) elseif (APPLE)
set(LIBCXX_CXX_ABI_LIBNAME "libcxxabi") set(LIBCXX_CXX_ABI_LIBNAME "libcxxabi")
set(LIBCXX_CXX_ABI_SYSTEM 1) set(LIBCXX_CXX_ABI_SYSTEM 1)
else() else()
set(LIBCXX_CXX_ABI_LIBNAME "default") set(LIBCXX_CXX_ABI_LIBNAME "default")
EricWFUnsubmitted
Not Done
ReplyInline Actions

Nit: I would rather see this be

elseif ((NOT LIBCXX_STANDALONE_BUILD OR HAVE_LIBCXXABI) AND
        IS_DIRECTORY "${LIBCXX_LIBCXXABI_INCLUDES_INTERNAL}")

That way we can avoid a level of nesting.

EricWF: Nit: I would rather see this be ``` elseif ((NOT LIBCXX_STANDALONE_BUILD OR HAVE_LIBCXXABI) AND…
smeenaiAuthorUnsubmitted
Not Done
ReplyInline Actions

Sounds good; will change before committing.

smeenai: Sounds good; will change before committing.
endif() endif()
endif()
else() else()
set(LIBCXX_CXX_ABI_LIBNAME "${LIBCXX_CXX_ABI}") set(LIBCXX_CXX_ABI_LIBNAME "${LIBCXX_CXX_ABI}")
endif() endif()
# Use a static copy of the ABI library when linking libc++. This option # Use a static copy of the ABI library when linking libc++. This option
# cannot be used with LIBCXX_ENABLE_ABI_LINKER_SCRIPT. # cannot be used with LIBCXX_ENABLE_ABI_LINKER_SCRIPT.
option(LIBCXX_ENABLE_STATIC_ABI_LIBRARY "Statically link the ABI library" OFF) option(LIBCXX_ENABLE_STATIC_ABI_LIBRARY "Statically link the ABI library" OFF)
▲ Show 20 Lines • Show All 493 Lines • Show Last 20 Lines