[clang-tools-extra] 5abe338 - [clang-tidy] Fix false positve for defaulted move constructor in performance-noexcept-move-constructor

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 14 10:45:32 PDT 2023


Author: AMS21
Date: 2023-04-14T17:43:37Z
New Revision: 5abe338f2a7f6f99708bd268419de6fb6e3da9e3

URL: https://github.com/llvm/llvm-project/commit/5abe338f2a7f6f99708bd268419de6fb6e3da9e3
DIFF: https://github.com/llvm/llvm-project/commit/5abe338f2a7f6f99708bd268419de6fb6e3da9e3.diff

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

Previously a struct like this:

template <typename>
struct A { A(A&&) = default; };

Would trigger a false positive, since even though it is not marked as
noexcept it still is due to the `= default`.
Now we only give a warning if the defaulted move constructor is
actually declared as throwing and correctly resolve it if they are
defaulted.

This fixes llvm#56026, llvm#41414, llvm#38081

Reviewed By: PiotrZSL

Differential Revision: https://reviews.llvm.org/D146922

Added: 
    clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
    clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h

Modified: 
    clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
    clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
    clang-tools-extra/clang-tidy/utils/CMakeLists.txt
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
    clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
index 3e651391e143b..bf659ad14eed4 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp
@@ -18,61 +18,55 @@ namespace clang::tidy::performance {
 
 void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-      cxxMethodDecl(anyOf(cxxConstructorDecl(), hasOverloadedOperatorName("=")),
-                    unless(isImplicit()), unless(isDeleted()))
+      cxxMethodDecl(unless(isImplicit()), unless(isDeleted()),
+                    anyOf(cxxConstructorDecl(isMoveConstructor()),
+                          isMoveAssignmentOperator()))
           .bind("decl"),
       this);
 }
 
 void NoexceptMoveConstructorCheck::check(
     const MatchFinder::MatchResult &Result) {
-  if (const auto *Decl = Result.Nodes.getNodeAs<CXXMethodDecl>("decl")) {
-    bool IsConstructor = false;
-    if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(Decl)) {
-      if (!Ctor->isMoveConstructor())
-        return;
-      IsConstructor = true;
-    } else if (!Decl->isMoveAssignmentOperator()) {
-      return;
-    }
-
-    const auto *ProtoType = Decl->getType()->castAs<FunctionProtoType>();
+  const auto *Decl = Result.Nodes.getNodeAs<CXXMethodDecl>("decl");
+  if (!Decl)
+    return;
 
-    if (isUnresolvedExceptionSpec(ProtoType->getExceptionSpecType()))
-      return;
+  if (SpecAnalyzer.analyze(Decl) !=
+      utils::ExceptionSpecAnalyzer::State::Throwing)
+    return;
 
-    if (!isNoexceptExceptionSpec(ProtoType->getExceptionSpecType())) {
-      auto Diag = diag(Decl->getLocation(),
-                       "move %select{assignment operator|constructor}0s should "
-                       "be marked noexcept")
-                  << IsConstructor;
-      // Add FixIt hints.
-      SourceManager &SM = *Result.SourceManager;
-      assert(Decl->getNumParams() > 0);
-      SourceLocation NoexceptLoc = Decl->getParamDecl(Decl->getNumParams() - 1)
-                                       ->getSourceRange()
-                                       .getEnd();
-      if (NoexceptLoc.isValid())
-        NoexceptLoc = Lexer::findLocationAfterToken(
-            NoexceptLoc, tok::r_paren, SM, Result.Context->getLangOpts(), true);
-      if (NoexceptLoc.isValid())
-        Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
-      return;
-    }
+  const bool IsConstructor = CXXConstructorDecl::classof(Decl);
 
-    // Don't complain about nothrow(false), but complain on nothrow(expr)
-    // where expr evaluates to false.
-    if (ProtoType->canThrow() == CT_Can) {
-      Expr *E = ProtoType->getNoexceptExpr();
-      E = E->IgnoreImplicit();
-      if (!isa<CXXBoolLiteralExpr>(E)) {
-        diag(E->getExprLoc(),
-             "noexcept specifier on the move %select{assignment "
-             "operator|constructor}0 evaluates to 'false'")
-            << IsConstructor;
-      }
+  // Don't complain about nothrow(false), but complain on nothrow(expr)
+  // where expr evaluates to false.
+  const auto *ProtoType = Decl->getType()->castAs<FunctionProtoType>();
+  const Expr *NoexceptExpr = ProtoType->getNoexceptExpr();
+  if (NoexceptExpr) {
+    NoexceptExpr = NoexceptExpr->IgnoreImplicit();
+    if (!isa<CXXBoolLiteralExpr>(NoexceptExpr)) {
+      diag(NoexceptExpr->getExprLoc(),
+           "noexcept specifier on the move %select{assignment "
+           "operator|constructor}0 evaluates to 'false'")
+          << IsConstructor;
     }
+    return;
   }
+
+  auto Diag = diag(Decl->getLocation(),
+                   "move %select{assignment operator|constructor}0s should "
+                   "be marked noexcept")
+              << IsConstructor;
+  // Add FixIt hints.
+  const SourceManager &SM = *Result.SourceManager;
+  assert(Decl->getNumParams() > 0);
+  SourceLocation NoexceptLoc =
+      Decl->getParamDecl(Decl->getNumParams() - 1)->getSourceRange().getEnd();
+  if (NoexceptLoc.isValid())
+    NoexceptLoc = Lexer::findLocationAfterToken(
+        NoexceptLoc, tok::r_paren, SM, Result.Context->getLangOpts(), true);
+  if (NoexceptLoc.isValid())
+    Diag << FixItHint::CreateInsertion(NoexceptLoc, " noexcept ");
+  return;
 }
 
 } // namespace clang::tidy::performance

diff  --git a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
index c68392abf7780..149aa24c1707b 100644
--- a/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOEXCEPTMOVECONSTRUCTORCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "../utils/ExceptionSpecAnalyzer.h"
 
 namespace clang::tidy::performance {
 
@@ -25,10 +26,13 @@ class NoexceptMoveConstructorCheck : public ClangTidyCheck {
   NoexceptMoveConstructorCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context) {}
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
-    return LangOpts.CPlusPlus11;
+    return LangOpts.CPlusPlus11 && LangOpts.CXXExceptions;
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  utils::ExceptionSpecAnalyzer SpecAnalyzer;
 };
 
 } // namespace clang::tidy::performance

diff  --git a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt
index 70edb9c33d74b..4556d7b0d134b 100644
--- a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt
@@ -8,6 +8,7 @@ add_clang_library(clangTidyUtils
   ASTUtils.cpp
   DeclRefExprUtils.cpp
   ExceptionAnalyzer.cpp
+  ExceptionSpecAnalyzer.cpp
   ExprSequence.cpp
   FileExtensionsUtils.cpp
   FixItHintUtils.cpp

diff  --git a/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
new file mode 100644
index 0000000000000..8795d9e99f391
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp
@@ -0,0 +1,273 @@
+//===--- ExceptionSpecAnalyzer.cpp - clang-tidy ---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ExceptionSpecAnalyzer.h"
+
+#include "clang/AST/Expr.h"
+
+namespace clang::tidy::utils {
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyze(const FunctionDecl *FuncDecl) {
+  ExceptionSpecAnalyzer::State State;
+
+  // Check if the function has already been analyzed and reuse that result.
+  const auto CacheEntry = FunctionCache.find(FuncDecl);
+  if (CacheEntry == FunctionCache.end()) {
+    State = analyzeImpl(FuncDecl);
+
+    // Cache the result of the analysis.
+    FunctionCache.try_emplace(FuncDecl, State);
+  } else
+    State = CacheEntry->getSecond();
+
+  return State;
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeUnresolvedOrDefaulted(
+    const CXXMethodDecl *MethodDecl, const FunctionProtoType *FuncProto) {
+  if (!FuncProto || !MethodDecl)
+    return State::Unknown;
+
+  const DefaultableMemberKind Kind = getDefaultableMemberKind(MethodDecl);
+
+  if (Kind == DefaultableMemberKind::None)
+    return State::Unknown;
+
+  return analyzeRecord(MethodDecl->getParent(), Kind, SkipMethods::Yes);
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeFieldDecl(const FieldDecl *FDecl,
+                                        DefaultableMemberKind Kind) {
+  if (!FDecl)
+    return State::Unknown;
+
+  if (const CXXRecordDecl *RecDecl =
+          FDecl->getType()->getUnqualifiedDesugaredType()->getAsCXXRecordDecl())
+    return analyzeRecord(RecDecl, Kind);
+
+  // Trivial types do not throw
+  if (FDecl->getType().isTrivialType(FDecl->getASTContext()))
+    return State::NotThrowing;
+
+  return State::Unknown;
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeBase(const CXXBaseSpecifier &Base,
+                                   DefaultableMemberKind Kind) {
+  const auto *RecType = Base.getType()->getAs<RecordType>();
+  if (!RecType)
+    return State::Unknown;
+
+  const auto *BaseClass = cast<CXXRecordDecl>(RecType->getDecl());
+
+  return analyzeRecord(BaseClass, Kind);
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeRecord(const CXXRecordDecl *RecordDecl,
+                                     DefaultableMemberKind Kind,
+                                     SkipMethods SkipMethods) {
+  if (!RecordDecl)
+    return State::Unknown;
+
+  // Trivial implies noexcept
+  if (hasTrivialMemberKind(RecordDecl, Kind))
+    return State::NotThrowing;
+
+  if (SkipMethods == SkipMethods::No)
+    for (const auto *MethodDecl : RecordDecl->methods())
+      if (getDefaultableMemberKind(MethodDecl) == Kind)
+        return analyze(MethodDecl);
+
+  for (const auto &BaseSpec : RecordDecl->bases()) {
+    State Result = analyzeBase(BaseSpec, Kind);
+    if (Result == State::Throwing || Result == State::Unknown)
+      return Result;
+  }
+
+  for (const auto &BaseSpec : RecordDecl->vbases()) {
+    State Result = analyzeBase(BaseSpec, Kind);
+    if (Result == State::Throwing || Result == State::Unknown)
+      return Result;
+  }
+
+  for (const auto *FDecl : RecordDecl->fields())
+    if (!FDecl->isInvalidDecl() && !FDecl->isUnnamedBitfield()) {
+      State Result = analyzeFieldDecl(FDecl, Kind);
+      if (Result == State::Throwing || Result == State::Unknown)
+        return Result;
+    }
+
+  return State::NotThrowing;
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeImpl(const FunctionDecl *FuncDecl) {
+  const auto *FuncProto = FuncDecl->getType()->getAs<FunctionProtoType>();
+  if (!FuncProto)
+    return State::Unknown;
+
+  const ExceptionSpecificationType EST = FuncProto->getExceptionSpecType();
+
+  if (EST == EST_Unevaluated || (EST == EST_None && FuncDecl->isDefaulted()))
+    return analyzeUnresolvedOrDefaulted(cast<CXXMethodDecl>(FuncDecl),
+                                        FuncProto);
+
+  return analyzeFunctionEST(FuncDecl, FuncProto);
+}
+
+ExceptionSpecAnalyzer::State
+ExceptionSpecAnalyzer::analyzeFunctionEST(const FunctionDecl *FuncDecl,
+                                          const FunctionProtoType *FuncProto) {
+  if (!FuncDecl || !FuncProto)
+    return State::Unknown;
+
+  if (isUnresolvedExceptionSpec(FuncProto->getExceptionSpecType()))
+    return State::Unknown;
+
+  switch (FuncProto->canThrow()) {
+  case CT_Cannot:
+    return State::NotThrowing;
+  case CT_Dependent: {
+    const Expr *NoexceptExpr = FuncProto->getNoexceptExpr();
+    bool Result;
+    return (NoexceptExpr && !NoexceptExpr->isValueDependent() &&
+            NoexceptExpr->EvaluateAsBooleanCondition(
+                Result, FuncDecl->getASTContext(), true) &&
+            Result)
+               ? State::NotThrowing
+               : State::Throwing;
+  }
+  default:
+    return State::Throwing;
+  };
+}
+
+bool ExceptionSpecAnalyzer::hasTrivialMemberKind(const CXXRecordDecl *RecDecl,
+                                                 DefaultableMemberKind Kind) {
+  if (!RecDecl)
+    return false;
+
+  switch (Kind) {
+  case DefaultableMemberKind::DefaultConstructor:
+    return RecDecl->hasTrivialDefaultConstructor();
+  case DefaultableMemberKind::CopyConstructor:
+    return RecDecl->hasTrivialCopyConstructor();
+  case DefaultableMemberKind::MoveConstructor:
+    return RecDecl->hasTrivialMoveConstructor();
+  case DefaultableMemberKind::CopyAssignment:
+    return RecDecl->hasTrivialCopyAssignment();
+  case DefaultableMemberKind::MoveAssignment:
+    return RecDecl->hasTrivialMoveAssignment();
+  case DefaultableMemberKind::Destructor:
+    return RecDecl->hasTrivialDestructor();
+
+  default:
+    return false;
+  }
+}
+
+bool ExceptionSpecAnalyzer::isConstructor(DefaultableMemberKind Kind) {
+  switch (Kind) {
+  case DefaultableMemberKind::DefaultConstructor:
+  case DefaultableMemberKind::CopyConstructor:
+  case DefaultableMemberKind::MoveConstructor:
+    return true;
+
+  default:
+    return false;
+  }
+}
+
+bool ExceptionSpecAnalyzer::isSpecialMember(DefaultableMemberKind Kind) {
+  switch (Kind) {
+  case DefaultableMemberKind::DefaultConstructor:
+  case DefaultableMemberKind::CopyConstructor:
+  case DefaultableMemberKind::MoveConstructor:
+  case DefaultableMemberKind::CopyAssignment:
+  case DefaultableMemberKind::MoveAssignment:
+  case DefaultableMemberKind::Destructor:
+    return true;
+  default:
+    return false;
+  }
+}
+
+bool ExceptionSpecAnalyzer::isComparison(DefaultableMemberKind Kind) {
+  switch (Kind) {
+  case DefaultableMemberKind::CompareEqual:
+  case DefaultableMemberKind::CompareNotEqual:
+  case DefaultableMemberKind::CompareRelational:
+  case DefaultableMemberKind::CompareThreeWay:
+    return true;
+  default:
+    return false;
+  }
+}
+
+ExceptionSpecAnalyzer::DefaultableMemberKind
+ExceptionSpecAnalyzer::getDefaultableMemberKind(const FunctionDecl *FuncDecl) {
+  if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FuncDecl)) {
+    if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(FuncDecl)) {
+      if (Ctor->isDefaultConstructor())
+        return DefaultableMemberKind::DefaultConstructor;
+
+      if (Ctor->isCopyConstructor())
+        return DefaultableMemberKind::CopyConstructor;
+
+      if (Ctor->isMoveConstructor())
+        return DefaultableMemberKind::MoveConstructor;
+    }
+
+    if (MethodDecl->isCopyAssignmentOperator())
+      return DefaultableMemberKind::CopyAssignment;
+
+    if (MethodDecl->isMoveAssignmentOperator())
+      return DefaultableMemberKind::MoveAssignment;
+
+    if (isa<CXXDestructorDecl>(FuncDecl))
+      return DefaultableMemberKind::Destructor;
+  }
+
+  const LangOptions &LangOpts = FuncDecl->getLangOpts();
+
+  switch (FuncDecl->getDeclName().getCXXOverloadedOperator()) {
+  case OO_EqualEqual:
+    return DefaultableMemberKind::CompareEqual;
+
+  case OO_ExclaimEqual:
+    return DefaultableMemberKind::CompareNotEqual;
+
+  case OO_Spaceship:
+    // No point allowing this if <=> doesn't exist in the current language mode.
+    if (!LangOpts.CPlusPlus20)
+      break;
+    return DefaultableMemberKind::CompareThreeWay;
+
+  case OO_Less:
+  case OO_LessEqual:
+  case OO_Greater:
+  case OO_GreaterEqual:
+    // No point allowing this if <=> doesn't exist in the current language mode.
+    if (!LangOpts.CPlusPlus20)
+      break;
+    return DefaultableMemberKind::CompareRelational;
+
+  default:
+    break;
+  }
+
+  // Not a defaultable member kind
+  return DefaultableMemberKind::None;
+}
+
+} // namespace clang::tidy::utils

diff  --git a/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h b/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h
new file mode 100644
index 0000000000000..9c9782f7a38c6
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h
@@ -0,0 +1,88 @@
+//===--- ExceptionSpecAnalyzer.h - clang-tidy -------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_SPEC_ANALYZER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_SPEC_ANALYZER_H
+
+#include "clang/AST/DeclCXX.h"
+#include "llvm/ADT/DenseMap.h"
+
+namespace clang::tidy::utils {
+
+/// This class analysis if a `FunctionDecl` has been declared implicitly through
+/// defaulting or explicitly as throwing or not and evaluates noexcept
+/// expressions if needed. Unlike the `ExceptionAnalyzer` however it can't tell
+/// you if the function will actually throw an exception or not.
+class ExceptionSpecAnalyzer {
+public:
+  enum class State {
+    Throwing,    ///< This function has been declared as possibly throwing.
+    NotThrowing, ///< This function has been declared as not throwing.
+    Unknown, ///< We're unable to tell if this function is declared as throwing
+             ///< or not.
+  };
+
+  ExceptionSpecAnalyzer() = default;
+
+  State analyze(const FunctionDecl *FuncDecl);
+
+private:
+  enum class DefaultableMemberKind {
+    DefaultConstructor,
+    CopyConstructor,
+    MoveConstructor,
+    CopyAssignment,
+    MoveAssignment,
+    Destructor,
+
+    CompareEqual,
+    CompareNotEqual,
+    CompareThreeWay,
+    CompareRelational,
+
+    None,
+  };
+
+  State analyzeImpl(const FunctionDecl *FuncDecl);
+
+  State analyzeUnresolvedOrDefaulted(const CXXMethodDecl *MethodDecl,
+                                     const FunctionProtoType *FuncProto);
+
+  State analyzeFieldDecl(const FieldDecl *FDecl, DefaultableMemberKind Kind);
+
+  State analyzeBase(const CXXBaseSpecifier &Base, DefaultableMemberKind Kind);
+
+  enum class SkipMethods : bool {
+    Yes = true,
+    No = false,
+  };
+
+  State analyzeRecord(const CXXRecordDecl *RecDecl, DefaultableMemberKind Kind,
+                      SkipMethods SkipMethods = SkipMethods::No);
+
+  static State analyzeFunctionEST(const FunctionDecl *FuncDecl,
+                                  const FunctionProtoType *FuncProto);
+
+  static bool hasTrivialMemberKind(const CXXRecordDecl *RecDecl,
+                                   DefaultableMemberKind Kind);
+
+  static bool isConstructor(DefaultableMemberKind Kind);
+
+  static bool isSpecialMember(DefaultableMemberKind Kind);
+
+  static bool isComparison(DefaultableMemberKind Kind);
+
+  static DefaultableMemberKind
+  getDefaultableMemberKind(const FunctionDecl *FuncDecl);
+
+  llvm::DenseMap<const FunctionDecl *, State> FunctionCache{32u};
+};
+
+} // namespace clang::tidy::utils
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_SPEC_ANALYZER_H

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7eb65d46b9cc1..4563aa8de731c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -303,6 +303,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/use-after-move>` to understand that there is a
   sequence point between designated initializers.
 
+- Fixed an issue in the :doc:`performance-noexcept-move-constructor
+  <clang-tidy/checks/performance/noexcept-move-constructor>` checker that was causing
+  false-positives when the move constructor or move assign operator were defaulted.
+
 - Fixed an issue in :doc:`readability-identifier-naming
   <clang-tidy/checks/readability/identifier-naming>` when specifying an empty
   string for ``Prefix`` or ``Suffix`` options could result in the style not

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
index 6b6e6c2cb025f..c826abfafbaf2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t
+// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t -- -- -fexceptions
 
 struct C_1 {
  ~C_1() {}
@@ -31,9 +31,7 @@ struct C_3 {
 };
 
 C_3::C_3(C_3&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
 C_3& C_3::operator=(C_3&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
 
 template <class T>
 struct C_4 {
@@ -41,7 +39,6 @@ struct C_4 {
 // CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
  ~C_4() {}
  C_4& operator=(C_4&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
 };
 
 template <class T>
@@ -50,7 +47,6 @@ struct C_5 {
 // CHECK-FIXES:){{.*}}noexcept{{.*}} {}
  ~C_5() {}
  auto operator=(C_5&& a)->C_5<T> = default;
-// CHECK-FIXES:){{.*}}noexcept{{.*}} = default;
 };
 
 template <class T>
@@ -64,4 +60,4 @@ struct C_6 {
 
 template <class T>
 auto C_6<T>::operator=(C_6<T>&& a) -> C_6<T> {}
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
\ No newline at end of file
+// CHECK-FIXES: ){{.*}}noexcept{{.*}} {}

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
index 70d70650c1d13..1ccdd0a2a0aae 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp
@@ -1,10 +1,37 @@
-// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t
+// RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t -- -- -fexceptions
+
+struct Empty
+{};
+
+struct IntWrapper {
+  int value;
+};
+
+template <typename T>
+struct FalseT {
+  static constexpr bool value = false;
+};
+
+template <typename T>
+struct TrueT {
+  static constexpr bool value = true;
+};
+
+struct ThrowingMoveConstructor
+{
+  ThrowingMoveConstructor() = default;
+  ThrowingMoveConstructor(ThrowingMoveConstructor&&) noexcept(false) {
+  }
+  ThrowingMoveConstructor& operator=(ThrowingMoveConstructor &&) noexcept(false) {
+    return *this;
+  }
+};
 
 class A {
   A(A &&);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
   A &operator=(A &&);
-  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
 };
 
 struct B {
@@ -13,6 +40,125 @@ struct B {
   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
 };
 
+template <typename>
+struct C
+{
+  C(C &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+  C& operator=(C &&);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+};
+
+struct D
+{
+  static constexpr bool kFalse = false;
+  D(D &&) noexcept(kFalse) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+  D& operator=(D &&) noexcept(kFalse) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template <typename>
+struct E
+{
+  static constexpr bool kFalse = false;
+  E(E &&) noexcept(kFalse);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+  E& operator=(E &&) noexcept(kFalse);
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template <typename>
+struct F
+{
+  static constexpr bool kFalse = false;
+  F(F &&) noexcept(kFalse) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+  F& operator=(F &&) noexcept(kFalse) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+struct G {
+  G(G &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+  G& operator=(G &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+
+  ThrowingMoveConstructor field;
+};
+
+void throwing_function() noexcept(false) {}
+
+struct H {
+  H(H &&) noexcept(noexcept(throwing_function()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+  H &operator=(H &&) noexcept(noexcept(throwing_function()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template <typename>
+struct I {
+  I(I &&) noexcept(noexcept(throwing_function()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+  I &operator=(I &&) noexcept(noexcept(throwing_function()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false'
+};
+
+template <typename TemplateType> struct TemplatedType {
+  static void f() {}
+};
+
+template <> struct TemplatedType<int> {
+  static void f() noexcept {}
+};
+
+struct J {
+  J(J &&) noexcept(noexcept(TemplatedType<double>::f()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: noexcept specifier on the move constructor evaluates to 'false' [performance-noexcept-move-constructor]
+  J &operator=(J &&) noexcept(noexcept(TemplatedType<double>::f()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: noexcept specifier on the move assignment operator evaluates to 'false' [performance-noexcept-move-constructor]
+};
+
+struct K : public ThrowingMoveConstructor {
+  K(K &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+  K &operator=(K &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+};
+
+struct InheritFromThrowingMoveConstrcutor : public ThrowingMoveConstructor
+{};
+
+struct L {
+  L(L &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+  L &operator=(L &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+
+  InheritFromThrowingMoveConstrcutor IFF;
+};
+
+struct M : public InheritFromThrowingMoveConstrcutor {
+  M(M &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+  M &operator=(M &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+};
+
+struct N : public IntWrapper, ThrowingMoveConstructor {
+  N(N &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+  N &operator=(N &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+};
+
+struct O : virtual IntWrapper, ThrowingMoveConstructor {
+  O(O &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: move constructors should be marked noexcept [performance-noexcept-move-constructor]
+  O &operator=(O &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: move assignment operators should be marked noexcept [performance-noexcept-move-constructor]
+};
+
 class OK {};
 
 void f() {
@@ -30,10 +176,9 @@ class OK1 {
   void g() noexcept;
 };
 
-class OK2 {
+struct OK2 {
   static constexpr bool kTrue = true;
 
-public:
   OK2(OK2 &&) noexcept(true) {}
   OK2 &operator=(OK2 &&) noexcept(kTrue) { return *this; }
 };
@@ -52,3 +197,182 @@ struct OK5 {
   OK5(OK5 &&) noexcept(true) = default;
   OK5 &operator=(OK5 &&) noexcept(true) = default;
 };
+
+struct OK6 {
+  OK6(OK6 &&) = default;
+  OK6& operator=(OK6 &&) = default;
+};
+
+template <typename>
+struct OK7 {
+  OK7(OK7 &&) = default;
+  OK7& operator=(OK7 &&) = default;
+};
+
+template <typename>
+struct OK8 {
+  OK8(OK8 &&) noexcept = default;
+  OK8& operator=(OK8 &&) noexcept = default;
+};
+
+template <typename>
+struct OK9 {
+  OK9(OK9 &&) noexcept(true) = default;
+  OK9& operator=(OK9 &&) noexcept(true) = default;
+};
+
+template <typename>
+struct OK10 {
+  OK10(OK10 &&) noexcept(false) = default;
+  OK10& operator=(OK10 &&) noexcept(false) = default;
+};
+
+template <typename>
+struct OK11 {
+  OK11(OK11 &&) = delete;
+  OK11& operator=(OK11 &&) = delete;
+};
+
+void noexcept_function() noexcept {}
+
+struct OK12 {
+  OK12(OK12 &&) noexcept(noexcept(noexcept_function()));
+  OK12 &operator=(OK12 &&) noexcept(noexcept(noexcept_function));
+};
+
+struct OK13 {
+  OK13(OK13 &&) noexcept(noexcept(noexcept_function)) = default;
+  OK13 &operator=(OK13 &&) noexcept(noexcept(noexcept_function)) = default;
+};
+
+template <typename> struct OK14 {
+  OK14(OK14 &&) noexcept(noexcept(TemplatedType<int>::f()));
+  OK14 &operator=(OK14 &&) noexcept(noexcept(TemplatedType<int>::f()));
+};
+
+struct OK15 {
+  OK15(OK15 &&) = default;
+  OK15 &operator=(OK15 &&) = default;
+
+  int member;
+};
+
+template <typename>
+struct OK16 {
+  OK16(OK16 &&) = default;
+  OK16 &operator=(OK16 &&) = default;
+
+  int member;
+};
+
+struct OK17
+{
+  OK17(OK17 &&) = default;
+  OK17 &operator=(OK17 &&) = default;
+  OK empty_field;
+};
+
+template <typename>
+struct OK18
+{
+  OK18(OK18 &&) = default;
+  OK18 &operator=(OK18 &&) = default;
+
+  OK empty_field;
+};
+
+struct OK19 : public OK
+{
+  OK19(OK19 &&) = default;
+  OK19 &operator=(OK19 &&) = default;
+};
+
+struct OK20 : virtual OK
+{
+  OK20(OK20 &&) = default;
+  OK20 &operator=(OK20 &&) = default;
+};
+
+template <typename T>
+struct OK21 : public T
+{
+  OK21() = default;
+  OK21(OK21 &&) = default;
+  OK21 &operator=(OK21 &&) = default;
+};
+
+template <typename T>
+struct OK22 : virtual T
+{
+  OK22() = default;
+  OK22(OK22 &&) = default;
+  OK22 &operator=(OK22 &&) = default;
+};
+
+template <typename T>
+struct OK23 {
+  OK23()= default;
+  OK23(OK23 &&) = default;
+  OK23 &operator=(OK23 &&) = default;
+
+  T member;
+};
+
+void testTemplates() {
+  OK21<Empty> value(OK21<Empty>{});
+  value = OK21<Empty>{};
+
+  OK22<Empty> value2{OK22<Empty>{}};
+  value2 = OK22<Empty>{};
+
+  OK23<Empty> value3{OK23<Empty>{}};
+  value3 =OK23<Empty>{};
+}
+
+struct OK24 : public Empty, OK1 {
+  OK24(OK24 &&) = default;
+  OK24 &operator=(OK24 &&) = default;
+};
+
+struct OK25 : virtual Empty, OK1 {
+  OK25(OK25 &&) = default;
+  OK25 &operator=(OK25 &&) = default;
+};
+
+struct OK26 : public Empty, IntWrapper {
+  OK26(OK26 &&) = default;
+  OK26 &operator=(OK26 &&) = default;
+};
+
+template <typename T>
+struct OK27 : public T
+{
+  OK27(OK27 &&) = default;
+  OK27 &operator=(OK27 &&) = default;
+};
+
+template <typename T>
+struct OK28 : virtual T
+{
+  OK28(OK28 &&) = default;
+  OK28 &operator=(OK28 &&) = default;
+};
+
+template <typename T>
+struct OK29 {
+  OK29(OK29 &&) = default;
+  OK29 &operator=(OK29 &&) = default;
+
+  T member;
+};
+
+struct OK30 {
+  OK30(OK30 &&) noexcept(TrueT<OK30>::value) = default;
+  OK30& operator=(OK30 &&) noexcept(TrueT<OK30>::value) = default;
+};
+
+template <typename>
+struct OK31 {
+  OK31(OK31 &&) noexcept(TrueT<int>::value) = default;
+  OK31& operator=(OK31 &&) noexcept(TrueT<int>::value) = default;
+};


        


More information about the cfe-commits mailing list