- User Since
- Jan 27 2014, 9:36 AM (264 w, 4 d)
Thu, Feb 21
Fri, Feb 15
Overall this LGTM, but it would be great to get @pcc to double-check. He's got way more attention to details than I do :)
Incidentally the handling of volatile atomic operations here is probably wrong as I don't think we can have any kind of repeated loading, but that's nothing to do with this change (both LDXR/STXR loop and CAS loop have this problem) and doesn't need to be fixed by it.
A few nits, LGTM otherwise.
Thu, Feb 14
- Fix polarity
This is a good start!
For the idempotent RMW, atomicrmw or, ptr, 0 sgtm. I assume that you'd do the same for floating-point (with a cast)? I wonder if in the long run we'd actually just want a separate instruction for this...
- Update with John's suggestion.
Wed, Feb 13
Two small comments, LGTM otherwise (but would like one of the libc++ maintainers to sign off too).
I talked to Akira in meatspace, and it seems like this updated patch does the right thing. He suggested changing the AST as a longer-term solution, but for now this approach seems simple enough.
- Check for references when looking for copyexpr directly.
Oh this is great, looking forward to it!
Tue, Feb 12
Overall this LGTM besides a few nits, and wanting input from @rjmccall.
Mon, Feb 11
The tests are exactly what I had in mind, great! LGTM.
Sat, Feb 9
You need support from the compiler for constexpr support. I don't think we want this without constexpr. @erik.pilkington was looking into this.
Fri, Feb 8
Do you ever use weak pointers? The single allocation is good until you start holding on to more things (because you can't just keep the block around, you need the entire allocation), so you should make sure you won’t be delaying memory being reclaimed.
Can you add a link to bug 40605 in the commit message?
Thu, Feb 7
- Remove whitespace changes.
- Simplify patch greatly.
- Only initialize __block's storage.
Regarding libstdc++: the check is currently conservatively correct, but not thorough enough, because older revisions of GCC still get bug fix release which pass the date check. Past GCC 7 we can use _GLIBCXX_RELEASE, but before this @tstellar points out how LLDB does it. I don't know if we have to fix this issue: we simply fail to warn a small subset of libstdc++ users.
Thanks for taking this on! As I said in the bug I think we definitely want something like this change.
Wed, Feb 6
Looks like clang-with-lto-ubuntu is still on an old version of GCC. Let's see if more bots are broken, I'm not sure how many stragglers we have.
One fix then this LGTM
Thanks for doing this!
FWIW you might find wg21.link/n4455 relevant :)
Tue, Feb 5
I'm wondering if, as a follow-up, the XFAIL tests that validate the behavior for exceptions should instead: