[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

[Soumi Manna via Phabricator via cfe-commits]

Manna marked 6 inline comments as done.
Manna added inline comments.


================
Comment at: clang/include/clang/Sema/ParsedAttr.h:704
   /// Move the given pool's allocations to this pool.
   AttributePool(AttributePool &&pool) = default;
 
----------------
tahonermann wrote:
> This class has a move constructor, but the implicit declaration of a move assignment operator will be inhibited due to the presence of this and other special member functions. That means that an attempted move assignment will find the deleted copy assignment. That is ok; if support for move assignment is desired some day, it can be added then.
I have added defaulted move assignment operator which seems needed based on analysis with other bugs.  


================
Comment at: clang/include/clang/Sema/Sema.h:1786
     SemaDiagnosticBuilder(SemaDiagnosticBuilder &&D);
+    SemaDiagnosticBuilder &operator=(SemaDiagnosticBuilder &&D);
     SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
----------------
tahonermann wrote:
> Since this class declares a move constructor, the declaration of the implicit move assignment operator is inhibited and the implicitly declared assignment operator is defined as deleted. This change declares a move assignment operator but doesn't define it (perhaps `= delete` was intended?). If support for assignment is not desired, then I think a declaration of a deleted copy assignment operator is all that is needed (matching the change made for `Strategy` below). Otherwise, I think a defaulted copy assignment operator should be declared and a move assignment operator should be defined that implements the same behavior as the move constructor.
Thanks @tahonermann for the comments. 

>> think a defaulted copy assignment operator should be declared and a move assignment operator should be defined that implements the same behavior as the move constructor.

I have updated patch based on further analysis and my understanding. This seems reasonable to me.


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

https://reviews.llvm.org/D149718