[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In https://reviews.llvm.org/D28441#646145, @EricWF wrote:

> In https://reviews.llvm.org/D28441#639023, @smeenai wrote:
>
> > I think the right thing to do would be to either always compile two versions of libc++, one linked against the debug libraries and the other against the non-debug libraries, or make the Debug configuration use the debug libraries and the Release configuration use the release libraries (and have `config.py` deal with this as well).
>
>
> Is it OK if this is implemented *after* this patch?


I guess so. It feels slightly ugly because there's a bit of fighting the build system going on in this patch (w.r.t. getting rid of flags), but this is also strictly better than what we have right now, so sure.



================
Comment at: CMakeLists.txt:488
 define_if_not(LIBCXX_ENABLE_ASSERTIONS -DNDEBUG)
-if (LIBCXX_ENABLE_ASSERTIONS)
+if (LIBCXX_ENABLE_ASSERTIONS AND NOT LIBCXX_TARGETING_MSVC)
   # MSVC doesn't like _DEBUG on release builds. See PR 4379.
----------------
Isn't this redundant, considering the `define_if_not(MSVC)` below? I suppose that can be changed to an unconditional define.


================
Comment at: lib/CMakeLists.txt:113
+  add_library_flags(msvcrt) # C runtime startup files
+  # Required for wide character formatting functions (e.g. `printfw`/`scanfw`)
+  add_library_flags(iso_stdio_wide_specifiers)
----------------
Would be slightly more specific: "required for **standards-compliant** wide-character formatting functions"


https://reviews.llvm.org/D28441