AMS21 added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp:45-49
+ const auto *ProtoType = Decl->getType()->castAs<FunctionProtoType>();
+ const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
+ if (NoexceptExpr) {
+ NoexceptExpr = NoexceptExpr->IgnoreImplicit();
+ if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) {
----------------
PiotrZSL wrote:
> AMS21 wrote:
> > PiotrZSL wrote:
> > > woudn't getExceptionSpecInfo() != EST_NoexceptFalse do a trick here ?
> > I've tested it and it seem that a struct like this:
> >
> > ```
> > struct B {
> > static constexpr bool kFalse = false;
> > B(B &&) noexcept(kFalse);
> > };
> > ```
> >
> > Also has the the `ExceptionSpecType` set to `EST_NoexceptFalse`. So
> > changing this would change the previous behavior.
> And thats why I pointing this, we should treat this function in same way as
> it would have noexcept(false)
>
> but functions that use templates like:
> noexcept(T::value) should not fall into this
Ah okay. But I'm not so sure about this change. It would effectively make us
report on less cases where we actually know that the move constructor is
declared as throwing.
It might be that the user made some logic mistake in their `noexcept`
expression which leads to it always evaluating to false and I think we should
still report in that case.
If you don't care about the performance implication of a throwing move
constructor or have other reasons to want them, you can always simply disable
this check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146922/new/
https://reviews.llvm.org/D146922
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
