Replace the use of _WIN32 in libc++. Replace most use with a C runtime check _LIBCPP_MSVCRT or the new _LIBCPP_WIN32 to indicate that we are using the Win32 API. Use a new _LIBCPP_WCHAR_IS_UCS2 to indicate that we are on an environment that has a short wchar_t.
Perhaps _LIBCPP_WIN32API instead, to be clear that this is specific to the usage of Win32 APIs, rather than just a general catch-all libc++ on Windows macro?
Windows has catopen?
Might be cleaner to have a _LIBCPP_COFF macro (both here and for the similar __config change), to make the intent clearer?
I don't think wchar_t is UCS-2 anymore, I read somewhere that they switched to UTF-16 as of Windows 2000.
Is it the actual encoding you're interested in, or the element size? Judging from the diff, it looks like both, but the code appears to assume UCS-2, so it could already be broken.
Maybe you could phrase it as _LIBCPP_16BIT_WCHAR or _LIBCPP_SHORT_WCHAR instead?
+1 for that name, since it is self documenting.
This is nested in a block that already excludes windows.
+1 for _LIBCPP_COFF or similar. I would rather explicitly exclude coff than include a list of supported formats.
I wonder if the drivers on solaris define __unix__.
@smeenai I think I tried to make a jumbled mess of points :-)
- Associating an encoding with a char/element type seems wrong; any number of encodings can use wchar_t.
- Windows allegedly uses UTF-16 now, not UCS-2
- There might be a bug in locale.cpp if it actually does UCS-2 -> UTF-8 conversion and Windows no longer uses UCS-2 (though I think they overlap for a lot of code-points)
@compnerd _LIBCPP_SHORT_WCHAR sounds good to me, especially in light of the front-end flag (didn't know that existed.)
Another belated thought: it all depends on the intended meaning of the symbol:
- Does it say anything about the element size (i.e. sizeof(wchar_t)?
- Does it specify the native encoding of wide-char strings for a platform (i.e. UTF-16 on Windows)?
It looks like the same symbol is now used for both things, which may not be a good idea.
I think that it will suffice for now. We can change the flag to be more granular in the future. I think that this is going well beyond the original intent of cleaning up the usage of the _WIN32 macros at this point.
Windows wouldn't have __LP64__ defined (since it's LLP64 anyway). According to MSDN _BitScanForward64 is available on x64 and ARM, so the conditional should reflect that (i.e. the existing conditional was incorrect, but since you're changing it you may as well fix it).
Same __LP64__ comment here.
I like this. A lot.
This patch accomplishes (or maybe just moves closer, I need to check) to a goal of mine, which is to have no references to _WIN32 in any header files other that <__config> (and maybe in support/)
LGTM. My comment is a suggestion, not a requirement.
I would be tempted to add another macro here. Something like:
#if (defined(_M_ARM) || defined(__arm__)) || \ (defined(_M_AMD64) || defined(__x86_64__)) #define _LIBCPP_WIN_HAS_BITSCAN #endif
to avoid repeating the "four-armed combo"