Manna added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:1789-1791
+ SemaDiagnosticBuilder &operator=(SemaDiagnosticBuilder &&D) = delete;
SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
+ SemaDiagnosticBuilder &operator=(const SemaDiagnosticBuilder &) = delete;
----------------
tahonermann wrote:
> aaronpuchert wrote:
> > Manna wrote:
> > > @tahonermann This is follow-up comments from
> > > https://reviews.llvm.org/D149718?id=519331#inline-1452044.
> > >
> > > >>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.
> > >
> > > I tried to define move assignment operator in ` clang/lib/Sema/Sema.cpp`
> > > but it failed because class Sema has deleted implicit copy assignment
> > > operator.
> > >
> > > ```
> > > /// Sema - This implements semantic analysis and AST building for C.
> > > class Sema final {
> > > Sema(const Sema &) = delete;
> > > void operator=(const Sema &) = delete;
> > > ```
> > > It seems like support for assignment is not desired, We probably need
> > > deleted copy/move assignment operator.
> > >
> > These are also implicitly deleted. Some code styles want this explicitly
> > spelled out, but I don't think ours does.
> > I tried to define move assignment operator in clang/lib/Sema/Sema.cpp but
> > it failed because class Sema has deleted implicit copy assignment operator.
>
> It is still permissible to define a move assignment operator if the implicit
> copy assignment operator is deleted. See https://godbolt.org/z/sGaWd9M44.
>
> I think it is fine to disable support for assignment for this class pending
> use cases. But, since a move constructor is explicitly defined, we should
> also be explicit above move assignment. I added a suggested edit. Without
> that change, I think Coverity will continue to complain.
I see! Thanks @tahonermann for the explanation
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150411/new/
https://reviews.llvm.org/D150411
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
