- User Since
- Jul 16 2012, 3:06 PM (352 w, 4 d)
Wed, Apr 17
I have no problem with this.
Tue, Apr 16
Why is this necessary? I just grepped the libc++abi sources, and I see no only two references to _LIBCPP_HAS_NO_THREADS, and they're both in comments (which should be fixed, btw, but that's neither here nor there).
This looks ok to me.
Mon, Apr 15
The paper suggests an implementation like this:
This is too simplistic.
Thu, Apr 4
Why does list<_Tp, _Alloc>::remove(const value_type& __x) gather all the deleted nodes into a separate list, while
list<_Tp, _Alloc>::remove_if(_Pred __pred) and list<_Tp, _Alloc>::unique() just delete them as they go?
Ok, so your update and my comments passed in flight. I'll try again :-)
I think I'd like to see the actual error messages.
Wed, Apr 3
How does this compare to D59344 ?
Thanks for doing all this. I'm not really ignoring these, it just seems like it. :-(
I think we're way, way out on the bell curve here.
People who are building their own libc++ and statically linking it with libc++abi and using the sanitizers.
Those people (I believe) will be able to set a CMake flag.
Does that address your concern?
I'm a bit worried about nodiscard_extensions.pass.cpp, because lit by default does not show warnings; just "N warnings in tests" in the summary.
Tue, Apr 2
You're going to have to find a different approach here.
Traversing the list to see how the size has changed is not acceptable.
The test is obviously incomplete.
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.
Mon, Apr 1
See https://stackoverflow.com/questions/31974237/why-is-libcs-vectorboolconst-reference-not-bool/31974238#31974238 for Howard's explanation of vector<bool>::const_reference
Other than those nits, this looks fine to me.
I like the tests. Still looking at the code.
Do we have to worry about other pointer types than id?
I committed the tests and an alternate fix in r357410
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.
Did you check the places that inherit from tuple_element? The public/private bits change between class and struct.
Sat, Mar 30
I don't think that this is right. All advance does is update an iterator. No data is being moved here.
I think we're going to get this from the work that @ldionne and Thomas Rodgers are doing on the pstl project. You should check with them.
Fri, Mar 29
Landed as r356162
I landed this already. Closing.
So, Roman and I did some playing around with compiler explorer, and we discovered that if you define _LIBCPP_DISABLE_EXTERN_TEMPLATE so that you're not using the string code that's in the dylib, then you get the benefits of this patch w/o any code changes.
I don't know about how much this will improve performance (there should be, as Roman says, before/after IR to verify this), but I'm satisfied with the correctness of this patch.
Thu, Mar 28
Wed, Mar 27
And it is part of C++20, so it's not final anyway.
Tue, Mar 26
I'm mostly worried about the change in behavior breaking existing programs.
How did you decide which modes fell into "when possible"?
I've started implementing and testing to_string via to_chars and realized that to_chars doesn't support wchar_t unlike to_wstring.
I'm going to extend __to_chars_itoa implementation in order to support wchar_t and reuse those internal template function.
Could you please confirm that this way is ok in order not to through away the third attempt on completion.
Mon, Mar 25
Fri, Mar 22
Thu, Mar 21
Very impressive. Still looking.
Mar 20 2019
Now that revision 356585 has landed, we can use std::to_chars to implement to_string
In general, I'm not happy with this direction.
I think it is unwise to have two different implementations of "integer to string" functionality in the library, doing the same thing.
I'm happy with the idea of making to_string faster, but not this way.
Mar 19 2019
Mar 18 2019
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 created?
Mar 15 2019
Mar 14 2019
I have marked test/libcxx/debug/containers/db_sequence_container_iterators.pass.cpp and libcxx/containers/sequences/array/array.zero/db_indexing.pass.cpp because I marked operator and front and back as noexcept.
Mar 13 2019
We don't usually gang tests for multiple containers together - one/container is more common,
Mar 12 2019
Now that we have to_chars, can we use those facilities for to_string? (rather than duplicate them?)
[ Ok, we don't have \to_chars` for FP yet ]
Address Eric's comments.
Remove all the explicit template lists from the tests.
Add a couple "odd" tests.
Add a couple more failing pointer tests.
Go for it. I'm curious to see how it works out :-)
How do the tests look? Do they need updating to ensure that we have the (new) correct behavior?
I see no tests here.
Mar 11 2019
I'm fine with this.
Mar 8 2019
So basically the contents of the file were deleted but the file itself wasn't removed
It was added as part of https://reviews.llvm.org/D17416, and that is all it's ever been.
Mar 7 2019
I don't like how intrusive and pervasive this is.
Mar 6 2019
You should probably have at least one (and probably several) simple tests that underlying_type<non enum type> is a valid type.
using Int = std::underlying_type<int>; using Float = std::underlying_type<float>;