This revision was automatically updated to reflect the committed changes.
Closed by commit rL304360: Fix Libc++ build with MinGW64 (authored by EricWF).
Changed prior to commit:
https://reviews.llvm.org/D33082?vs=100358&id=100926#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33082
Files
smeenai accepted this revision.
smeenai added a comment.
LGTM. Thanks for all the revisions!
https://reviews.llvm.org/D33082
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
martell added a comment.
> ! In https://reviews.llvm.org/D33082#765898, @EricWF wrote:
> I removed the section you're referring to because it's unneeded.
> `<__locale>` does the exact same `#include` dance. and `` includes
> `<__locale>`.
> It should not have an affect on functionality.
It th
EricWF added a comment.
In https://reviews.llvm.org/D33082#765644, @martell wrote:
> LGTM but I can't speak for the area where you added `#include ` and
> killed off the `_NEWLIB_VERSION` check
> Seems in order based on
> https://sourceware.org/ml/newlib-cvs/2014-q3/msg00038.html
> Maybe make
martell added a comment.
LGTM but I can't speak for the area where you added `#include ` and
killed off the `_NEWLIB_VERSION` check
Seems in order based on
https://sourceware.org/ml/newlib-cvs/2014-q3/msg00038.html
Maybe make a note of the minimum newlib version supported somewhere?
Others may
EricWF updated this revision to Diff 100358.
EricWF added a comment.
- remove `asprintf` declaration and definition entirely. It's not used anywhere
within libc++.
https://reviews.llvm.org/D33082
Files:
include/__config
include/__locale
include/locale
include/stdio.h
include/support/
EricWF added a comment.
In https://reviews.llvm.org/D33082#765282, @bcraig wrote:
> I think that we generally shouldn't be giving functions names that are
> already claimed elsewhere (like mbsnrtowcs and wcsnrtombs). It is my opinion
> that these should always be qualified as __libcpp_* symbol
bcraig added a comment.
> Are you suggesting that libc++ should stop declaring/defining these
> functions, at least publically?
I think that we generally shouldn't be giving functions names that are already
claimed elsewhere (like mbsnrtowcs and wcsnrtombs). It is my opinion that
these should
martell added a comment.
> ! In https://reviews.llvm.org/D33082#764617, @EricWF wrote:
> Defining `_GNU_SOURCE` during the library build isn't enough, it's also has
> to be defined when people are using the headers,
> and in that case we must depend on the compiler to pre-define it. Is it
> im
EricWF updated this revision to Diff 100263.
EricWF added a comment.
Add `_LIBCPP_MSVCRT_LIKE` and use it to replace `_LIBCPP_WIN32API` where
appropriate.
https://reviews.llvm.org/D33082
Files:
include/__config
include/__locale
include/locale
include/stdio.h
include/support/win32/loc
EricWF marked 3 inline comments as done.
EricWF added a comment.
In https://reviews.llvm.org/D33082#760516, @martell wrote:
> I want to give some context here to dispel the confusion of what is and isn't
> win32 api specific.
>
> First lets take `vasprintf` and `asprintf ` which are not implemen
EricWF marked 6 inline comments as done.
EricWF added inline comments.
Comment at: include/__locale:370
static const mask blank = _ISblank;
-#elif defined(_LIBCPP_MSVCRT)
+#elif defined(_LIBCPP_WIN32API)
typedef unsigned short mask;
compnerd wrote:
> A
martell added a comment.
I want to give some context here to dispel the confusion of what is and isn't
win32 api specific.
First lets take `vasprintf` and `asprintf ` which are not implemented in msvcrt.
In mingw-w64 we would just have posix implementations of both.
They are hidden behind the gu
smeenai added a comment.
In https://reviews.llvm.org/D33082#754451, @compnerd wrote:
> Sure, a `_LIBCPP_MSVCRT_LIKE` WFM. I just want to make sure that we don''t
> conflate the underlying libc implementation with the Win32 API set.
Yup, that was my concern as well.
In https://reviews.llvm.or
compnerd added a comment.
Sure, a `_LIBCPP_MSVCRT_LIKE` WFM. I just want to make sure that we don''t
conflate the underlying libc implementation with the Win32 API set.
https://reviews.llvm.org/D33082
___
cfe-commits mailing list
cfe-commits@lists
smeenai added a comment.
@compnerd see my previous comment:
> @EricWF and I discussed this on IRC a bit. I'm not a fan of overloading
> _LIBCPP_WIN32API for this purpose, since to me that macro is meant for
> guarding Windows API functions, not for CRT functions. Eric suggested adding
> a new
compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.
Comment at: include/__locale:370
static const mask blank = _ISblank;
-#elif defined(_LIBCPP_MSVCRT)
+#elif defined(_LIBCPP_WIN32API)
typedef unsi
mati865 accepted this revision.
mati865 added a comment.
This revision is now accepted and ready to land.
I don't know if it is MSYS2 specific or general MinGW issue but liblibc++.a is
created. Could you check your build?
Here is line to blame:
https://github.com/llvm-mirror/libcxx/blob/master/l
bcraig added a comment.
I agree that we need to be precise in our platform detection macros. On
Windows, we currently need to worry about which compiler is being used, whether
we are targeting the mingw environment or the native (?) environment, and which
c-runtime is being used.
For my msvc-
smeenai added a comment.
@EricWF and I discussed this on IRC a bit. I'm not a fan of overloading
`_LIBCPP_WIN32API` for this purpose, since to me that macro is meant for
guarding Windows API functions, not for CRT functions. Eric suggested adding a
new macro `_LIBCPP_MSVCRT_LIKE`, which I'd be
EricWF created this revision.
This patch corrects the build errors I encountered when building on MinGW64.
https://reviews.llvm.org/D33082
Files:
include/__locale
include/locale
include/stdio.h
include/support/win32/locale_win32.h
include/wchar.h
src/new.cpp
src/support/win32/loca
21 matches
Mail list logo
