[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

mibintc abandoned this revision. mibintc added a comment. The bug should be fixed at the atomic-load (drop _Atomic qualifier) then this patch won't be needed in StmtExpr Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59307/new/ https://reviews.llvm.org/D59307

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

mibintc added a comment. In D59307#1427853 , @erichkeane wrote: > This is my concern here: > https://godbolt.org/z/icS4fa > The patch will change template instantiation. > > GCC doesn't seem to allow using _Atomic in C++ mode, which is perhaps a > nece

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

mibintc added a comment. In D59307#1428145 , @jfb wrote: > In D59307#1428101 , @mibintc wrote: > > > In D59307#1427644 , @jfb wrote: > > > > > I think you also want to test C

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

jfb added a comment. In D59307#1428101 , @mibintc wrote: > In D59307#1427644 , @jfb wrote: > > > I think you also want to test C++ `std::atomic` ... > > > The bug described in https://bugs.llvm.org/show_bug.cgi?id=4

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

mibintc added a comment. In D59307#1427644 , @jfb wrote: > I think you also want to test C++ `std::atomic` ... The bug described in https://bugs.llvm.org/show_bug.cgi?id=41033 doesn't occur using C++ atomic, I rewrote the test case this way, and it comp

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

erichkeane added a comment. This is my concern here: https://godbolt.org/z/icS4fa The patch will change template instantiation. GCC doesn't seem to allow using _Atomic in C++ mode, which is perhaps a necessary part of this solution? I see we already consider overload sets with int and _Atomic

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

mibintc added a comment. In D59307#1427663 , @jfb wrote: > Thinking some more, this is really a silly gotcha in code. We should probably > follow what we're about to do with wg21.link/P1152 > and ban chaining. Statement e

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

mibintc added a comment. In D59307#1427665 , @jfb wrote: > From an offline discussion, I'm told that @jwakely uses this in GCC headers > and expects some semantics from it? Jonathan, why?!?! Yes this is how I stumbled into the issue. Repository: rC

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

mibintc marked 2 inline comments as done. mibintc added a comment. Thanks for test suggestions, will follow up Comment at: test/Sema/atomic-expr-stmt.c:7 + //CHECK: %atomic-load = load atomic i32, i32* %a.addr seq_cst, align 4 + //CHECK: store atomic i32 %atomic-load, i32* %t

RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

lists.llvm.org; Keane, Erich ; > rich...@metafoo.co.uk; r...@google.com; jfbast...@apple.com > Cc: jdoerf...@anl.gov; mlek...@skidmore.edu; blitzrak...@gmail.com; > shen...@google.com > Subject: RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of > statement expressions >

RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

com > Subject: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of > statement expressions > > jfb requested changes to this revision. > jfb added a comment. > This revision now requires changes to proceed. > > I think you also want t

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

jfb added a subscriber: jwakely. jfb added a comment. From an offline discussion, I'm told that @jwakely uses this in GCC headers and expects some semantics from it? Jonathan, why?!?! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59307/new/ https://reviews.llvm.

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

jfb added a comment. Thinking some more, this is really a silly gotcha in code. We should probably follow what we're about to do with wg21.link/P1152 and ban chaining. Statement expressions should therefore not be allowed to return `_Atomic`, `std::atomic`, nor

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

jfb requested changes to this revision. jfb added a comment. This revision now requires changes to proceed. I think you also want to test C++ `std::atomic` as well as regular `volatile`. Comment at: test/Sema/atomic-expr-stmt.c:7 + //CHECK: %atomic-load = load atomic i32, i32*

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

mibintc created this revision. mibintc added reviewers: cfe-commits, erichkeane, rsmith. Herald added subscribers: jdoerfert, jfb. Herald added a project: clang. Clang considers the type of a statement expression that returns the value of an _Atomic(ty) to be atomic, and this makes the statement