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