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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
