[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

2021-08-12 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I suggest you take all the techniques at http://graphics.stanford.edu/~seander/bithacks.html and make sure they don't cause a warning. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108003/new/ https://reviews.llvm.org/D108003 _

[PATCH] D80743: (PR46111) Desugar Elaborated types in Deduction Guides.

2020-06-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I've got nothing to say here. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80743/new/ https://reviews.llvm.org/D80743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D69292: Proposal to add -Wtautological-compare to -Wall

2019-11-14 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Sorry I'm late to the party; I've been traveling for 3+ weeks. I would like to be reassured that the following code will not warn: ` long foo = ...; // some calculation if (foo < std::numeric_limits::min() || foo > std::numeric_limits::max()) . This is imp

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

2019-11-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: libcxx/test/std/containers/views/span.cons/span.fail.cpp:78 - -// Try to remove const and/or volatile (static -> static) -{ Ok. The comment here is wrong; this is testing dynamic -> static. However, why are you

[PATCH] D68364: Prototype implementation of P0784R7: mark all members of std::allocator and std::allocator_traits as constexpr.

2019-10-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Thanks for doing this, Richard. A few things: - Needs tests (as you said) - The config macro stuff is all wrong (we build that file from a template) - Use `__libcpp_is_constant_evaluated` instead of `__builtin_is_constant_evaluated` because it doesn't have to be `#if

[PATCH] D67052: Add reference type transformation builtins

2019-09-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. If you're going to do `__add__lvalue_reference`, `__add_rvalue_reference`, and `__remove_reference`, why not go all the way and add `__is_reference`, `__is_lvalue_reference` and `__is_rvalue_reference`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D40259: [libcxx] LWG2993: reference_wrapper

2019-07-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Did this ever get landed? If not, was there a reason? Note that the synopsis at the top of `` also needs to be updated. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40259/new/ https://reviews.llvm.org/D40259 __

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

2019-07-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. This is an old patch; is this still needed/desired? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37182/new/ https://reviews.llvm.org/D37182 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D24372: [libcxx] Sprinkle constexpr over compressed_pair

2019-07-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Is this patch relevant any more? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D24372/new/ https://reviews.llvm.org/D24372 ___

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

2019-07-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Is there a reason this hasn't been committed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61366/new/ https://reviews.llvm.org/D61366 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D43159: Modernize: Use nullptr more.

2019-07-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: include/__threading_support:323 bool __libcpp_thread_isnull(const __libcpp_thread_t *__t) { - return *__t == 0; + return *__t == nullptr; } mclow.lists wrote: > This one is wrong. `__libcpp_thread_t` is an alias

[PATCH] D43159: Modernize: Use nullptr more.

2019-07-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: include/algorithm:4431 value_type* __p = __buff; -for (_BidirectionalIterator __i = __first; __i != __middle; __d.__incr((value_type*)0), (void) ++__i, ++__p) +for (_BidirectionalIterator __i = __first; __i

[PATCH] D43159: Modernize: Use nullptr more.

2019-07-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In ``, you missed a couple of `(value_type*)0`. Line 3356, 3369, 3486 and 3499. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43159/new/ https://reviews.llvm.org/D43159 ___ cfe-commit

[PATCH] D43159: Modernize: Use nullptr more.

2019-07-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists requested changes to this revision. mclow.lists added a comment. This revision now requires changes to proceed. Did you try to build libc++ or run the tests before submitting this? Comment at: include/__threading_support:323 bool __libcpp_thread_isnull(const __libc

[PATCH] D51262: Implement P0553 and P0556

2019-07-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Committed as revision 364862 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51262/new/ https://reviews.llvm.org/D51262 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D51262: Implement P0553 and P0556

2019-07-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked 2 inline comments as done. mclow.lists added inline comments. Comment at: libcxx/include/bit:203 +template +inline _LIBCPP_INLINE_VISIBILITY constexpr +enable_if_t<__bitop_unsigned_integer<_Tp>::value, _Tp> EricWF wrote: > Why the explicit inli

[PATCH] D51262: Implement P0553 and P0556

2019-07-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked 2 inline comments as done. mclow.lists added inline comments. Comment at: libcxx/include/bit:208 +const unsigned int __dig = numeric_limits<_Tp>::digits; +return (__cnt % __dig) == 0 +? __t EricWF wrote: > Why did you choose to

[PATCH] D51262: Implement P0553 and P0556

2019-07-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists updated this revision to Diff 207395. mclow.lists marked an inline comment as done. mclow.lists added a comment. Removed explicit `inline`s, changed ternary into `if` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51262/new/ https://reviews.llvm.org/D51262 Files: libcxx/in

[PATCH] D51262: Implement P0553 and P0556

2019-07-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked 5 inline comments as done. mclow.lists added a comment. I missed a couple of Eric's comments. Comment at: libcxx/include/bit:211 +? __t +: (__t << (__cnt % __dig)) | (__t >> (__dig - (__cnt % __dig))); +} xbolva00 wrote: > mcl

[PATCH] D51262: Implement P0553 and P0556

2019-07-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists updated this revision to Diff 207394. mclow.lists marked an inline comment as done. mclow.lists added a comment. Updated patch based on Eric's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51262/new/ https://reviews.llvm.org/D51262 Files: libcxx/include/bit li

[PATCH] D51262: Implement P0553 and P0556

2019-07-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked 7 inline comments as done. mclow.lists added inline comments. Comment at: libcxx/include/bit:236 + +if constexpr (sizeof(_Tp) <= sizeof(unsigned int)) +return __clz(static_cast(__t)) EricWF wrote: > Weird indentation. Clang-for

[PATCH] D62782: Fix -Wdouble-promotion warnings.

2019-07-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. Sorry for the slow response. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62782/new/ https://reviews.llvm.org/D62782 _

[PATCH] D51262: Implement P0553 and P0556

2019-06-25 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists updated this revision to Diff 206571. mclow.lists marked 4 inline comments as done. mclow.lists added a comment. Address a couple of review comments. Added a few more `uint128_t` rotation tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51262/new/ https://reviews.llvm.org

[PATCH] D51262: Implement P0553 and P0556

2019-06-25 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In D51262#1557154 , @jwakely wrote: > In D51262#1213514 , @mclow.lists > wrote: > > > I should also mention that as a conforming extension, I have implemented > > the non-numeric bit o

[PATCH] D51262: Implement P0553 and P0556

2019-06-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked an inline comment as done. mclow.lists added inline comments. Comment at: libcxx/include/bit:378 + const unsigned __retVal = 1u << (__n + __extra); + return (_Tp) (__retVal >> __extra); +} mclow.lists wrote: > mclow.lists wrote:

[PATCH] D51262: Implement P0553 and P0556

2019-06-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked an inline comment as done. mclow.lists added inline comments. Comment at: libcxx/include/bit:378 + const unsigned __retVal = 1u << (__n + __extra); + return (_Tp) (__retVal >> __extra); +} mclow.lists wrote: > Quuxplusone wrote:

[PATCH] D51262: Implement P0553 and P0556

2019-06-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked 4 inline comments as done. mclow.lists added inline comments. Comment at: libcxx/include/bit:252 +while (true) { +__t = rotr<_Tp>(__t, __ulldigits); +if ((__iter = countl_zero(static_cast(__t))) != __ulldigits)

[PATCH] D51262: Implement P0553 and P0556

2019-06-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked an inline comment as done. mclow.lists added inline comments. Comment at: libcxx/include/bit:378 + const unsigned __retVal = 1u << (__n + __extra); + return (_Tp) (__retVal >> __extra); +} mclow.lists wrote: > Quuxplusone wrote:

[PATCH] D51262: Implement P0553 and P0556

2019-06-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked 2 inline comments as done. mclow.lists added inline comments. Comment at: libcxx/include/bit:199 +!is_same_v, char32_t> + > {}; + Quuxplusone wrote: > Given how heavily the code controlled by this trait depends on > `numeric_limits

[PATCH] D51262: Implement P0553 and P0556

2019-06-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked an inline comment as done. mclow.lists added inline comments. Comment at: libcxx/include/bit:378 + const unsigned __retVal = 1u << (__n + __extra); + return (_Tp) (__retVal >> __extra); +} Quuxplusone wrote: > Why so complicated

[PATCH] D51262: Implement P0553 and P0556

2019-06-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. > Also are the unsigned types that aren't integral? I believe that people are allowed to specialize `is_unsigned` for their own (bignum, say) types. However, `is_integral` is not extensible. It refers to the types in `[basic.fundamental]` > Type `bool` is a distinc

[PATCH] D51262: Implement P0553 and P0556

2019-06-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked 4 inline comments as done. mclow.lists added inline comments. Comment at: include/bit:254 + +if constexpr (sizeof(_Tp) <= sizeof(unsigned int)) + return __clz(static_cast(__t)) EricWF wrote: > Cool use of `if constexpr`. > > Ple

[PATCH] D51262: Implement P0553 and P0556

2019-06-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists updated this revision to Diff 206339. mclow.lists added a comment. Update this patch to implement P1355R2 which makes out-of-bound inputs for `ceil2`UB. This was easy for integer types that are at least as big as `int`, but harder for smaller ones. CHANG

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

2019-06-18 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This looks good to me. Thanks! Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44865/new/ https://reviews.llvm.org/D44865 ___

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

2019-06-10 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. The patch looks fine to me, but I think you should consider making a couple of `T.fail.cpp` tests, and check to make sure you get the "right error". Comment at: test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp:130 { usi

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

2019-06-10 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In D44865#1051228 , @EricWF wrote: > This LGTM. > > Also I would like @mclow.lists input about applying this DR early since LWG > hasn't commented on it yet. Even though LWG didn't vote it out as a DR; that's what it is, and

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

2019-06-09 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In D44865#1391996 , @lichray wrote: > Ping @mclow.lists @EricWF ; the patch still applies, is there any other > thing I need to address? Just a note: P0608R3 was adopted in San Diego. Repository: rCXX libc++ CHANGES SI

[PATCH] D43226: __threading_support: Remove (void) in favor of ().

2019-06-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This looks fine to me. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43226/new/ https://reviews.llvm.org/D43226 ___

[PATCH] D62782: Fix -Wdouble-promotion warnings.

2019-06-03 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. This looks fine to me; but where were you getting these warnings? Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62782/new/ https://reviews.llvm.org/D62782 ___ cfe-commits mailing list

[PATCH] D43159: Modernize: Use nullptr more.

2019-06-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In D43159#1526170 , @brucem wrote: > Can we revive this review? I'd still like to land this ... What was the result of testing with `-std=c++98` and/or `-std=gnu++98` ? The code changes look fine; but as @Ericwf said Re

[PATCH] D62654: [Docs] Modernize references to macOS

2019-05-30 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I'm fine with the libc++ changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62654/new/ https://reviews.llvm.org/D62654 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D59253: [AIX][libcxx] AIX system headers need stdint.h and inttypes.h to be re-enterable when macro _STD_TYPES_T is defined

2019-05-23 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: libcxx/include/stdint.h:16 +#endif // _STD_TYPES_T /* I don't think that this will do what you want it to. Is this a supported use case? ``` #include #define _STD_TYPES_T #include ``` Repository: rG LLVM

[PATCH] D62034: compiler-rt/lib/builtins: s/#include /#include /g to match proper case.

2019-05-17 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I have no objection to this; but I'm not really confident that this won't break more than it solves. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62034/new/ https://reviews.llvm.org/D62034 _

[PATCH] D61858: Make `__is_base_of` more friendly with unions

2019-05-13 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Committed as revision 360614 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61858/new/ https://reviews.llvm.org/D61858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D61858: Make `__is_base_of` more friendly with unions

2019-05-13 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In D61858#1500270 , @ldionne wrote: > Shouldn't we also add unit tests for PR41843 in libc++? Yes. But I want to do that later. After this has landed (and probably wait a week for all the bots that are running trunk to updat

[PATCH] D61858: Make `__is_base_of` more friendly with unions

2019-05-13 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Note: There are tabs in `clang/test/SemaCXX/type-traits.cpp` that I didn't remove because it would have cluttered up the diff. I can de-tabify the file when it is committed if people want. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61858/new/ https://rev

[PATCH] D61858: Make `__is_base_of` more friendly with unions

2019-05-13 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists created this revision. mclow.lists added reviewers: rsmith, t.p.northover, EricWF, ldionne. mclow.lists added a project: clang. Herald added a subscriber: dexonsmith. Unions are never base classes, and never have base classes. It doesn't matter if they are complete or not. See http://l

[PATCH] D47358: : Implement {un,}synchronized_pool_resource.

2019-04-25 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In D47358#1438929 , @Quuxplusone wrote: > Rebased. Added `_NOEXCEPT` to `upstream_resource()` and `options()` (this is > OK per [res.on.exception.handling]/5). That's fine, but then we should have a test for that. We have th

[PATCH] D60417: [libunwind] Add support for ARMv7-M architecture which uses the Thumb 2 ISA (unified syntax)

2019-04-17 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I have no problem with this. Repository: rUNW libunwind CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60417/new/ https://reviews.llvm.org/D60417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

2019-04-02 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. I'm OK with this. I think, strictly speaking, that libc++ is following the standard, and everyone else is not - but what everyone else is doing makes sense. Comment at: libcxx/include/istream:365 __input_arith

[PATCH] D60069: Declare std::tuple_element as struct instead of class

2019-04-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. I'm less enthusiastic about this change than the one for PR39871, because there we were being inconsistent with ourselves. However, my lack of enthusiasm is no reason not to land this. CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D60069: Declare std::tuple_element as struct instead of class

2019-04-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In D60069#1449937 , @jdoerrie wrote: > Right, there shouldn't be any inheritance. Some of the `public:` access > specifications are now redundant, though. And I think that you should get rid of them. Repository: rCXX lib

[PATCH] D60069: Declare std::tuple_element as struct instead of class

2019-04-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In D60069#1449928 , @mclow.lists wrote: > Did you check the places that inherit from `tuple_element`? The > public/private bits change between class and struct. Never mind. I was thinking of something else; I don't think th

[PATCH] D60069: Declare std::tuple_element as struct instead of class

2019-04-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Did you check the places that inherit from `tuple_element`? The public/private bits change between class and struct. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60069/new/ https://reviews.llvm.org/D60069

[PATCH] D40181: [libcxx] Allow to set locale on Windows.

2019-03-18 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Herald added a project: LLVM. I have a bug report - https://bugs.llvm.org/show_bug.cgi?id=41131 - that says that we can reduce the number of calls in __libcpp_locale_guard from 10 to 2 by calling: `setlocale(LC_ALL, ...)` . Was this considered when this patch was cr

[PATCH] D59253: [AIX][libcxx] AIX system headers need stdint.h and inttypes.h to be re-enterable when macro _STD_TYPES_T is defined

2019-03-12 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists requested changes to this revision. mclow.lists added a comment. This revision now requires changes to proceed. I see no tests here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59253/new/ https://reviews.llvm.org/D59253 ___

[PATCH] D37994: Implement LWG2946: More ambiguity in `string` vs. `string_view`

2019-02-25 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists abandoned this revision. mclow.lists added a comment. Herald added a subscriber: jdoerfert. This appears to have been applied in a slightly different form. Certainly the functionality is there. Closing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37994/new/ https://review

[PATCH] D44823: [libcxx] Improving std::vector and std::deque perfomance

2019-01-29 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In D44823#1375590 , @mclow.lists wrote: > I just tried this (on Compiler Explorer) using LLVM 7, and the code for my > original test in https://bugs.llvm.org/show_bug.cgi?id=35637 is now optimal. Pilot error - it's still the

[PATCH] D44823: [libcxx] Improving std::vector and std::deque perfomance

2019-01-29 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Herald added a subscriber: libcxx-commits. I just tried this (on Compiler Explorer) using LLVM 7, and the code for my original test in https://bugs.llvm.org/show_bug.cgi?id=35637 is now optimal. Looking briefly at your test case, it seems to be fixed now too. Can you

[PATCH] D14686: Protect against overloaded comma in random_shuffle and improve tests

2019-01-24 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Herald added a subscriber: llvm-commits. (Finally) committed this as revision 352087. I cut out most of the random_shuffle_rand.pass.cpp test, because it relied on C++11 features, and didn't work for C++03. If you want to re-submit th

[PATCH] D16541: [libc++] Renable test/std/re/re.alg/re.alg.match/awk.pass.cpp

2019-01-23 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Herald added a reviewer: EricWF. For some reason, this test was not marked `// XFAIL gnu-linux` like all the other tests that use the `LOCALE_cs_CZ_ISO8859_2` locale. In revision 352006, I uncommented this entire test, and added the X

[PATCH] D26110: Add a check for GCC to the _LIBCPP_EXPLICIT define

2019-01-23 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Yes, this appears to have been landed. Closing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D26110/new/ https://reviews.llvm.org/D26110 ___ cfe-commits mailing list cfe-commits

[PATCH] D28248: Work around GCC PR37804

2019-01-23 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. I changed the `_LIBCPP_TYPE_VIS_ONLY` to `_LIBCPP_TEMPLATE_VIS` and committed this as revision 351993. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28248/new/ https://reviews.llvm.org/D28248

[PATCH] D57001: [libunwind] Don't define unw_fpreg_t to uint64_t for __ARM_DWARF_EH__

2019-01-23 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I have no problem with the code change, but no context on whether or not it is correct. I'm hoping some other people familiar with ARM and DWARF can chime in. Repository: rUNW libunwind CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57001/new/ https://rev

[PATCH] D56905: [libunwind] [SjLj] Don't use __declspec(thread) in MinGW mode

2019-01-18 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. Sigh. At least we've got a macro wrapping this; we only have to pore over this once. Repository: rUNW libunwind CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56905/new/ https://reviews.llvm.org/D56905 __

[PATCH] D51762: First part of the calendar stuff

2019-01-14 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists marked 2 inline comments as done. mclow.lists added inline comments. Comment at: include/chrono:2667 +#if _LIBCPP_STD_VER > 17 +constexpr chrono::day operator ""d(unsigned long long __d) noexcept +{ EricWF wrote: > Including this file with Cla

[PATCH] D53763: [libc++] [test] Fix logic error in tests; enable for MSVC previews

2019-01-14 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. I'm a bit concerned about the `TEST_HAS_NO_SPACESHIP_OPERATOR` and how it tracks with `_LIBCPP_HAS_NO_SPACESHIP_OPERATOR`, but I'm not going to hold this up for that. CHANGES SINCE

[PATCH] D53956: Fix test assumption that Linux implies glibc.

2018-11-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I hate this chunk of code. :-( `TEST_HAS_C11_FEATURES` basically means that we can use C11 library features like `aligned_alloc`, and `timespec` etc. Repository: rCXX libc++ https://reviews.llvm.org/D53956 ___ cfe-co

[PATCH] D51762: First part of the calendar stuff

2018-10-15 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In https://reviews.llvm.org/D51762#1266086, @NoQ wrote: > Had to revert. Sorry! https://reviews.llvm.org/rL344580. > > This failure was masked by another error, so i guess it was missed. This was supposed to be fixed by commit r344535. https://reviews.llvm.org/D51

[PATCH] D51762: First part of the calendar stuff

2018-10-15 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. After discussions with @EricWF, I landed a modified version as revision 344529. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D51762: First part of the calendar stuff

2018-10-12 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. This is the first of part of this functionality. Some of these bits are not here yet. Comment at: test/std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.members/ctor.sys_days.pass.cpp:10 +// UNSUPPORTED: c++03, c++11, c++14, c++17 +// XFAIL

[PATCH] D42242: Make libc++abi work with gcc's ARM unwind library

2018-10-10 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Committed as revision 344152 https://reviews.llvm.org/D42242 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42242: Make libc++abi work with gcc's ARM unwind library

2018-10-08 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In https://reviews.llvm.org/D42242#1257428, @mgorny wrote: > @mclow.lists , ping. Any chance to get a proper version upstream? I'd rather > not pull patches from Fedora when we can have something official. I gave up on this patch several months ago, because no one

[PATCH] D46975: Implement deduction guides for `std::deque`

2018-10-08 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. Committed as r332785 https://reviews.llvm.org/D46975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-10-01 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. Ok. I'm fine with this. Thanks for your patience. Repository: rL LLVM https://reviews.llvm.org/D52401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-30 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In https://reviews.llvm.org/D52401#1250552, @MaskRay wrote: > > I seem to recall a problem with that > > I would like to know if your impression came from the common PWN technique > when the attacker found a heap buffer overflow :) No; that's not what I'm looking i

[PATCH] D52401: Remove redundant null pointer check in operator delete

2018-09-30 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I suspect it's fine, but I need to check some stuff on old versions of glibc (I seem to recall a problem with that). Repository: rL LLVM https://reviews.llvm.org/D52401 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D52368: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32

2018-09-21 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. Repository: rCXXA libc++abi https://reviews.llvm.org/D52368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

[PATCH] D52368: [libc++abi] is_strcmp parameter to is_equal is unused for WIN32

2018-09-21 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. This seems like overkill to me; why not add `(void) use_strcmp` right before line 67 instead? Repository: rCXXA libc++abi https://reviews.llvm.org/D52368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

2018-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. Symbol added: _ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEElsEDn {'is_defined': True, 'type': 'FUNC', 'name': '_ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEElsEDn'} Symbol added: _ZNSt3__113basic_ostreamIwNS_11char_traitsIwEEElsEDn {'is_defined': True, 'type

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

2018-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists reopened this revision. mclow.lists added a comment. This revision is now accepted and ready to land. Reverted in r342590 while I investigate additions to the dylib. https://reviews.llvm.org/D44263 ___ cfe-commits mailing list cfe-commit

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

2018-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Landed as revision 342566 https://reviews.llvm.org/D44263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50551: [libcxx] [test] Add missing to several tests.

2018-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LFTM https://reviews.llvm.org/D50551 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D50799: Fix for PR 38495: no longer compiles on FreeBSD, due to lack of timespec_get()

2018-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Landed as r339816 https://reviews.llvm.org/D50799 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50815: Establish the header

2018-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Landed as r339943 https://reviews.llvm.org/D50815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50876: Clean up newly created header

2018-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. Landed as r340049 https://reviews.llvm.org/D50876 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-19 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: test/support/verbose_assert.h:26 static_assert(ST == -1, "specialization required for ST != -1"); static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; } }; > The renaming is to clarify that a

[PATCH] D51868: [libcxx] Build and test fixes for Windows

2018-09-18 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: test/support/verbose_assert.h:24 : (IsStreamable::value ? 2 : -1))> -struct SelectStream { +struct SelectErrStream { static_assert(ST == -1, "specialization required for ST != -1"); Why the renaming here?

[PATCH] D51955: Create infrastructure for defining and testing feature test macros

2018-09-12 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. landed as revision 342073 https://reviews.llvm.org/D51955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51955: Create infrastructure for defining and testing feature test macros

2018-09-11 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists created this revision. mclow.lists added reviewers: ldionne, EricWF. Herald added a reviewer: jfb. Herald added a subscriber: jfb. mclow.lists edited the summary of this revision. In P0941 , the committee standardized a bunch of feature test macros. [ I think

[PATCH] D50101: [asan] Update a vector's storage annotation during destruction.

2018-09-07 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists closed this revision. mclow.lists added a comment. landed as revision 341671. Repository: rCXX libc++ https://reviews.llvm.org/D50101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D51262: Implement P0553 and P0556

2018-08-25 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. I should also mention that as a conforming extension, I have implemented the non-numeric bit operations for `std::byte` https://reviews.llvm.org/D51262 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

[PATCH] D51262: Implement P0553 and P0556

2018-08-25 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists created this revision. mclow.lists added reviewers: EricWF, ldionne. Herald added a subscriber: christof. LWG adopted https://wg21.link/P0553 in Rapperswil, and suggested minor changes to https://wg21.link/P0556. They kind of go together; for example, `ispow2` is easily implemented u

[PATCH] D50876: Clean up newly created header

2018-08-17 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. In https://reviews.llvm.org/D50876#1204531, @mclow.lists wrote: > @craig.topper - that's existing code; I'm not changing it. > If we have a test bot that I can test this against, I'm happy to update it. I'm not really sure that this code is actually used anywhere.

[PATCH] D50876: Clean up newly created header

2018-08-17 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment. @craig.topper - that's existing code; I'm not changing it. If we have a test bot that I can test this against, I'm happy to update it. https://reviews.llvm.org/D50876 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D50876: Clean up newly created header

2018-08-17 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: include/bit:96 #if defined(_LIBCPP_HAS_BITSCAN64) (defined(_M_AMD64) || defined(__x86_64__)) + if (_BitScanForward64(&__where, __x)) I'm not sure this code is ever used - since how can this compile? https://

[PATCH] D50876: Clean up newly created header

2018-08-17 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists updated this revision to Diff 161284. mclow.lists added a comment. Clean up the windows code a bit - though I don't think is used - since I don't think it will compile. https://reviews.llvm.org/D50876 Files: include/__bit_reference include/bit Index: include/bit ==

[PATCH] D50876: Clean up newly created header

2018-08-17 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: include/bit:61 +inline _LIBCPP_INLINE_VISIBILITY +int __popcount(unsigned __x) { return __builtin_popcount (__x); } + ldionne wrote: > Funny spacing between `__builtin_popcount` and `(__x)` It's actually t

[PATCH] D50876: Clean up newly created header

2018-08-17 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: include/bit:117 + unsigned long __where; // Search from LSB to MSB for first set bit. // Returns zero if no set bit is found. lebedev.ri wrote: > Like i commented in the original review, this should probably b

[PATCH] D50876: Clean up newly created header

2018-08-16 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added inline comments. Comment at: include/bit:113 inline _LIBCPP_INLINE_VISIBILITY unsigned __clz(unsigned __x) { static_assert(sizeof(unsigned) == sizeof(unsigned long), ""); Missed this one. Should be `int` https://reviews.llvm.org/D50876

  1   2   3   4   5   >