[libcxx-commits] [PATCH] D90968: Fix for the Bug 41784

Mon Dec 14 07:49:50 PST 2020

ldionne added inline comments.

================
Comment at: libcxx/include/atomic:1866
+    // delete explicitly in the most derived class to fix the Bug 41784
+#ifndef _LIBCPP_CXX03_LANG
+    atomic& operator=(const atomic&) = delete;
----------------
rarutyun wrote:
> zoecarver wrote:
> > rarutyun wrote:
> > > zoecarver wrote:
> > > > Correct me if I'm wrong, but I don't think we need this. Clang in C++03 mode supports these. 
> > > I am not sure. I've checked it and it works without that macro check but I did that for some subset of compilers. I took into account both GCC and clang. I suspect it may give some warnings on some compilers but on the other hand we have #pragma GCC system_header, so I am not sure how to prove the necessity of that branch. I also tried to reproduce some strange behavior by playing with `-pedantic` option but with no luck.
> > > 
> > > The reason why I add that check is it initially was in `__atomic_base` class and it's still there for copy-constructor. Unless we can prove this check is unnecessary and everything would work on all platforms with any compiler I would prefer to leave it as is.
> > Thanks for going to all the trouble of testing with several configurations. I'm actually pretty sure we don't need it. We use to support many more compilers (and namely compilers that didn't support deleting copy constructors in C++03 mode)-- that's why it was/still is present in parts of the codebase. 
> > 
> > The only compiler we support in C++03 mode is Clang 4+, so, as I said, I'm pretty sure we're in the clear. Also, as long as the buildbots pass, I think we're OK (for future reference). 
> Ok, I have made that. Let's see if CI shows any problems with that change.
Can you please rebase the patch onto `main` and re-upload the patch? It will trigger CI.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90968