EricWF added inline comments.
================
Comment at: include/__config:232-235
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+
----------------
compnerd wrote:
> bcraig wrote:
> > I can see this helping when we are build libc++, but I don't think it helps
> > client apps. They could have included windows.h before including our
> > headers.
> >
> > Don't get me wrong, I dislike the min and max macros, and bear no hard
> > feelings towards people that define this project wide on the command line,
> > I'm just not sure it will get things done right here.
> >
> > In the past, I've just surrounded my min and max declarations with
> > parenthesis to suppress macro expansion.
> Yeah, I don't think that this should be defined in `__config`. Can we do
> something like `#pragma push_macro` and `#pragma pop_macro` in the necessary
> files?
Alright I'll remove the `NOMINMAX` define in `__config` and start to sprinkle
the `__undef_min_max` include where it's currently needed.
@compnerd, using `push_macro` and `pop_macro` are going to be the correct way
to handle this, but that's a change for another patch.
================
Comment at: include/support/win32/msvc_builtin_support.h:33
+
+_LIBCPP_ALWAYS_INLINE int __builtin_popcount(unsigned int x)
+{
----------------
majnemer wrote:
> compnerd wrote:
> > I think I prefer the following implementation:
> >
> > _LIBCPP_ALWAYS_INLINE int __builtin_popcount(unsigned int value) {
> > return __popcnt(value);
> > }
> I think it'd be better not to call it `__builtin_anything`. MSVC uses the
> __builtin_ namespace too, see https://godbolt.org/g/HwMskX
>
> Maybe create a wrapper called `__libcpp_popcount`?
That's a change for another patch, one that's just meant to restructure the
headers.
This patch simply moves this code to another file.
================
Comment at: include/support/win32/msvc_builtin_support.h:33-51
+_LIBCPP_ALWAYS_INLINE int __builtin_popcount(unsigned int x)
+{
+ // Binary: 0101...
+ static const unsigned int m1 = 0x55555555;
+ // Binary: 00110011..
+ static const unsigned int m2 = 0x33333333;
+ // Binary: 4 zeros, 4 ones ...
----------------
EricWF wrote:
> majnemer wrote:
> > compnerd wrote:
> > > I think I prefer the following implementation:
> > >
> > > _LIBCPP_ALWAYS_INLINE int __builtin_popcount(unsigned int value) {
> > > return __popcnt(value);
> > > }
> > I think it'd be better not to call it `__builtin_anything`. MSVC uses the
> > __builtin_ namespace too, see https://godbolt.org/g/HwMskX
> >
> > Maybe create a wrapper called `__libcpp_popcount`?
> That's a change for another patch, one that's just meant to restructure the
> headers.
>
> This patch simply moves this code to another file.
That's a change for another patch, one that's just meant to restructure the
headers.
This patch simply moves this code to another file.
================
Comment at: src/string.cpp:430
#else
- return static_cast<int (__cdecl*)(wchar_t* __restrict, size_t, const
wchar_t*__restrict, ...)>(swprintf);
+ return static_cast<int (__cdecl*)(wchar_t* __restrict, size_t, const
wchar_t*__restrict, ...)>(_snwprintf);
#endif
----------------
compnerd wrote:
> This seems scary. Why do we need to cast the function?
No idea.
https://reviews.llvm.org/D32988
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
