- User Since
- Feb 11 2015, 3:26 PM (292 w, 3 d)
Fri, Sep 18
Assuming you already have them, I'd love to see the rest of the patches in the queue to know what we're getting into.
A few more comments, but this is looking pretty good.
Thu, Sep 17
This is superseded by https://reviews.llvm.org/D68364. Thanks a lot for your work on this @mpark, I based my patch on top of yours. I tried taking all the comments from this review and incorporating them into D68364.
Complete implementation of P0784 with tests
- Reduce your commit message to get rid of the benchmark output
- Run: for std in c++03 c++11 c++14 c++17 c++2a; do <build-dir>/bin/llvm-lit -sv libcxx/test/std/containers/associative/map --param std=$std; done to make sure it all works
This looks really good, thanks! Just one comment about structured bindings, and I think we're good to go.
Wed, Sep 16
This is superseded by https://reviews.llvm.org/D31413. Closing to clear up the review queue.
@hans Is there still time to cherry-pick this to the 11 release?
That's one hell of a commit message :-)
Added a test. I can't reproduce the issue, so I don't know whether the test is useful or not. Please help with that!
This is abandoned for https://reviews.llvm.org/D31413.
Let's close this in favour of https://reviews.llvm.org/D31413 to clean things up.
I did some work on trying to reproduce this a while ago but never got to reproducing it on macOS.
Tue, Sep 15
I'll ship this now. Thanks.
No answer in a while -- it looks like this isn't relevant anymore. Cleaning up the review queue.
@tkoeppe Does this look like a suitable resolution to PR45099?
I like this change. It's simple but I think it makes sense. I'll add tests and finish it up.
Abandoned to clear up the review queue.
This review isn't relevant anymore, the _PSTL_VERSION is now tied to LLVM's version.
@rsmith Would you like me to commit this?
Please add a comment explaining why it's disabled for no-exceptions, and commit. It's far from obvious.
The approach to be taken for all runtime libraries is the one I described with specifying a custom config file. Please abandon!
Mon, Sep 14
Is this review still relevant? If not, let's abandon it to clear up the review queue.
Change __cxa_atexit back to returning int
Re-committed as 3ed89b51da38f081fedb57727076262abb81d149 with UNSUPPORTED markup.
Is everyone happy with this?
Remove the declarations of cxa_finalize and cxa_atexit
Trying to clean up the review queue. Commandeering to propose what seems to be the consensus here.
For the record, I tested this in all standard modes using
Finish up the tests
Commandeering to show exactly what we meant by the requested test changes, however we're very very close here. Thanks a lot for your contribution @Nicholas-Baron !
Commandeering so I can abandon to clean up the review queue.
This sounds reasonable to me, however = default is not going to work in C++03 (unless there's a Clang extension I'm not aware of). @zoecarver Do you still need this?
Is this still relevant? If not, can we close this to clean up the review queue?
Is this still relevant? You should now be able to specify your own linker flags in the config file by modelling your file on something like libcxx/test/configs/libcxx-trunk-shared.cfg.in.
Please take a look at libcxx/test/configs/libcxx-trunk-shared.cfg.in and LIBCXX_TEST_CONFIG in CMake for how to customize exactly how you're running the test suite. This is a lot more powerful and simple than stringing options through CMake!
This was committed in fc4bff0cd37fa84ee74e6dff7170b643df3ffa42.
I'm not seeing any issues with the perfect forwarding anymore. This looks pretty good, with a few comments.
This almost LGTM. You can commit after fixing those comments and I'll adjust the markup for AppleClang as needed.
I like to keep these executors really simple and doing only a single thing. Would you consider splitting this out into a separate executor instead? Here's what I got, roughly:
LGTM, but based on D84421, do you actually need the extra scp args?
Thanks for working on that! I left some general comments, however I believe the best person to give you an in-depth review about this is @__simt__, as a co-author of the paper and as being the de-facto owner of <atomic>.
Fri, Sep 11
Implement the changes for libc++.
I thought use_system_cxx_lib was only used on Apple platforms!