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