- User Since
- Jun 26 2014, 12:44 PM (325 w, 2 d)
Wed, Sep 9
Tue, Sep 1
Aug 17 2020
LGTM, this fixes some build failures at Google.
Aug 12 2020
Jul 15 2020
LGTM as well. Thanks for working on this!
Jul 13 2020
IDK if I know what's causing the issue, but this change LGTM.
Jun 19 2020
Jun 18 2020
This is not the ABI we want.
Jun 17 2020
Jun 15 2020
Jun 9 2020
How do we implement the "Mandates" requirement specified here?
Jun 4 2020
std::any, like other type erased types, cannot follow the allocator model and therefore does not use allocators.
All of that being said, this code is correct and LGTM**.
May 29 2020
Why is this code an improvement?
@mvels Do we have macro-benchmark results for this change?
May 13 2020
I'm a bit confused by the build size numbers you gave compared to the patch.
We could lift the two macros that depend on _NEWLIB_VERSION into the headers that use them.
Then we'll avoid the include in config.
May 7 2020
I can't find any usages of this internally.
May 6 2020
If we don't want the static env, we should remove it entirely.
Meaning, we should convert each of these tests to use dynamic_test_env and setup the required files like the remainder of the tests do.
While I support this change, it should really be accompanied by a library working group issue or paper.
May 5 2020
Unless I'm mistaken, this change is racy. Tests in different files are run in parallel.
One test could be tearing down the static environment while another in trying to construct it.
When I initially wrote the filesystem tests, I had divided them between "static" and "dynamic"; that is, tests which modified the target filesystem and those which did not.
There are some simplifications suggested in the inline comments.
Once those are addressed, this LGTM.
Apr 23 2020
Apr 20 2020
Apr 16 2020
The entire point of the static test environment is that it's /static/.
Meaning it's checked in and not created on the fly.
This needs a bunch of additional tests. Specifically of the passing kind :-)
Apr 15 2020
Apr 10 2020
Apr 9 2020
Apr 8 2020
I assume this is a behavioral change? Why are there no new tests?
Committed as 601f7631827ae6ac08117a282c83a62b67dedf48
Apr 7 2020
I have some concerns with how we harden our concepts,
but these can be addressed in follow up commits.
Apr 4 2020
Apr 3 2020
I like tests.
Assuming all of them pass;: LGTM.
Mar 31 2020
Mar 26 2020
I don't necessarily agree.
Lets not start writing tests like this.
Mar 24 2020
We don't implement non-standard extensions in libc++.
Only when this trait is proposed for standardization can we consider adding it to the library
Mar 23 2020
I would like us to be even more aggressive bumping our compiler support,
but that's a conversation for another time.
Mar 21 2020
Why do we need separate options for each project?
I think that's the wrong direction.
Mar 18 2020
These tests still require that "arbitrary" warning flag be enabled.
I'm not sure I love this, but I'm OK demoting warnings to errors in the XFAIL tests.
Mar 16 2020
Mar 14 2020
Mar 13 2020
Could we add a .fail.cpp test that ensures when _LIBCPP_HAS_NO_FGETPOS is defined we don't actually have it?
Some of the changes in this patch are not correct. Please revert them in a follow up commit.
Mar 11 2020
I think there is a bug in the paper, but I think the return type should be difference_type not size_type.
The paper says the return value is equivalent to:
LGTM other than the lack of tests.
Does this really need concepts?
Could you please upload this diff with more context?
Is this just a clean move, or is the class definition changed as well?
I can't tell with all the linter warnings in the way.
Your good to commit.
- This check should suggest fixes. It knows what function is actually being resolved, and it has enough information to create a minimally qualified name that resolves it.
- This check should ignore hidden friends, since their entire purpose is to be called via ADL.
- This check should allow whitelisting namespaces that opt-out of ADL into their namespace. This makes it much easier to roll out the clang-tidy incrementally.
I'll be picking this up seriously this week.
I'm currently getting an internal version of this clang-tidy reviewed,
and my plan was to submit it upstream after because I didn't see this
patch until now (bad email filters).
I've already stated my disapproval of this patch. Libc++ has never and will never provide nor value C++03 conformance.
Moving backwards to C++03 is inconsistent with the libraries general direction.
Why are we declaring this function at all?