[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-07-17 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment. > Could something like _WIN32 && _MSC_VER make this better? That would allow > us to differentiate between the various environments. It's kinda icky, but, > does make sense in a round about way. The check in the patch was already under a `#if defined(_WIN32)`, having

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-07-17 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL308225: Check for _MSC_VER before defining _LIBCPP_MSVCRT (authored by bruno). Changed prior to commit: https://reviews.llvm.org/D34588?vs=103975&id=106960#toc Repository: rL LLVM https://reviews.ll

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-07-13 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. > Some non-windows targets using MS extensions define _WIN32 for compatibility > with Windows but do not have MSVC compatibility. So, these hypothetical targets have the Win32 API available, but the

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-07-13 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment. In https://reviews.llvm.org/D34588#802533, @bruno wrote: > > Thinking more about this, on Windows, is there a strong reason to default > > to a different libc by default on Windows? > > I'm mostly concerned with `-fms-extensions` + darwin, which doesn't apply to > this

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-07-09 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment. > Thinking more about this, on Windows, is there a strong reason to default to > a different libc by default on Windows? I'm mostly concerned with `-fms-extensions` + darwin, which doesn't apply to this scenario, maybe someone else knows a better answer here. > @bruno

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-27 Thread Ben Craig via Phabricator via cfe-commits
bcraig added a comment. _LIBCPP_MS_CRT seems fine too. https://reviews.llvm.org/D34588 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-26 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment. Thinking more about this, on Windows, is there a strong reason to default to a different libc by default on Windows? @bruno would reusing `-ffreestanding` work for you here? Or is there something else that we can identify about the target environment that can indicat

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-26 Thread Ben Craig via Phabricator via cfe-commits
bcraig added a comment. LGTM, but you should probably get approval from somebody a bit more senior with the project. Comment at: include/__config:234-235 +// a MS compatibility version is specified. # ifndef __MINGW32__ -#define _LIBCPP_MSVCRT // Using Microsoft's C Runt

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-26 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno updated this revision to Diff 103975. bruno added a comment. Update patch after reviewer suggestions! https://reviews.llvm.org/D34588 Files: include/__config Index: include/__config === --- include/__config +++ include/__

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-26 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno marked an inline comment as done. bruno added inline comments. Comment at: include/__config:234-235 +// a MS compatibility version is specified. # ifndef __MINGW32__ -#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#ifdef _MSC_VER +# define _LI

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-24 Thread Ben Craig via Phabricator via cfe-commits
bcraig added inline comments. Comment at: include/__config:234-235 +// a MS compatibility version is specified. # ifndef __MINGW32__ -#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#ifdef _MSC_VER +# define _LIBCPP_MSVCRT // Using Microsoft's C Runt

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-24 Thread David Majnemer via Phabricator via cfe-commits
majnemer added inline comments. Comment at: include/__config:234-235 +// a MS compatibility version is specified. # ifndef __MINGW32__ -#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#ifdef _MSC_VER +# define _LIBCPP_MSVCRT // Using Microsoft's C Ru

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-24 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added inline comments. Comment at: include/__config:234-235 +// a MS compatibility version is specified. # ifndef __MINGW32__ -#define _LIBCPP_MSVCRT // Using Microsoft's C Runtime library +#ifdef _MSC_VER +# define _LIBCPP_MSVCRT // Using Microsoft's C Ru

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-23 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added reviewers: compnerd, majnemer, rnk. smeenai added subscribers: majnemer, compnerd. smeenai added a comment. This looks sensible to me. I don't know if there are any scenarios in which you'd be using the Microsoft CRT without having `_MSC_VER` defined, however. Added some people who

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

2017-06-23 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno created this revision. _LIBCPP_MSVCRT is defined because _WIN32 is defined and __MINGW32__ is not. Some non-windows targets using MS extensions define _WIN32 for compatibility with Windows but do not have MSVC compatibility. This patch is an attempt to do not have _LIBCPP_MSVCRT defined f