- User Since
- Jul 16 2012, 3:06 PM (309 w, 6 h)
Thu, Jun 14
I'm not really happy with gcc's redundant declaration warnings; I think they're "nannying" rather than useful.
Tue, May 29
Howard just pointed out to me that __clear_and_shrink should be noexcept - otherwise we get the generation of an exception table and a call to terminate in string's move assignment operator. Fixed in revision 333435.
May 18 2018
Committed as revision 332768
May 17 2018
Added tests (and fix!) for the implicit deduction guides.
May 16 2018
I should probably add some tests for the implicit deduction guides, too.
May 15 2018
I'm fine with this, since it is localized (sorry) inside the Android bits.
I have no opinion on the >= 21 or >= 26 discussion.
What we need to do here is to here is to add a new macro _LIBCPP_ABI_REGEX_MEMORY in <__config> (around line 89) and then make these changes available only when this macro is defined.
Sorry this took so long. Please update test/libcxx/double_include.sh.cpp and commit.
One way we could deal with this is by adding an attribute to the compiler to indicate "the const is a lie", that we can apply to std::pair::first, with the semantics being that a top-level const is ignored when determining the "real" type of the member (and so mutating the member after a const_cast has defined behavior). This would apply to all std::pairs, not just the node_handle case, but in practice we don't optimize on the basis of a member being declared const *anyway*, so this isn't such a big deal right now.
I'm a fan of this idea, this would also have the bonus of fixing some pre-existing type-punning between pair<const K, V> and pair<K, V> (see hash_value_type and value_type in <unordered_map> and <map>).
May 1 2018
Apr 25 2018
BTW, you can gang several failing tests together, and check all the error messages - see libcxx/test/libcxx/containers/unord/unord.set/missing_hash_specialization.fail.cpp for an example.
This looks OK to me.
Apr 24 2018
Please move the tests into test/std and commit.
Apr 23 2018
This looks ready to land to me.
I'm fine with this change, but I'm wondering:
- if there are similar tests in the filesystem test suite, and you haven't tripped over them because you spelled your error message the same as we did.
- Should part of this test be moved into test/libcxx
Apr 4 2018
When you add a new header file to libc++, you have to update two additional files:
Apr 2 2018
[rand.device]/2 seems to be the authoritative word here:
If implementation limitations prevent generating nondeterministic random numbers, the implementation may employ a random number engine.
Mar 27 2018
My first response (based on looking at the patch, and doing a bit of research) is:
- This is needed
- I'm not sure this is quite the right approach.
Mar 26 2018
Please don't commit this.
Mar 21 2018
I'm going to stop here, because all the things I've noted are ticky-tack; formatting and minor changes.
More substantial comments coming soon.
A few small comments...
Sorry I've let this lie fallow for so long.
Mar 19 2018
We don't usually do this in source files, because they don't get included in people's builds - just the library build.
Mar 8 2018
Mar 7 2018
Added the proposed resolution for https://wg21.link/lwg3075 as well, and some deduction guide tests for string_view
Mar 6 2018
This was landed as r324619
Mar 1 2018
Add the tuple bits. Regularize the equality comparisons of the containers; i.e, don't try to be clever - let the compiler be clever.
Feb 26 2018
I can certainly make them all the same.
Is there a subtle reason for this inconsistency that I'm not seeing?
forgot to include the changes for <forward_list> in the diff.
Feb 24 2018
LGTM - thanks!
Feb 21 2018
Feb 19 2018
Feb 17 2018
LGTM. Do you need me to commit this?
Feb 15 2018
The <cassert> bit is fine.
The veryLarge is "ok".
The rest of them are just bogus.
Which clang-tidy module generates this warning, and what is the actual warning?
Feb 14 2018
If we *have* to do this (and we still need to have that discussion), I think something like this would be better
A couple of points to consider here:
- "Windows users expect" is not a compelling argument.
- If done, this behavior should be windows-specific.
- You should include <__config> before anything else.
I see no benefit to this change. bool(0) is false.
Feb 13 2018
Feb 7 2018
Since we've implemented P0600, I believe that this patch is out of date.
The correct macro is _LIBCPP_NODISCARD_AFTER_CXX17, and I think that several of these cases are now handled.
This all looks good to me.
I think that one more test should be added - and that's one that tests __sqr directly.
Since that's not a public routine, the test should go in "test/libcxx/numerics/complex.number"
Feb 6 2018
I'm fine with this, though I wonder if using the more specific _LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS might be a better thing to define.
Committed as revision 324378
The second change is not quite right. There's a couple of lines that need indentation changes.
Phab is not really good about changes like this. "This file was changed only by adding or removing whitespace"
Jan 30 2018
I like this. One nit and a question.
Please be sure to update www/cxx2a_status.html to mark 2993 as "Complete".
Other than that, looks good to me! Thanks!
Jan 25 2018
Looks good to me. Thanks!
I'm pretty sure I don't want to know what MSFT is doing putting align_val_t in *that* header file.
This LGTM. Sorry for the breakage.
Clearly we need a test with a non op== BinaryPredicate
Jan 23 2018
Committed as revision 323296
This looks fine to me.
Find *all* the places that we are using exception_class and wrap them in helper routines.
Jan 22 2018
Update macro checks.
Jan 21 2018
Since __clear_and_shrink() is private
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.
Jan 19 2018
This looks fine to me. Thanks!
This looks good to me.
Please add a test in test/libcxx/strings/string.modifiers and commit.
Jan 18 2018
Jan 17 2018
I'd like to call attention to http://libcxxabi.llvm.org, specifically the FAQ entry: Why are the destructors for the standard exception classes defined in libc++abi? They're just empty, can't they be defined inline?
I think that there needs to be more docs on the CFI checking; https://clang.llvm.org/docs/ControlFlowIntegrity.html is about using it, not about the design, goals, and implementation.
Jan 16 2018
I'm a bit leery of this patch. Not because of what it's trying to do, but rather, the introduction of a method __clear_and_shrink that leaves the string in an invalid state. For all the uses that you put it to, I don't think that's a problem (though I'm still working through the failure possibilities), but I can see other people attempting to use this method - and not realizing that you have to "put the string back together" afterwards.
Jan 13 2018
Can you share your benchmark results, please?
Jan 11 2018
Committed as r322295
Do you need me to commit this?
Please make the formatting match the rest of the file.
A couple of nits, but other than that, looks fine.
Jan 10 2018
I like the fact that you've removed the extra test that you've added.
However, I think that modifying the tests as you've done is more work than is needed.
I *suspect* that all you need to do is to move a couple of test cases from the "calls which should fail" to the "calls that should succeed" list (and adjust for the fact that the calls return false even though they succeed)