[PATCH] D146922: [clang-tidy] Fix false positve for defaulted move constructor in performance-noexcept-move-constructor

[André Schackier via Phabricator via cfe-commits]

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