[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This looks good to me. Please add a test in test/libcxx/strings/string.modifiers and commit. Something like this: #include #include int main () { std::string l = "Th

[PATCH] D42291: [libcxx] Correctly handle invalid regex character class names

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This looks fine to me. Thanks! Comment at: test/std/re/re.regex/re.regex.construct/bad_ctype.pass.cpp:28 +} catch (const std::regex_error &ex) { +result

[PATCH] D42344: [libc++] Use multi-key tree search for {map, set}::{count, equal_range}

mclow.lists added a comment. Sometimes you get lucky ;-) I have a task for next week that says "The transparent lookup stuff in the associative containers needs tests", and here is this patch. Instead of "transparent_count.pass.cpp", please name the test "count_transparent.pass.cpp" so it will

[PATCH] D42344: [libc++] Use multi-key tree search for {map, set}::{count, equal_range}

mclow.lists added a comment. Shouldn't there be tests for `multimap` and `multiset`, too? https://reviews.llvm.org/D42344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42344: [libc++] Use multi-key tree search for {map, set}::{count, equal_range}

mclow.lists added a comment. In https://reviews.llvm.org/D42344#983265, @ng wrote: > In https://reviews.llvm.org/D42344#983264, @mclow.lists wrote: > > > Shouldn't there be tests for `multimap` and `multiset`, too? > > > Sure; will the same tests as for map/set be alright? Actually, I think the

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

mclow.lists added a comment. In https://reviews.llvm.org/D41976#982978, @tvanslyke wrote: > I don't have commit access myself so I've added the test to the diff. I'll commit it then. https://reviews.llvm.org/D41976 ___ cfe-commits mailing list cf

[PATCH] D41976: Low-hanging fruit optimization in string::__move_assign().

mclow.lists added a comment. > Since __clear_and_shrink() is private There's no reason it has to be private. People aren't supposed to call anything with a reserved name. https://reviews.llvm.org/D41976 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D42357: Under limitation of allocated buffer, inplace_merge() does NOT take usage of partial allocated buffer but applies native rotate directly.

mclow.lists added inline comments. Comment at: include/algorithm:2511 +difference_type __len1, __len2; +__len1 = __middle - __first; +__len2 = __last - __middle; These iterator calculations only work for random access iterators. Does this actually wor

[PATCH] D35472: Implement P0463R1: "Endian just Endian"

mclow.lists updated this revision to Diff 130991. mclow.lists added a comment. Update macro checks. https://reviews.llvm.org/D35472 Files: include/type_traits test/std/utilities/meta/meta.type.synop/endian.pass.cpp Index: test/std/utilities/meta/meta.type.synop/endian.pass.cpp ===

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

mclow.lists updated this revision to Diff 131102. mclow.lists added a comment. Find *all* the places that we are using `exception_class` and wrap them in helper routines. https://reviews.llvm.org/D42242 Files: src/cxa_default_handlers.cpp src/cxa_exception.cpp src/cxa_exception.hpp src

[PATCH] D42357: Under limitation of allocated buffer, inplace_merge() does NOT take usage of partial allocated buffer but applies native rotate directly.

mclow.lists added inline comments. Comment at: include/algorithm:2515 +{ +pointer __buff_end = __move(__first, __middle, __buff); +__move(__middle, __last, __first); mclow.lists wrote: > Probably a good idea to qualify these calls with `_VSTD

[PATCH] D35472: Implement P0463R1: "Endian just Endian"

mclow.lists closed this revision. mclow.lists added a comment. Committed as revision 323296 https://reviews.llvm.org/D35472 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42354: Fix libcxx MSVC C++17 redefinition of 'align_val_t'

mclow.lists added a comment. I'm pretty sure I don't want to know what MSFT is doing putting `align_val_t` in *that* header file. Now I'm wondering if those values `__zero` and `__max` are actually used. Repository: rCXX libc++ https://reviews.llvm.org/D42354

[PATCH] D40259: [libcxx] LWG2993: reference_wrapper

mclow.lists accepted this revision. mclow.lists added a comment. Please be sure to update www/cxx2a_status.html to mark 2993 as "Complete". Other than that, looks good to me! Thanks! https://reviews.llvm.org/D40259 ___ cfe-commits mailing list cfe-c

[PATCH] D42693: [libcxx] Handle invalid escaped characters in POSIX regex

mclow.lists added a comment. I like this. One nit and a question. Comment at: include/regex:3490 { switch (*__temp) { Do we need any more cases here? Comment at: test/std/re/re.regex/re.regex.co

[PATCH] D28933: Revert the return type for `emplace_(back|front)` to `void` in C++14 and before

mclow.lists closed this revision. mclow.lists added a comment. Landed as r292990. https://reviews.llvm.org/D28933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D20660: Remove `auto_ptr` in C++17.

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. Landed as r292986 https://reviews.llvm.org/D20660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

[PATCH] D28473: Implement http://wg21.link/P0426 Constexpr for std::char_traits

mclow.lists closed this revision. mclow.lists added a comment. Landed as r291741, r291742 and r293154. https://reviews.llvm.org/D28473 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28172: [libcxx] Remove unexpected handlers in C++17

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. Sorry for the slow response. https://reviews.llvm.org/D28172 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

[PATCH] D30045: Remove `std::random_shuffle` in C++17

mclow.lists created this revision. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4190 removed `random_shuffle` from C++1z. (and other stuff) Wrap all the random_shuffle bits in an #ifdef so they disappear when compiling with `-std=c++1z` or later. Introduce a new configuration opti

[PATCH] D30035: Add const to function parameters

mclow.lists added a comment. Absent I good motivation (i.e, a documented, significant performance change), I don't think we want this - not because it's not a reasonable change (it is!), but because it's an ABI break. It's unfortunate the `__atoms` were not passed as const in the first place. A

[PATCH] D27068: Improve string::find

mclow.lists added a comment. Definitely want to see numbers. https://reviews.llvm.org/D27068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26991: Hoist redundant load

mclow.lists added a comment. /me wonders what the perf difference when `_LIBCPP_UNROLL_LOOPS` is defined or not. I think this (`_LIBCPP_UNROLL_LOOPS`) falls squarely into Chandler's request that we complain to him when the compiler generates sub-optimal code, instead of doing things like manu

[PATCH] D26991: Hoist redundant load

mclow.lists added a comment. __search is the only place where `_LIBCPP_UNROLL_LOOPS` is currently used. https://reviews.llvm.org/D26991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D26896: [libcxx] Make constexpr char_traits and char_traits

mclow.lists added inline comments. Comment at: include/__string:213 -static inline int compare(const char_type* __s1, const char_type* __s2, size_t __n) _NOEXCEPT -{return __n == 0 ? 0 : memcmp(__s1, __s2, __n);} -static inline size_t length(const char_type* __

[PATCH] D27096: Protect locale tests under noexceptions

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This LGTM. https://reviews.llvm.org/D27096 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[PATCH] D27093: Protect std::{, unordered_}map tests under noexceptions

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D27093 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D27095: Protect std::array tests under noexceptions

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. Other than the missing `assert`s, (which are not your fault, but I would appreciate you fixing) this LGTM. Comment at: test/std/containers/sequences/array/at.pass.

[PATCH] D26611: Protect test for dynarray under libcpp-no-exceptions

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D26611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D26612: Protect std::string tests under libcpp-no-exceptions

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. Thanks. https://reviews.llvm.org/D26612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D26991: Hoist redundant load

mclow.lists added a comment. There are no uses of `_LIBCPP_UNROLL_LOOPS` in LLVM (other than the ones in ``. Googling for `_LIBCPP_UNROLL_LOOPS` on github finds the ones in libc++, and no others. I think I'll just take it out, and see what happens. https://reviews.llvm.org/D26991 _

[PATCH] D26991: Hoist redundant load

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This looks fine to me - though I wonder if the compiler can hoist `*__first2` w/o us helping it. https://reviews.llvm.org/D26991 ___ c

[PATCH] D26991: Hoist redundant load

mclow.lists requested changes to this revision. mclow.lists added inline comments. This revision now requires changes to proceed. Comment at: libcxx/include/algorithm:1499 +// Load the first element from __first2 outside the loop because it is loop invariant +typename it

[PATCH] D27252: Protect sequences test under libcpp-no-exceptions

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM> https://reviews.llvm.org/D27252 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D27255: Protect std::ostream::sentry test under libcpp-no-exceptions

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D27255 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D27254: Protect optional test under libcpp-no-exceptions

mclow.lists added inline comments. Comment at: test/std/experimental/optional/optional.specalg/swap.pass.cpp:225 } +#ifndef TEST_HAS_NO_EXCEPTIONS { Why is this here, and not before line L#236? https://reviews.llvm.org/D27254 __

[PATCH] D27253: Protect futures test under libcpp-no-exceptions

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D27253 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[PATCH] D27199: [libcxx] Make std::ignore constexpr

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This looks good to me. https://reviews.llvm.org/D27199 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D27254: Protect optional test under libcpp-no-exceptions

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. Looks good now - thanks. https://reviews.llvm.org/D27254 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D27068: Improve string::find

mclow.lists added a comment. This is starting to look good. Comment at: libcxx/include/__string:549 +// Stop short when source is smaller than pattern. +ptrdiff_t __len2 = __last2 - __first2; +if (__len2 == 0) Is there a reason that you calculate th

[PATCH] D27436: [libcxx] [test] std::get<0>([std::variant constant expression]) *is* noexcept

mclow.lists accepted this revision. mclow.lists added a comment. I'd appreciate it if someone made a note to revisit this test when the clang bug is fixed, and change the `#ifndef __clang__ ` to something like `#ifndef __clang__ || clang_version < `". This LGTM. https://reviews.llvm.org/D

[PATCH] D27429: [Chrono][Darwin] On Darwin use CLOCK_UPTIME_RAW instead of CLOCK_MONOTONIC

mclow.lists added a comment. I agree with @EricWF . If `CLOCK_UPTIME_RAW` doesn't meet the requirements of `steady_clock` (i.e, monotonically increasing, and advances in real time), then we can't use it. https://reviews.llvm.org/D27429 ___ cfe-

[PATCH] D33033: [libc++] Fix PR32979 - types with a private std::enable_shared_from_this base break shared_ptr

mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33033 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D33021: [libcxx] [test] libc++ test changes for CWG 2094

mclow.lists added a comment. In general, I prefer having the space between `static_assert(` and the condition. That makes it easy (at least for me) to see if there's a `!` there or not. Other than that, this LGTM. FWIW, I think this is a language change that core got wrong, and it has already

<    1   2   3   4   5