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

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D59307#1428145 <https://reviews.llvm.org/D59307#1428145>, @jfb wrote:

> In D59307#1428101 <https://reviews.llvm.org/D59307#1428101>, @mibintc wrote:
>
> > In D59307#1427644 <https://reviews.llvm.org/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 compiles without error.  The "load" operation returns int since all the atomic operations occur under the covers using builtins.   Does this convey the test you have in mind?
> >
> > #include <atomic>
> >  int fum(int y) {
> >
> >   std::atomic<int> x(1);
> >   y = ({x.load();});
> >
> > }
>
>
> What I had in mind with `atomic` isn't relevant, because it would try to call `atomic(const atomic&) = delete;`. Your test case here isn't something I'm worried about.
>
> `volatile` is still and issue.
>
> I'm still not sure this is something we want.


Now I think the problem needs to be fixed at the atomic load, not in this place.  Thanks for pointing out the odd use of tmp. I'm going to abandon this patch.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59307/new/

https://reviews.llvm.org/D59307