This revision was automatically updated to reflect the committed changes.
Closed by commit rL292001: [libc++] [CMake] Link with /nodefaultlibs on Windows
(authored by EricWF).
Changed prior to commit:
https://reviews.llvm.org/D28441?vs=84428&id=84430#toc
Repository:
rL LLVM
https://reviews.
EricWF updated this revision to Diff 84428.
EricWF added a comment.
- Address inline comments.
https://reviews.llvm.org/D28441
Files:
CMakeLists.txt
cmake/Modules/HandleLibcxxFlags.cmake
lib/CMakeLists.txt
test/libcxx/test/config.py
Index: test/libcxx/test/config.py
==
EricWF marked an inline comment as done.
EricWF added inline comments.
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_spe
smeenai added inline comments.
Comment at: lib/CMakeLists.txt:112
+ add_library_flags(vcruntime) # C++ runtime
+ add_library_flags(msvcrt) # C runtime startup files
+ add_library_flags(iso_stdio_wide_specifiers)
EricWF wrote:
> smeenai wrote:
> > EricWF wrote:
smeenai accepted this revision.
smeenai added a comment.
This revision is now accepted and ready to land.
There's a bunch of follow-up work here, but I'm fine with this for now.
https://reviews.llvm.org/D28441
___
cfe-commits mailing list
cfe-commit
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
EricWF updated this revision to Diff 84414.
EricWF added a comment.
Attempt to address review comments
- Rename `LIBCXX_TARGETING_WINDOWS` to `LIBCXX_TARGETING_MSVC`. It is set to on
when CMake defines `MSVC` to `ON`. This macro is intented to represent cases
where we are targeting native Windo
EricWF added a comment.
In https://reviews.llvm.org/D28441#639023, @smeenai wrote:
> One potential issue with always forcing the non-debug msvcrt is that any apps
> that link against libc++ would also then need to use the non-debug CRT
> libraries, otherwise you'd get all sorts of fun runtime e
rnk added inline comments.
Comment at: CMakeLists.txt:43
+if (WIN32 AND NOT MINGW)
+ set(LIBCXX_TARGETING_WINDOWS ON)
+else()
smeenai wrote:
> EricWF wrote:
> > EricWF wrote:
> > > smeenai wrote:
> > > > Not the biggest fan of this name, since it's not obvious w
smeenai added a comment.
One potential issue with always forcing the non-debug msvcrt is that any apps
that link against libc++ would also then need to use the non-debug CRT
libraries, otherwise you'd get all sorts of fun runtime errors (e.g.
cross-domain frees). I think the default Visual Stud
halyavin added inline comments.
Comment at: lib/CMakeLists.txt:113
+ add_library_flags(msvcrt) # C runtime startup files
+ add_library_flags(iso_stdio_wide_specifiers)
+endif()
EricWF wrote:
> halyavin wrote:
> > EricWF wrote:
> > > smeenai wrote:
> > > > Maybe
EricWF updated this revision to Diff 83526.
EricWF added a comment.
- Add comment explaining need for `iso_stdio_wide_specifiers.lib`.
https://reviews.llvm.org/D28441
Files:
CMakeLists.txt
cmake/Modules/HandleLibcxxFlags.cmake
lib/CMakeLists.txt
test/libcxx/test/config.py
Index: test/
EricWF added inline comments.
Comment at: lib/CMakeLists.txt:113
+ add_library_flags(msvcrt) # C runtime startup files
+ add_library_flags(iso_stdio_wide_specifiers)
+endif()
halyavin wrote:
> EricWF wrote:
> > smeenai wrote:
> > > Maybe add a comment explainin
halyavin added inline comments.
Comment at: lib/CMakeLists.txt:113
+ add_library_flags(msvcrt) # C runtime startup files
+ add_library_flags(iso_stdio_wide_specifiers)
+endif()
EricWF wrote:
> smeenai wrote:
> > Maybe add a comment explaining the purpose of thi
EricWF added inline comments.
Comment at: CMakeLists.txt:43
+if (WIN32 AND NOT MINGW)
+ set(LIBCXX_TARGETING_WINDOWS ON)
+else()
EricWF wrote:
> smeenai wrote:
> > Not the biggest fan of this name, since it's not obvious why MinGW
> > shouldn't count as targeti
halyavin added inline comments.
Comment at: lib/CMakeLists.txt:112
+ add_library_flags(vcruntime) # C++ runtime
+ add_library_flags(msvcrt) # C runtime startup files
+ add_library_flags(iso_stdio_wide_specifiers)
halyavin wrote:
> As far as I know, application
halyavin added inline comments.
Comment at: lib/CMakeLists.txt:112
+ add_library_flags(vcruntime) # C++ runtime
+ add_library_flags(msvcrt) # C runtime startup files
+ add_library_flags(iso_stdio_wide_specifiers)
As far as I know, applications shouldn't use ms
EricWF added inline comments.
Comment at: CMakeLists.txt:43
+if (WIN32 AND NOT MINGW)
+ set(LIBCXX_TARGETING_WINDOWS ON)
+else()
smeenai wrote:
> Not the biggest fan of this name, since it's not obvious why MinGW shouldn't
> count as targeting Windows. I though
EricWF added inline comments.
Comment at: CMakeLists.txt:388
+# non-debug DLLs
+remove_flags("/D_DEBUG" "/MTd" "/MDd" "/MT" "/Md" "/RTC1")
+
smeenai wrote:
> cl (and presumably clang-cl) also accepts all these flags with a leading dash
> instead of a leading sla
smeenai added inline comments.
Comment at: lib/CMakeLists.txt:108-109
+if (LIBCXX_TARGETING_WINDOWS)
+ add_compile_flags(/Zl)
+ add_link_flags(/nodefaultlib)
+ add_library_flags(ucrt) # Universal C runtime
halyavin wrote:
> smeenai wrote:
> > These should be g
halyavin added inline comments.
Comment at: lib/CMakeLists.txt:108-109
+if (LIBCXX_TARGETING_WINDOWS)
+ add_compile_flags(/Zl)
+ add_link_flags(/nodefaultlib)
+ add_library_flags(ucrt) # Universal C runtime
smeenai wrote:
> These should be guarded under a chec
smeenai added inline comments.
Comment at: CMakeLists.txt:43
+if (WIN32 AND NOT MINGW)
+ set(LIBCXX_TARGETING_WINDOWS ON)
+else()
Not the biggest fan of this name, since it's not obvious why MinGW shouldn't
count as targeting Windows. I thought of `LIBCXX_TARGE
EricWF added a comment.
Adding cfe-commits
https://reviews.llvm.org/D28441
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
23 matches
Mail list logo
