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

Piotr Zegar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 26 11:28:48 PDT 2023


PiotrZSL requested changes to this revision.
PiotrZSL added a comment.
This revision now requires changes to proceed.

Current solution is invalid, defaulted move constructors can throw by default.
There are 2 more issues for this case:
https://github.com/llvm/llvm-project/issues/38081
https://github.com/llvm/llvm-project/issues/41414



================
Comment at: clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp:46
+    // specifier
+    if (Decl->isDefaulted() && ProtoType->getExceptionSpecType() == EST_None)
+      return;
----------------
that's not true...

```
#include <memory>

struct Field
{
    Field() = default;
    Field(Field&&) {

    }
};

struct Test
{
    Test(Test&&) = default;
    Field field;
};

static_assert(std::is_nothrow_move_constructible<Test>::value);
```

Is default, not noexcept...

Try something like this:
```
bool cannotThrow(const FunctionDecl *Func) {
  const auto *FunProto = Func->getType()->getAs<FunctionProtoType>();
  if (!FunProto)
    return false;

  switch (FunProto->canThrow()) {
  case CT_Cannot:
    return true;
  case CT_Dependent: {
    const Expr *NoexceptExpr = FunProto->getNoexceptExpr();
    bool Result;
    return NoexceptExpr && !NoexceptExpr->isValueDependent() &&
           NoexceptExpr->EvaluateAsBooleanCondition(
               Result, Func->getASTContext(), true) &&
           Result;
  }
  default:
    return false;
  };
}

// Source: https://reviews.llvm.org/D145865
```

so you could do something like this:

```
if (cannotThrow(Decl))
    return;
```

I don't see reason to limit this check only to default'ed. This could be improved, to simply validate also things like ```noexcept(std::is_nothrow_move_constructible<TemplateType>::value)```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146922/new/

https://reviews.llvm.org/D146922



More information about the cfe-commits mailing list