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 implemented in > msvcrt. > In mingw-w64 we would just have posix implementations of both. > They are hidden behind the guard `_GNU_SOURCE`, we don't need to declare > them in `include/stdio.h` in libcxx but > By default gcc doesn't pass this flag for the mingw target so we should add > it in cmake within libc++. 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 important that MinGW not define `_GNU_SOURCE` by default? Also do you know why `asprintf` is declared by mingw-w64 but `vasprintf` isn't? At minimum I think we still need to declare `vasprintf` in the headers because we can't count on `_GNU_SOURCE` being defined before `<features.h>` is first included, but we should be able to omit the definition. > Next example `mbsnrtowcs` and `wcsnrtombs` above > There is technically a win32api specific implementation in msvc. The > mingw-w64 implementation or lack there of would rely on the MSVC > implementation. > This is why a custom implementation lives in libc++ in `support.cpp` that is > posix compliant. Good to know. > Unfortunately a posix implementation might not be accepted upstream into > mingw-w64 because they need to maintain compatibility with msvc, even with > some bugs eek. I'm not personally concerned about upstreaming MinGW-w64 changes/fixes. If we can adequately solve it within libc++ without any terrible hacks I'm happy with that for now. > It generally comes down to if a bunch of projects are using it already and we > should not disrupt them relying on msvc implementations/bugs. > This is probably not the case for these two functions but there are many > others already in the library that follow this guideline. Are you suggesting that libc++ should stop declaring/defining these functions, at least publically? > A good read of this specific bug is here. > http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130325/077071.html > > @smeenai suggestion of `_LIBCPP_MSVCRT_LIKE` makes a lot of sense here > because mingw-w64 is only partially posixy, this leaves room for future > possible targets like midipix which uses musl libc to ignore this section of > code where mingw and msvc opt in. https://reviews.llvm.org/D33082 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
