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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

Overall looks good to me. Leave it open for few days, maybe someone else want to comment.



================
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)) {
----------------
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


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:261-262
+- Fixed an issue in :doc:`performance-noexcept-move-constructor
+  <clang-tidy/checks/performance/noexcept-move-constructor>` which resulted in false
+  positives if the constructor was defaulted.
+
----------------
Maybe: Fixed an issue in the performance-noexcept-move-constructor checker that was causing false-positives when the move constructor or move assign operator were defaulted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146922