- User Since
- Jun 26 2014, 12:44 PM (160 w, 4 d)
Tue, Jul 18
Checking out libcxx should not be a requirement since libunwind should be usable independently on libcxx.
Wed, Jul 12
+1 for moving this file to LLVM's internal style.
Add more appropriate test.
Tue, Jul 11
- Address most review comments except those about spaceBreakBetween and spaceBreakBefore.
@alexfh Thanks for correcting the reviewers. I had no idea who to add so I used something like git-suggest-reviewers.
And they lack reserved names. Sorry for the repeated comment spam.
The return types for the new __libcpp_sync builtins are incorrect as well.
I'm not OK with adding a new header here. The problem is that __refstring requires using the new <__atomic_support> but <__refstring> shouldn't be a header anyway.
Mon, Jul 10
Sun, Jul 9
@morehouse Thanks for fixing this!
Maybe using a static const variable instead of an enum would work better? __long_mask should never actually be ODR used so a definition for it should never actually be needed.
I think the test could be improved. First could you add the test within test/SemaCXX/coroutines.cpp? Second could you add some negative tests that check the diagnostics generated when you don't provide a specialization of coroutine traits (ie that the old behaviour of not including the class type now produces diagnostics). Could you also add tests for const/volatile and lvalue/rvalue qualified member functions?
LGTM. I'll deal with ensuring all vendors are willing to take this ABI break (or that they avoid it).
This patch causes test_demangle.pass.cpp to fail with UBSan.
However there is another bug here. operator=(function&&) doesn't correctly call the destructor of the functor. I'll fix that as a separate commit.
@dexonsmith I'm not sure it's sane to allow reentrant behavior. Could you explain why you think it is? Should the copy assignment operator allow reentrancy as well?
@dexonsmith Mind if I hijack this and check in your changes to <functional> with my tests?
Wed, Jul 5
Creating a function pointer with proper parameters pointing to std::next() or std::prev() should work.
This LGTM. I'm assuming it works but we can iron out the kinks once it lands. We should probably add builders that run the tests as well.
Why don't you start by adding __libcpp_sync_foo functions that we can use, then we can decide on what macros to use to guard their definition.
LGTM. @STL_MSFT This seems like you could commit it without review. You're a professional STL developer after all, if you can't get this right may God have mercy on your users.
I'll try and get to this tonight.
Tue, Jul 4
On architectures without atomic instructions, the atomic built-ins cannot be lowered. If _LIBCPP_HAS_NO_THREADS is enabled, we should just use regular code.
Jun 21 2017
Jun 20 2017
Jun 18 2017
Jun 17 2017
@rsmith ping. I need this for http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0607r0.html
Jun 16 2017
Jun 15 2017
- Address inline comments.
A large portion of this patch is UB. Any function that unconditionally calls __get_db() can never be a valid constant expression, hence UB.
Could you re-upload this with a updated diff against trunk, and one with more context.
- Update against trunk.
- Fix the test when -discard-value-names is present, at least my part of the test.
- Address most inline comments.
Jun 14 2017
Jun 13 2017
- Improve test by using regular expressions and [[Name]] where possible.
- Improve tests
- Remove unneeded assertion.
- Fix assertion for co_yield.
- Remove changes to how CoroutineSuspendExprs ExprValueType is calculated. They were incorrect. However this means that Clang still fails to compile co_await and co_yield expressions where await_resume returns an lvalue reference. @GorNishanov Can you take a look at this? Reproducer:
Could you please add the benchmark under libcxx/benchmarks.