compnerd closed this revision.
compnerd added a comment.
SVN r290910. I like your idea for an additional BITSCAN macro. I think that
Id rather do that in a follow up.
Repository:
rL LLVM
https://reviews.llvm.org/D28223
___
cfe-commits mailing
mclow.lists accepted this revision.
mclow.lists added a comment.
This revision is now accepted and ready to land.
LGTM. My comment is a suggestion, not a requirement.
Comment at: include/support/win32/support.h:112
// Search from LSB to MSB for first set bit.
// Returns zero
compnerd updated this revision to Diff 82944.
compnerd added a comment.
fix `__LP64__` usage as pointed out by @smeenai
Repository:
rL LLVM
https://reviews.llvm.org/D28223
Files:
include/__config
include/__locale
include/support/win32/support.h
include/type_traits
src/chrono.cpp
mclow.lists added a comment.
I like this. A lot.
I'm a bit concerned about @smeenai 's comments about __LP64_, and @EricWF 's
comment about solaris.
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 heade
smeenai added inline comments.
Comment at: include/support/win32/support.h:113
// Returns zero if no set bit is found.
-#if defined(_WIN64)
+#if defined(__LP64__)
if (_BitScanForward64(&where, mask))
Windows wouldn't have `__LP64__` defined (since it's LLP64
kimgr added a comment.
I agree. Since the current set of encodings all have different element sizes
and this is not likely to change, this looks good.
Repository:
rL LLVM
https://reviews.llvm.org/D28223
___
cfe-commits mailing list
cfe-commits@l
compnerd added a comment.
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.
Repository:
rL LLVM
https://reviews.llvm.org/
kimgr added a comment.
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 s
compnerd updated this revision to Diff 82927.
compnerd added a comment.
Switch to `_LIBCPP_SHORT_WCHAR`.
Repository:
rL LLVM
https://reviews.llvm.org/D28223
Files:
include/__config
include/__locale
include/support/win32/support.h
include/type_traits
src/chrono.cpp
src/include/con
kimgr added a comment.
@smeenai I think I tried to make a jumbled mess of points :-)
1. Associating an encoding with a char/element type seems wrong; any number of
encodings can use wchar_t.
2. Windows allegedly uses UTF-16 now, not UCS-2
3. There might be a bug in locale.cpp if it actually does
smeenai added a comment.
In https://reviews.llvm.org/D28223#634199, @compnerd wrote:
> I figured that using the explicit encoding is nicer since it means that its
> more documenting.
I think @kimgr's point was that if you're using an explicit encoding, it should
be UTF-16 rather than UCS-2 :)
compnerd updated this revision to Diff 82912.
compnerd added a comment.
Update for `_LIBCPP_WIN32API` and addition of `_LIBCPP_OBJECT_FORMAT_COFF`,
`_LIBCPP_OBJECT_FORMAT_ELF`, `_LIBCPP_OBJECT_FORMAT_MACHO`
Repository:
rL LLVM
https://reviews.llvm.org/D28223
Files:
include/__config
incl
compnerd added a comment.
I figured that using the explicit encoding is nicer since it means that its
more documenting. I dont have a problem with `_LIBCPP_SHORT_WCHAR` as that
maps quite well to `-fshort-wchar`.
Repository:
rL LLVM
https://reviews.llvm.org/D28223
__
compnerd added inline comments.
Comment at: include/__config:158
+#if defined(_WIN32)
+# define _LIBCPP_WIN32 1
# define _LIBCPP_LITTLE_ENDIAN 1
EricWF wrote:
> smeenai wrote:
> > Perhaps `_LIBCPP_WIN32API` instead, to be clear that this is specific to
EricWF added inline comments.
Comment at: include/__config:158
+#if defined(_WIN32)
+# define _LIBCPP_WIN32 1
# define _LIBCPP_LITTLE_ENDIAN 1
smeenai wrote:
> Perhaps `_LIBCPP_WIN32API` instead, to be clear that this is specific to the
> usage of Win
kimgr added a comment.
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 alr
smeenai added a comment.
Sweet.
Comment at: include/__config:158
+#if defined(_WIN32)
+# define _LIBCPP_WIN32 1
# define _LIBCPP_LITTLE_ENDIAN 1
Perhaps `_LIBCPP_WIN32API` instead, to be clear that this is specific to the
usage of Win32 APIs, rather
compnerd created this revision.
compnerd added reviewers: EricWF, mclow.lists, smeenai.
compnerd added a subscriber: cfe-commits.
compnerd set the repository for this revision to rL LLVM.
Replace the use of `_WIN32` in libc++. Replace most use with a C runtime check
`_LIBCPP_MSVCRT` or the new `
18 matches
Mail list logo
