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

[AMS21]

Summary

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

[PiotrZSL]

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

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

156

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)`

[PiotrZSL]

Update ReleaseNotes for this check, and validate documentation.

[Eugene.Zelenko]

clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp

64

Please add.

[AMS21]

@PiotrZSL thanks for the quick response.

I've tried to work on the Field test case but seem to have hit a roadblock. From what I can see the FunctionProtoType exception specificaion is EST_Unevaluated which causes it to give up because of the check on Line 41.
I'm not very familiar with the codebase but looked around and found the ResolveExceptionSpec() function inside Sema.h which seems to do exactly what I need: tell me if the implicit definition is throwing or not.
But I can't quite seem to figure out how to get a Sema object or construct one to use said function.
Or maybe there is another more clever way to go about it which I'm not aware of?

Any pointers into the right direction would be kindly appreciated.

[PiotrZSL]

Don't waste time of Sema, no way to access it, as far as I know, it's already destroyed when checks are executed.

[AMS21]

Implemented suggested fixes from reviews

[PiotrZSL]

Try to move some code to separate file, some utilty/...
Add tests with tyepdefs.
Add tests with class derive from type that it gets from template.

Probably this code will be used in few more checks in future.
try implement some virtual base validation,
add some cache so we could avoid validating same class/method again.

Maybe change this into separate class.
Anyway good work..

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

21

Constrcutor -> Constructor

27

consider moving this all from this file to some utils/something, and don't restrict this only to move operator/constructor but also support destructor/other constructors and so on.
Simply if is not default, ten use evaluateFunctionEST, if is default, then try analyse as you doing now.

31

i think that you may still need to use isUnresolvedExceptionSpec here, and return Unknown if that is true

61

getType()->getUnqualifiedDesugaredType()->getAsCXXRecordDecl();

81

but we shoudn't, we should test now fields of that record.... because it may not have move constructor... (in theory). Probably we should also check if it has constructor, but is's implicit one, then we should also check fields and bases of this class instead of trying to check constructor.

95

isInvalidDecl looks redudant.

98–100

this looks redudant..

131

use isMoveConstructor() here, instead of using Ctor->isMoveConstructor() later.

[PiotrZSL]

Try refactoring code a lite bit, check suggestions, add tests for typedefs, and inheritance from template type.

[AMS21]

Move functionality to a seperate file
Also the noexcept move constrcutor check now only runs with exceptions enabled like other checks.

[AMS21]

In D146922#4227832, @PiotrZSL wrote:

Try refactoring code a lite bit, check suggestions, add tests for typedefs, and inheritance from template type.

I'm not quite sure what you mean by tests for typedefs. Maybe you could give me an example?

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp

83 ↗

(On Diff #509405)

Should be const auto *.

87 ↗

(On Diff #509405)

Ditto.

[PiotrZSL]

In D146922#4231317, @AMS21 wrote:

In D146922#4227832, @PiotrZSL wrote:

Try refactoring code a lite bit, check suggestions, add tests for typedefs, and inheritance from template type.

I'm not quite sure what you mean by tests for typedefs. Maybe you could give me an example?

struct SomeStruct { ... };
using TSomeStruct = SomeStruct;

struct OK {
  OK(OK &&) = default;
  OK &operator=(OK&&) = default;

  TSomeStruct member;
};

struct OK : TSomeStruct {
  OK(OK &&) = default;
  OK &operator=(OK&&) = default;
};

Anyway, looks promising... I still need to check this more deeply.

clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp

241

for those 2 templates do test with and without instantiation

// like this:
void testTemplates() {
  OK21<SomeStruct> value(OK21<SomeStruct>());
  value = OK21<SomeStruct>();
}

,
add test with template as member.
like:

template <typename T>
struct OK22 
{
  OK22(OK22 &&) = default;
  OK22 &operator=(OK22 &&) = default;

  T member;
};

[PiotrZSL]

clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp

252

add test with 2 base classes (no templates).

[AMS21]

Sprinkle some more const

[PiotrZSL]

clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h

79 ↗

(On Diff #509405)

std::unordered_map, or try to check if llvm doesn't have something better with some pre-allocation.

[AMS21]

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

21

Well thats embarassing

[AMS21]

Use llvm::DenseMap instead of std::map
Add some more test cases

[PiotrZSL]

Looks ok, would be good to run this on some code base.

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

132

use isMoveAssignmentOperator instead of hasOverloadedOperatorName

132

move those 2 to begining...

139

invert this if to reduce nesting.

const auto *Decl = Result.Nodes.getNodeAs<CXXMethodDecl>("decl")
if (!Decl)
    return;

141–147

IsConstructor = CXXConstructorDecl::classof(Decl);

154

remove {}, leave just

if (SpecAnalyzer.analyze(Decl) !=     utils::ExceptionSpecAnalyzer::State::Throwing) return;

156–162

woudn't getExceptionSpecInfo() != EST_NoexceptFalse do a trick here ?

clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp

32 ↗

(On Diff #509417)

use FunctionCache.find to avoid double search in line 32 and 38 (and reuse result)

36 ↗

(On Diff #509417)

try_emplace ?

44 ↗

(On Diff #509417)

pass FunctionProtoType also to this function

50–52 ↗

(On Diff #509417)

technically you could change this class to operator on CXXMethodDecl since begining

76 ↗

(On Diff #509417)

check if this is trivial type, if its trivial type then cannot throw...
FDecl->getType()->isTrivialType(FDecl->getAstContext());

95 ↗

(On Diff #509417)

Can we hit some endless recursion here ?
Maybe so protection against checking Record that we currently checking.

108–114 ↗

(On Diff #509417)

I'm not sure if we need to be so picky...
In short we could check all bases.
Virtual, Abstract or not...

155 ↗

(On Diff #509417)

pass FunctionProtoType also to this function, to avoid double checking

clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h

23 ↗

(On Diff #509417)

No need for std::int8_t, and if you really want then use std::uint8_t

78 ↗

(On Diff #509417)

this also can be static

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

260 ↗

(On Diff #509417)

Please keep alphabetical order (by check name) in this section.

[AMS21]

Implement review suggestions

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

156–162

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.

clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.cpp

95 ↗

(On Diff #509417)

Yes we can hit an infinite recursion here.
Take this class:

struct A {
    A(A &&) = default;
};

Since the move constructor is defaulted, we need to call analyzeUnresolvedOrDefaulted. There we determine the kind of defaulted member function which in this case is a move constructor.
Then we call analyzeRecord. Without setting SkipMethods::Yes we would try to check the move constructor of the class which in this case is A. We would call analyze on the move constructor of A which is exactly where we started and we'd have an infinite loop.

That is why I've added the SkipMethods parameter. While analyzing the defaulted move constructor of A we only want to look at its base classes and it's fields and determine if they are declared as throwing or not. While for their the base classes and fields of A we also want to check their move constructor (if they have any).

I hope my explanation was at least a bit helpful.

There might be a better solution to this, Before this I had essentially the same code twice for checking the bases and field and wanted to combine them.

95 ↗

(On Diff #509417)

Another way to defiantly have an infinite recursion is if to resolve a struct A; we needed to resolve struct B; and to resolve that we needed to resolve struct A. You would need something like A inheriting from B and B inheriting from A or having the other type as a member variable. Same goes for a struct which contains itself.

But I'm currently unaware on how to create such a scenario with legal C++.

108–114 ↗

(On Diff #509417)

Honestly I not sure about it. I just tried to copy what Sema does when resolving noexcept.
Removing it definitely makes the code easier to read.

clang-tools-extra/clang-tidy/utils/ExceptionSpecAnalyzer.h

23 ↗

(On Diff #509417)

I prefer to not use it, but saw ExceptionAnalyzer use it. So I just copied it from there.

[PiotrZSL]

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

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

156–162

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

clang-tools-extra/docs/ReleaseNotes.rst

261–262 ↗

(On Diff #509695)

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.

[AMS21]

clang-tools-extra/docs/ReleaseNotes.rst

260 ↗

(On Diff #509417)

Sure but I'm a bit confused. I assuming the check name is without the prefix so in this case noexcept-move-constructor. So it should be after readability-avoid-underscore-in-googletest-name. But why then is use-after-move sorted before identifier-naming.
I'm probably missing something very obvious.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

260 ↗

(On Diff #509417)

Full check name (i.e. performance-noexcept-move-constructor) should be accounted.

[AMS21]

Fixed docs and cleanup tests a bit

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

156–162

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.

[PiotrZSL]

Any plans in delivering this ?

[AMS21]

I'm fine with pushing this change, but since I don't have commit access someone would need to commit this for me

[PiotrZSL]

I'm pushing this as it looks fine, and I don't see any reason to block this.
@carlosgalvezp Fell free to look into this in some free time if you see any bugs/issues.

clang-tools-extra/clang-tidy/performance/NoexceptMoveConstructorCheck.cpp

 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Tooling/FixIt.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::performance {
 
+namespace {
+
+enum CanMoveConstrcutorThrowResult {
+  CMCT_CanThrow,    ///< We are sure the move constructor can throw
+  CMCT_CannotThrow, ///< We are sure the move constrcutor cannot throw
+  CMCT_Unknown,     ///< We don't know if the move constrcutor can throw or not
+};
+
+CanMoveConstrcutorThrowResult evaluateFunctionEST(const FunctionDecl *Func) {
+  const auto *FunProto = Func->getType()->getAs<FunctionProtoType>();
+  if (!FunProto)
+    return CMCT_Unknown;
+
+  switch (FunProto->canThrow()) {
+  case CT_Cannot:
+    return CMCT_CannotThrow;
+  case CT_Dependent: {
+    const Expr *NoexceptExpr = FunProto->getNoexceptExpr();
+    bool Result;
+    return (NoexceptExpr && !NoexceptExpr->isValueDependent() &&
+            NoexceptExpr->EvaluateAsBooleanCondition(
+                Result, Func->getASTContext(), true) &&
+            Result)
+               ? CMCT_CannotThrow
+               : CMCT_CanThrow;
+  }
+  default:
+    return CMCT_CanThrow;
+  };
+}
+
+CanMoveConstrcutorThrowResult
+canMoveConstructorThrow(const FunctionDecl *FuncDecl);
+
+CanMoveConstrcutorThrowResult
+canFieldMoveConstructorThrow(const FieldDecl *FDecl, bool TestConstructor) {
+  if (!FDecl)
+    return CMCT_Unknown;
+
+  const ASTContext &Context = FDecl->getASTContext();
+
+  if (auto *FieldRecordType =
+          Context.getBaseElementType(FDecl->getType())->getAs<RecordType>()) {
+    if (const auto *FieldRecordDecl =
+            cast<CXXRecordDecl>(FieldRecordType->getDecl())) {
+
+      // Trivial implies noexcept
+      if ((FieldRecordDecl->hasTrivialMoveConstructor() && TestConstructor) ||
+          (FieldRecordDecl->hasTrivialMoveAssignment() && !TestConstructor))
+        return CMCT_CannotThrow;
+
+      for (const auto *Method : FieldRecordDecl->methods()) {
+        const auto *Ctor = dyn_cast<CXXConstructorDecl>(Method);
+
+        if ((TestConstructor && Ctor && Ctor->isMoveConstructor()) ||
+            (Method->isMoveAssignmentOperator() && !TestConstructor))
+          return canMoveConstructorThrow(Method);
+      }
+    }
+  }
+
+  // We assume non record types cannot throw
+  return CMCT_CannotThrow;
+}
+
+CanMoveConstrcutorThrowResult
+resolveUnevaluatedOrDefaulted(const FunctionDecl *FuncDecl) {
+  const auto *FunProto = FuncDecl->getType()->getAs<FunctionProtoType>();
+  if (!FunProto)
+    return CMCT_Unknown;
+
+  const CXXMethodDecl *MethodDecl = cast<CXXMethodDecl>(FuncDecl);
+  if (!MethodDecl || MethodDecl->isInvalidDecl())
+    return CMCT_Unknown;
+
+  const CXXRecordDecl *RecordDecl = MethodDecl->getParent();
+  if (!RecordDecl || RecordDecl->isInvalidDecl())
+    return CMCT_Unknown;
+
+  const ASTContext &Context = FuncDecl->getASTContext();
+  if (Context.getRecordType(RecordDecl).isNull())
+    return CMCT_Unknown;
+
+  const bool IsConstructor = !MethodDecl->isMoveAssignmentOperator();
+
+  // FIXME: We currectly ignore virtual and non virtual bases
+
+  for (const auto *FDecl : RecordDecl->fields())
+    if (!FDecl->isInvalidDecl() && !FDecl->isUnnamedBitfield())
+      if (canFieldMoveConstructorThrow(FDecl, IsConstructor) == CMCT_CanThrow)
+        return CMCT_CanThrow;
+
+  return CMCT_CannotThrow;
+}
+
+CanMoveConstrcutorThrowResult
+canMoveConstructorThrow(const FunctionDecl *FuncDecl) {
+  const auto *FunProto = FuncDecl->getType()->getAs<FunctionProtoType>();
+  if (!FunProto)
+    return CMCT_Unknown;
+
+  const ExceptionSpecificationType EST = FunProto->getExceptionSpecType();
+
+  if (EST == EST_Unevaluated || (EST == EST_None && FuncDecl->isDefaulted()))
+    return resolveUnevaluatedOrDefaulted(FuncDecl);
+
+  return evaluateFunctionEST(FuncDecl);
+}
+} // namespace
+
 void NoexceptMoveConstructorCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       cxxMethodDecl(anyOf(cxxConstructorDecl(), hasOverloadedOperatorName("=")),
                     unless(isImplicit()), unless(isDeleted()))
           .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 CanMoveConstrcutorThrowResult result = canMoveConstructorThrow(Decl);
 
-    if (isUnresolvedExceptionSpec(ProtoType->getExceptionSpecType()))
+    if (result != CMCT_CanThrow)
+      return;
+
+    // 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;
+    }
 
-    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;
-    }
-
-    // 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;
-      }
-    }
-  }
+    SourceLocation NoexceptLoc =
+        Decl->getParamDecl(Decl->getNumParams() - 1)->getSourceRange().getEnd();
 }
 
 } // namespace clang::tidy::performance

clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor-fix.cpp

  ~C_3() {}
  C_3(C_3&& a);
 // CHECK-FIXES: ){{.*}}noexcept{{.*}};
  C_3& operator=(C_3&& a);
 // CHECK-FIXES: ){{.*}}noexcept{{.*}};
 };
 
 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 {
  C_4(C_4<T>&&) {}
 // CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
  ~C_4() {}
  C_4& operator=(C_4&& a) = default;
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} = default;
 };
 
 template <class T>
 struct C_5 {
  C_5(C_5<T>&&) {}
 // CHECK-FIXES:){{.*}}noexcept{{.*}} {}
  ~C_5() {}
  auto operator=(C_5&& a)->C_5<T> = default;
-// CHECK-FIXES:){{.*}}noexcept{{.*}} = default;
 };
 
 template <class T>
 struct C_6 {
  C_6(C_6<T>&&) {}
 // CHECK-FIXES:){{.*}}noexcept{{.*}} {}
  ~C_6() {}
  auto operator=(C_6&& a)->C_6<T>;
 // CHECK-FIXES:){{.*}}noexcept{{.*}};
 };
 
 template <class T>
 auto C_6<T>::operator=(C_6<T>&& a) -> C_6<T> {}
-// CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
+// CHECK-FIXES: ){{.*}}noexcept{{.*}} {}
 

clang-tools-extra/test/clang-tidy/checkers/performance/noexcept-move-constructor.cpp

 // RUN: %check_clang_tidy %s performance-noexcept-move-constructor %t
 
 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 {
   static constexpr bool kFalse = false;
   B(B &&) noexcept(kFalse);
   // 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 Field
+{
+  Field() = default;
+  Field(Field&&) noexcept(false) {
+  }
+  Field& operator=(Field &&) noexcept(false) {
+    return *this;
+  }
+};
+
+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]
+
+  Field 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]
+};
+
 class OK {};
 
 void f() {
   OK a;
   a = OK();
 }
 
 class OK1 {
struct OK4 {
   OK4(OK4 &&) noexcept = default;
   OK4 &operator=(OK4 &&) noexcept = default;
 };
 
 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 nothrowing_function() noexcept {}
+
+struct OK12 {
+  OK12(OK12 &&) noexcept(noexcept(nothrowing_function()));
+  OK12 &operator=(OK12 &&) noexcept(noexcept(nothrowing_function));
+};
+
+struct OK13 {
+  OK13(OK13 &&) noexcept(noexcept(nothrowing_function)) = default;
+  OK13 &operator=(OK13 &&) noexcept(noexcept(nothrowing_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;
+};