- User Since
- Feb 11 2015, 3:26 PM (228 w, 4 h)
Please note that this review is really about integrating pstl into libc++, not pstl itself. I know there are changed needed before pstl is ready for prime time (and LIBCXX_ENABLE_PARALLEL_ALGORITHMS becomes ON by default).
Thu, Jun 20
Non-draft version of the patch.
Wed, Jun 19
From my perspective, this patch is fine since we already have __libcpp_condvar_t and __libcpp_mutex_t, so in some way we're even making things more consistent. I'll commit this tomorrow unless there are reasons why this is harmful.
Tue, Jun 18
Can you test this against Clang trunk and confirm that it works?
Mon, Jun 17
I don't mind the change (it's safe, simple, etc.). However, I don't understand why timespec is being tied to threads and < __external_threading>. Is is possible that it's only a convenient way of doing it right now?
Fri, Jun 14
I couldn't find an equivalent to -exported_symbols_lists on GNU ld, but I'm all ears.
Thu, Jun 13
I'll leave this open for a little while so that people have a chance to say if they're still using it -- but I think this is just dead code now.
I looked at it and I don't think this is an ABI break. Because of the way __regex_word is used (it's never passed through an ABI boundary, instead we use "w". Also this doesn't impact Apple platforms, so I'm personally fine with it.
Wed, Jun 12
Tue, Jun 11
Thu, Jun 6
Wed, Jun 5
2750 nothing to do because std::experimental::optional no longer exists.
Thanks for working on this.
I'll ship this next week once I've tested it on a Linux Docker image (I don't have access to that right now). I'm not sure, but I think I remember trying to make this change a while back and breaking the bots.
Tue, Jun 4
Just to confirm, @mclow.lists are we OK with this since the LWG issue has been filed?
I'm okay with this (I don't care much about GCC), but I want to point out you're basically reducing the usefulness of the test suite on GCC 5/6 by just wholesale removing all C++17 tests, when GCC5/6 can actually run a large subset of the tests.
@phosek You may want to take a look too.
I'll fix it when applying the patch.
Mon, Jun 3
I think separate tests for a >= b and a <= max would be better, though.
This seems reasonable to me. We seem to already be passing this test.
Fri, May 31
Address reviewer's comments