[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign
Soumi Manna via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed May 10 12:45:01 PDT 2023
Manna added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:1786
SemaDiagnosticBuilder(SemaDiagnosticBuilder &&D);
+ SemaDiagnosticBuilder &operator=(SemaDiagnosticBuilder &&D);
SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
----------------
tahonermann wrote:
> Manna wrote:
> > 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.
> This change still declares a move assignment operator, but doesn't provide a definition. The move constructor is implemented in `clang/lib/Sema/Sema.cpp`, so I would expect to see the move assignment operator definition provided there as well.
Thanks @tahonermann for the comments. I have removed `Sema.h` change from the current patch. I will address it in a separate review pull request. Need to look into this more.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149718/new/
https://reviews.llvm.org/D149718
More information about the cfe-commits
mailing list