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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver accepted this revision.
zoecarver added inline comments.


================
Comment at: libcxx/include/atomic:1799
+    // delete explicitly in the most derived class to fix the Bug 41784
+    atomic& operator=(const atomic&) = delete;
+    atomic& operator=(const atomic&) volatile = delete;
----------------
rarutyun wrote:
> zoecarver wrote:
> > IIUC this line is a NFC? I think it makes sense to have these both here, just checking. 
> If my assumption is correct that NFC is "Not Functional Change")) then the answer is not exactly. Technically I think we can leave one overload but in that case user would get less informative compiler message (checked on clang). Depending on what overload we choose to leave the user may see either message about ambiguity of `operator=` or that `atomic` does not provide the copy-assignment with `volatile` qualifier.
> 
> With the current patch user sees clear message: "overload resolution selected deleted operator '='"
> If my assumption is correct that NFC is "Not Functional Change")) then the answer is not exactly.

Yes, that's what I mean. I'm asking because if there is a new error message, maybe we should test it. But it doesn't look like there is...

Using the Godbolt link you sent, it looks like for non-volatile atomic variables, libc++ says:

> error: overload resolution selected deleted operator '='
> error: object of type 'std::atomic<int>' cannot be assigned because its copy assignment operator is implicitly deleted

So, the first part at least won't change. 

Anyway, this still looks good to me. Thanks.


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

https://reviews.llvm.org/D90968