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

[Tom Honermann via Phabricator via cfe-commits]

tahonermann requested changes to this revision.
tahonermann added a comment.
This revision now requires changes to proceed.

This looks good with one exception; I think the changes for class `SemaDiagnosticBuilder` are not what was intended.



================
Comment at: clang/include/clang/Analysis/BodyFarm.h:41-44
   BodyFarm(const BodyFarm &other) = delete;
 
+  /// Delete copy assignment operator.
+  BodyFarm &operator=(const BodyFarm &other) = delete;
----------------
This looks fine; move constructor and assignment declarations are inhibited.


================
Comment at: clang/include/clang/Sema/ParsedAttr.h:704
   /// Move the given pool's allocations to this pool.
   AttributePool(AttributePool &&pool) = default;
 
----------------
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.


================
Comment at: clang/include/clang/Sema/ParsedAttr.h:915-916
   ParsedAttributes(AttributeFactory &factory) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes &operator=(const ParsedAttributes &) = delete;
 
----------------
This looks fine; move constructor and assignment declarations are inhibited.


================
Comment at: clang/include/clang/Sema/Sema.h:1786
     SemaDiagnosticBuilder(SemaDiagnosticBuilder &&D);
+    SemaDiagnosticBuilder &operator=(SemaDiagnosticBuilder &&D);
     SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
----------------
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.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:764-766
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy &operator=(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
----------------
This is ok. The implicit move assignment declaration is inhibited and since copy assignment is deleted, no assignment is supported. If move assignment is desired someday, it can be added.


================
Comment at: clang/lib/Serialization/ASTWriterStmt.cpp:44-45
 
     ASTStmtWriter(const ASTStmtWriter&) = delete;
+    ASTStmtWriter &operator=(const ASTStmtWriter &) = delete;
 
----------------
This looks fine; implicit move construction and assignment declarations are inhibited.


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

https://reviews.llvm.org/D149718