[libcxx-commits] [PATCH] D99263: [libcxx] [test] Link against msvcprt in tests

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/utils/libcxx/test/config.py:449
 
     def configure_link_flags_abi_library(self):
         cxx_abi = self.get_lit_conf('cxx_abi', 'libcxxabi')
----------------
curdeius wrote:
> Could you add a comment (somewhere) mentioning the corresponding flags in CMakeLists.txt, please?
> They should probably be always in sync.
I could add a comment for the MSVC specific case that I'm editing. For the rest of this function, the whole concept of "add flags for linking ABI library" is handled quite differently between the main cmake build and the tests - for other ABI libraries, the necessary options are set in cmake/Modules/HandleLibCXXABI.cmake, while these MSVC specific ones are set in the toplevel CMakeLists.txt within an LIBCXX_TARGETING_MSVC case.

It's a bit iffy because part of these (ucrt, msvcrt) correspond to adding `-lc` and `-lgcc` (in toplevel CMakeLists.txt, to counter the fact that we're linking with `-nodefaultlibs` or `-nostdlib`), while others of them (msvcprt, maybe vcruntime) correspond to the C++ ABI library. Not entirely sure at what level it's "supported"/working to build for MSVC targets with a abi other than vcruntime.

There are people who build for the "windows-itanium" target, with an almost-msvc-like environment but with libcxxabi. I think some of such cases do it with some out-of-tree patches to libcxx though. Or maybe building libcxx with that configuration, with LIBCXX_CXX_ABI=libcxxabi works out of the box right now and we're just needlessly linking msvcprt (without actually ending up using anything from it?) in those cases, who knows. In any case, sorting out those bits is a rabbit hole I'd refrain from diving into at this point :-)

TL;DR, can add a comment for the MSVC specific case, but I'd refrain from making it too generic about things being more in sync than they really are.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99263/new/

https://reviews.llvm.org/D99263