- User Since
- Jul 16 2012, 3:06 PM (292 w, 4 d)
Wed, Feb 21
Mon, Feb 19
Sat, Feb 17
LGTM. Do you need me to commit this?
Thu, Feb 15
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?
Wed, Feb 14
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.
Tue, Feb 13
Wed, Feb 7
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"
Tue, Feb 6
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"
Tue, Jan 30
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!
Thu, Jan 25
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)
Jan 9 2018
Investigating the assignability of not_fn. All the rest of this looks fine.
According to 15.8.2 [class.copy.assign]/2 and /4, this makes call_wrapper non-assignable.
Jan 5 2018
The use of iota targeting vector<unsigned char> with an int parameter triggers warnings on MSVC++ assigning an into a unsigned char&
Jan 2 2018
This was committed as part of r321658
Dec 21 2017
Dec 20 2017
I've got an implementation for this, too - at https://github.com/mclow/snippets/blob/master/to_chars.cpp
Dec 19 2017
LGTM. I'm going to sprinkle const throughout this file later, but that is a drive-by thing.
All the lines that start out ptrdiff_t __hm = will soon be const ptrdiff_t __hm = ,
but that's not necessary for this bug fix.
Dec 18 2017
The rest of this LGTM.
Dec 15 2017
I'm wondering if it's not a better idea to have an explicit specialization for size == 0
Dec 14 2017
More context in the diff, and removed some tabs.
Also commented out the constexpr tests for divide, since they fail at the moment.
There are no tests because this should not change any functionality.
Dec 13 2017
Other than the actual text being output, this LGTM.
I'ld like to see the changes I suggested in the test go in, but they're really minor.
LGTM. Do you need someone to commit it?
Dec 12 2017
Thanks, I've checked this in without the changes to TODO.TXT
These look fine to me. Thanks for the attention to detail.
Avoid MSVC "warning C4293: '<<': shift count negative or too big, undefined behavior".
MSVC sees (1ULL << N) and warns - being guarded by const bool canFit is insufficient.
Dec 11 2017
This looks good to me; with a couple of nits.
Dan - I think I need a bit more context here.
How does UBSan get triggered?
Except for the stuff in TODO.txt these look good to me.
We need to do some work on that file - it's pretty out of date.
Dec 10 2017
Ah - that was the factor I was missing.
The tests pass for me with -std=c++2a, but fail for std=c++17
These tests don't fail for me. (using a clang I built two days ago)
Dec 4 2017
Committed as revision 319736
Committed as revision 319687
Nov 30 2017
Nov 28 2017
Wrapped the string_view bits in #ifdef for C++2a.
Sorry for the extra bits.
Nov 23 2017
Committed as revision 318919
LGTM. I will apply.
Nov 22 2017
Nov 21 2017
_VSTD:: qualify the call to __launder.
De-dup error messages in test.
D'Oh - that was a paste that went wrong.
You are correct that assert(c.size() == 3) is correct.