[PATCH] D150411: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.


================
Comment at: clang/include/clang/Sema/Sema.h:1790
     SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
+    SemaDiagnosticBuilder &operator=(const SemaDiagnosticBuilder &) = delete;
     ~SemaDiagnosticBuilder();
----------------
aaronpuchert wrote:
> tahonermann wrote:
> > 
> Being explicit is certainly a good thing, but the C++ committee wants to go into the direction of "copy/move operations are implicitly deleted if any of them or the destructor is user-declared." See [depr.impldec](https://eel.is/c++draft/depr.impldec). It is already partially the case, for example here. So why do need to spell something out explicitly when the language goes into the direction of being even more implicit?
I have low expectations of that deprecation ever being acted on. In fact, there have been discussions about un-deprecating it because of the lack of such intent.

The motivation for deprecation is not based on a judgement of whether implicit or explicit is better, but rather based on a safety argument; if a copy constructor or destructor is user-declared, that is probably because some form of resource management is needed that won't be performed by the defaulted copy assignment operator. Thus, defining the implicit copy assignment operator as deleted would avoid the possibility of the compiler injecting bugs in the program.

The rules for when the copy vs assignment operators are implicitly defined as deleted can be hard to remember. I think being explicit is useful if only to indicate that the author thought about whether such operations should be provided. Since we don't have a principled reason for defining these operations as deleted, it might not be a bad idea to add a comment that states something like "The copy/move assignment operator is defined as deleted pending further motivation".


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

https://reviews.llvm.org/D150411