[clang-tidy] Add check 'misc-use-after-move'

[mboehme]

Summary

The check warns if an object is used after it has been moved, without an
intervening reinitialization.

See user-facing documentation for details.

[mboehme]

Apologies for the size of this patch. alexfh@ said he was willing to review it as-is -- however, I'm happy to resubmit in smaller pieces if necessary.

[fowles]

test/clang-tidy/misc-use-after-move.cpp

158 ↗

(On Diff #67524)

would this warn on:

std::unique_ptr<A> ptr;
std::move(ptr);
ptr->Foo();

I would like it to since that is a likely segfault.

279 ↗

(On Diff #67524)

can you add tests with reference capture?

also what about:

A a;
auto lambda = [&]() { a.foo(); };
std::move(a);
lambda();

[Prazek]

I will review it later, but my first thoughts:

1. I think we should make some other group, because misc seems to be overloaded. I discussed it with Alex months ago - something like bugprone would be good.
2. Also it would be good to make link in cppcoreguidelines.

[Eugene.Zelenko]

docs/ReleaseNotes.rst

75 ↗

(On Diff #67524)

Please remove This check and capitalize warns

docs/clang-tidy/checks/misc-use-after-move.rst

15 ↗

(On Diff #67524)

str is not language construct, please use `.

51 ↗

(On Diff #67524)

i is not language construct, please use `.

78 ↗

(On Diff #67524)

Please highlight std::unique_ptr and std::shared_ptr with ``

181 ↗

(On Diff #67524)

Please insert empty line and indent code as in previous examples.

190 ↗

(On Diff #67524)

s, s.str and S are not language construct. Please use `.

[Eugene.Zelenko]

clang-tidy/misc/UseAfterMoveCheck.cpp

512 ↗

(On Diff #67524)

Please insert empty line before.

[alexfh]

A few initial comments.

clang-tidy/misc/UseAfterMoveCheck.cpp

492 ↗

(On Diff #67524)

Any reason to avoid range-based for loops here?

505 ↗

(On Diff #67524)

The type is specified verbatim in the initializer, so it's better to use const auto * here.

506 ↗

(On Diff #67524)

It's uncommon to use braces for single-line if/for/... bodies in LLVM/Clang code.

docs/clang-tidy/checks/misc-use-after-move.rst

51 ↗

(On Diff #67524)

It's an inline code snippet, isn't it? I'd format it as code (double backquotes).

190 ↗

(On Diff #67524)

Seems fine to format these as code as well.

[mboehme]

Responses to reviewer comments.

[mboehme]

clang-tidy/misc/UseAfterMoveCheck.cpp

493 ↗

(On Diff #68147)

For some reason, I thought I had read that they weren't compatible with CFG / CFGBlock -- but obviously I must have been imagining things. ;)

Also changed the other occurrences.

docs/clang-tidy/checks/misc-use-after-move.rst

16 ↗

(On Diff #68147)

There doesn't seem to be a clear preference here (see alexfh's comments on similar cases), so I'll leave this open until it's resolved one way or the other.

182 ↗

(On Diff #68147)

Sorry -- forgot to check that the documentation compiles (it does now).

test/clang-tidy/misc-use-after-move.cpp

159 ↗

(On Diff #68147)

No, it doesn't. At the moment, the check intentionally disregards all uses of unique_ptr and shared_ptr (see also the documentation).

I agree that it definitely makes sense to check for scenarios like the one you mention. They're a bit of a different beast though because unique_ptr and shared_ptr have a well-defined state after they've been moved from. This means they would require some special logic -- we'd want to disallow ptr->Foo() after a std::move, but not ptr.get(). For this reason, I've left them out of this initial version.

Also, I'm not sure whether this check is the best place for these unique_ptr and shared_ptr checks to live. Because the after-move state of unique_ptr and shared_ptr is well defined, the "use, then dereference" case is really just a subset of what could be a more general "dereference null pointer" check.

280 ↗

(On Diff #68147)

can you add tests with reference capture?

Done.

also what about: [snip]

The check won't warn about this. More generally, it doesn't do any kind of inter-procedural analysis.

Inter-procedural analysis would certainly help in a number of ways. For example, it could be used to answer the following:

• If a function takes a non-const reference to an object, does it reinitialize that object? (Currently, we optimistically assume that it always does.)
• If a function (that isn't a move constructor or move assignment operator) takes an rvalue reference to an object, does it actually move from that object, and does it do so unconditionally?

However, this would take significant additional implementation effort and would also run more slowly.

[mboehme]

In D23353#511362, @Prazek wrote:

I will review it later, but my first thoughts:

1. I think we should make some other group, because misc seems to be overloaded. I discussed it with Alex months ago - something like bugprone would be good.

Agree that "misc" seems pretty overcrowded. I'll defer to those who have been working on clang-tidy longer than me to make this call.

2. Also it would be good to make link in cppcoreguidelines.

How exactly would I create such a "link"? Are you just thinking of a link in the documentation, or is there a way to have one clang-tidy check activate another (and is this what you're thinking of)?

[omtcyfz]

clang-tidy/misc/UseAfterMoveCheck.cpp

659 ↗

(On Diff #68147)

Nit: As I discussed with Alex, it is better to omit {} in conditional statements if the body only contains one statement. Even if it wasn't so, it'd be better to use one "style" (i.e. either always omit {} or always have {}) at least inside a single check and you have no {} in many places inside this file (L637, L647, L577, ...).

Just a stylistic thing, doesn't matter too much, though.

[alexfh]

clang-tidy/misc/UseAfterMoveCheck.cpp

659 ↗

(On Diff #68147)

I always say "single-line", not "single statement", which is a bit different ;)

[mboehme]

Remove braces around single-line bodies of if statements

[mboehme]

Remove braces around another single-line if statement block

[mboehme]

clang-tidy/misc/UseAfterMoveCheck.cpp

659 ↗

(On Diff #68147)

Agree -- I'm just used to putting the braces in there. ;)

I went through and cleaned up a bunch of these -- hope I found them all.

[Prazek]

In D23353#516314, @mboehme wrote:

In D23353#511362, @Prazek wrote:

I will review it later, but my first thoughts:

1. I think we should make some other group, because misc seems to be overloaded. I discussed it with Alex months ago - something like bugprone would be good.

Agree that "misc" seems pretty overcrowded. I'll defer to those who have been working on clang-tidy longer than me to make this call.

2. Also it would be good to make link in cppcoreguidelines.

How exactly would I create such a "link"? Are you just thinking of a link in the documentation, or is there a way to have one clang-tidy check activate another (and is this what you're thinking of)?

I am not sure if there is any other mechanism than just links in documentation.
In the perfect word it would be nice to invoke this check using cppcoreguidelines-use-after-move also with some special options like Pedantic=1 (That would warn about any use after move, even after reinitialization - this is what cppcoreguidelines says)

clang-tidy/misc/UseAfterMoveCheck.cpp

134 ↗

(On Diff #67524)

What do you think about moving this, and maybe other things to some different header file to make it not so scary?

649–652 ↗

(On Diff #67524)

you can replace it with

else
  return;

test/clang-tidy/misc-use-after-move.cpp

504–505 ↗

(On Diff #67524)

I would like to mark it as use after move with some pedantic flag

[alexfh]

clang-tidy/misc/UseAfterMoveCheck.cpp

29 ↗

(On Diff #68366)

Please enclose inline code snippets in double backquotes (doxygen supports markdown to a certain degree).

46 ↗

(On Diff #68366)

Does it look feasible?

135 ↗

(On Diff #68366)

It's definitely subjective, but I don't think it's scary. And the size of the file doesn't seem to be an issue yet. IMO, a reason to move this to a header would appear once this class is needed elsewhere.

216 ↗

(On Diff #68366)

nit: How about for (const auto &SyntheticStmt : llvm::make_range(TheCFG->synthetic_stmt_begin(), TheCFG->synthetic_stmt_end())) ...?

Alternatively, we could add CFG::synthetic_stmt() method that would return an iterator_range<...>.

435 ↗

(On Diff #68366)

Use llvm::make_range and a range-for loop?

465 ↗

(On Diff #68366)

nit: Variable names should start with a capital letter.

[mboehme]

Responses to reviewer comments.

[mboehme]

2. Also it would be good to make link in cppcoreguidelines.

How exactly would I create such a "link"? Are you just thinking of a link in the documentation, or is there a way to have one clang-tidy check activate another (and is this what you're thinking of)?

I am not sure if there is any other mechanism than just links in documentation.

I've taken a look, but I'm not sure where I would put this link. I could add another file that starts with "cppcoreguidelines-", but that would make it look as if an actual check with that name exists, and I'm not sure that's what we want. Do you have a suggestion?

In the perfect word it would be nice to invoke this check using cppcoreguidelines-use-after-move also with some special options like Pedantic=1 (That would warn about any use after move, even after reinitialization - this is what cppcoreguidelines says)

Do you have a pointer to something in CppCoreGuidelines that says this explicitly?

The closest I've been able to find is in F.18: "Flag access to moved-from objects." It's not entirely clear here whether "access" is meant to include reinitialization.

However, other parts of CppCoreGuidelines seem to imply that it _is_ OK to reinitialize an object:

From ES.56:
"Usually, a std::move() is used as an argument to a && parameter. And after you do that, assume the object has been moved from (see C.64) and don't read its state again until you first set it to a new value."

And from C.64:
"Unless there is an exceptionally strong reason not to, make x = std::move(y); y = z; work with the conventional semantics."

clang-tidy/misc/UseAfterMoveCheck.cpp

29 ↗

(On Diff #68366)

Shouldn't those be single backquotes? (See https://www.stack.nl/~dimitri/doxygen/manual/markdown.html)

Done with single backquotes.

46 ↗

(On Diff #68366)

At least it's not something that I feel I would be able to do without breaking things -- I'm not familiar enough with SequenceChecker for that.

135 ↗

(On Diff #68366)

Leaving it here for now.

217 ↗

(On Diff #69206)

I've added CFG::synthetic_stmts() in D23842.

436 ↗

(On Diff #69206)

Added CFGBlock::succs() in D23842. This looks much nicer now!

test/clang-tidy/misc-use-after-move.cpp

505–506 ↗

(On Diff #69206)

Noted for a future addition. I'd like to get this patch in though without further expanding the functionality (it's already pretty big...).

[Prazek]

clang-tidy/misc/UseAfterMoveCheck.cpp

191 ↗

(On Diff #69206)

Dry: const auto *

[alexfh]

LG

clang-tidy/misc/UseAfterMoveCheck.cpp

30 ↗

(On Diff #69206)

Yep, you're right. I was thinking about .rst, probably.

[mboehme]

Responses to reviewer comments.

clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt

add_clang_library(clangTidyMiscModule
   SwappedArgumentsCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp
   UndelegatedConstructor.cpp
   UniqueptrResetReleaseCheck.cpp
   UnusedAliasDeclsCheck.cpp
   UnusedParametersCheck.cpp
   UnusedRAIICheck.cpp
   UnusedUsingDeclsCheck.cpp
+  UseAfterMoveCheck.cpp
   VirtualNearMissCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   clangTooling
   )

clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp

 #include "SwappedArgumentsCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedAliasDeclsCheck.h"
 #include "UnusedParametersCheck.h"
 #include "UnusedRAIICheck.h"
 #include "UnusedUsingDeclsCheck.h"
+#include "UseAfterMoveCheck.h"
 #include "VirtualNearMissCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 class MiscModule : public ClangTidyModule {
 public:
CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
         "misc-uniqueptr-reset-release");
     CheckFactories.registerCheck<UnusedAliasDeclsCheck>(
         "misc-unused-alias-decls");
     CheckFactories.registerCheck<UnusedParametersCheck>(
         "misc-unused-parameters");
     CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
     CheckFactories.registerCheck<UnusedUsingDeclsCheck>(
         "misc-unused-using-decls");
+    CheckFactories.registerCheck<UseAfterMoveCheck>("misc-use-after-move");
     CheckFactories.registerCheck<VirtualNearMissCheck>(
         "misc-virtual-near-miss");
   }
 };
 
 } // namespace misc
 
 // Register the MiscTidyModule using this statically initialized variable.

clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h

+//===--- UseAfterMoveCheck.h - clang-tidy ---------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEAFTERMOVECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEAFTERMOVECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// The check warns if an object is used after it has been moved, without an
+/// intervening reinitialization.
+///
+/// For details, see the user-facing documentation:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-use-after-move.html
+class UseAfterMoveCheck : public ClangTidyCheck {
+public:
+  UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEAFTERMOVECHECK_H

clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp

+//===--- UseAfterMoveCheck.cpp - clang-tidy -------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseAfterMoveCheck.h"
+
+#include "clang/Analysis/CFG.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallVector.h"
+
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+
+/// Provides information about the evaluation order of (sub-)expressions within
+/// a `CFGBlock`.
+///
+/// While a `CFGBlock` does contain individual `CFGElement`s for some
+/// sub-expressions, the order in which those `CFGElement`s appear reflects
+/// only one possible order in which the sub-expressions may be evaluated.
+/// However, we want to warn if any of the potential evaluation orders can lead
+/// to a use-after-move, not just the one contained in the `CFGBlock`.
+///
+/// This class implements only a simplified version of the C++ sequencing rules
+/// that is, however, sufficient for the purposes of this check. The main
+/// limitation is that we do not distinguish between value computation and side
+/// effect -- see the "Implementation" section for more details.
+///
+/// Note: `SequenceChecker` from SemaChecking.cpp does a similar job (and much
+/// more thoroughly), but using it would require
+/// - Pulling `SequenceChecker` out into a header file (i.e. making it part of
+///   the API),
+/// - Removing the dependency of `SequenceChecker` on `Sema`, and
+/// - (Probably) modifying `SequenceChecker` to make it suitable to be used in
+///   this context.
+/// For the moment, it seems preferable to re-implement our own version of
+/// sequence checking that is special-cased to what we need here.
+///
+/// Implementation
+/// --------------
+///
+/// `ExprSequence` uses two types of sequencing edges between nodes in the AST:
+///
+/// - Every `Stmt` is assumed to be sequenced after its children. This is
+///   overly optimistic because the standard only states that value computations
+///   of operands are sequenced before the value computation of the operator,
+///   making no guarantees about side effects (in general).
+///
+///   For our purposes, this rule is sufficient, however, because this check is
+///   interested in operations on objects, which are generally performed through
+///   function calls (whether explicit and implicit). Function calls guarantee
+///   that the value computations and side effects for all function arguments
+///   are sequenced before the execution fo the function.
+///
+/// - In addition, some `Stmt`s are known to be sequenced before or after
+///   their siblings. For example, the `Stmt`s that make up a `CompoundStmt`are
+///   all sequenced relative to each other. The function
+///   `getSequenceSuccessor()` implements these sequencing rules.
+class ExprSequence {
+public:
+  /// Initializes this `ExprSequence` with sequence information for the given
+  /// `CFG`.
+  ExprSequence(const CFG *TheCFG, ASTContext *TheContext);
+
+  /// Returns whether \p Before is sequenced before \p After.
+  bool inSequence(const Stmt *Before, const Stmt *After) const;
+
+  /// Returns whether \p After can potentially be evaluated after \p Before.
+  /// This is exactly equivalent to `!inSequence(After, Before)` but makes some
+  /// conditions read more naturally.
+  bool potentiallyAfter(const Stmt *After, const Stmt *Before) const;
+
+private:
+  // Returns the sibling of \p S (if any) that is directly sequenced after \p S,
+  // or nullptr if no such sibling exists. For example, if \p S is the child of
+  // a `CompoundStmt`, this would return the Stmt that directly follows \p S in
+  // the `CompoundStmt`.
+  //
+  // As the sequencing of many constructs that change control flow is already
+  // encoded in the `CFG`, this function only implements the sequencing rules
+  // for those constructs where sequencing cannot be inferred from the `CFG`.
+  const Stmt *getSequenceSuccessor(const Stmt *S) const;
+
+  const Stmt *resolveSyntheticStmt(const Stmt *S) const;
+
+  ASTContext *Context;
+
+  llvm::DenseMap<const Stmt *, const Stmt *> SyntheticStmtSourceMap;
+};
+
+/// Maps `Stmt`s to the `CFGBlock` that contains them. Some `Stmt`s may be
+/// contained in more than one `CFGBlock`; in this case, they are mapped to the
+/// innermost block (i.e. the one that is furthest from the root of the tree).
+class StmtToBlockMap {
+public:
+  /// Initializes the map for the given `CFG`.
+  StmtToBlockMap(const CFG *TheCFG, ASTContext *TheContext);
+
+  /// Returns the block that \p S is contained in. Some `Stmt`s may be contained
+  /// in more than one `CFGBlock`; in this case, this function returns the
+  /// innermost block (i.e. the one that is furthest from the root of the tree).
+  const CFGBlock *blockContainingStmt(const Stmt *S) const;
+
+private:
+  ASTContext *Context;
+
+  llvm::DenseMap<const Stmt *, const CFGBlock *> Map;
+};
+
+/// Contains information about a use-after-move.
+struct UseAfterMove {
+  // The DeclRefExpr that constituted the use of the object.
+  const DeclRefExpr *DeclRef;
+
+  // Is the order in which the move and the use are evaluated undefined?
+  bool EvaluationOrderUndefined;
+};
+
+/// Finds uses of a variable after a move (and maintains state required by the
+/// various internal helper functions).
+class UseAfterMoveFinder {
+public:
+  UseAfterMoveFinder(ASTContext *TheContext);
+
+  // Within the given function body, finds the first use of 'MovedVariable' that
+  // occurs after 'MovingCall' (the expression that performs the move). If a
+  // use-after-move is found, writes information about it to 'TheUseAfterMove'.
+  // Returns whether a use-after-move was found.
+  bool find(Stmt *FunctionBody, const Expr *MovingCall,
+            const ValueDecl *MovedVariable, UseAfterMove *TheUseAfterMove);
+
+private:
+  bool findInternal(const CFGBlock *Block, const Expr *MovingCall,
+                    const ValueDecl *MovedVariable,
+                    UseAfterMove *TheUseAfterMove);
+  void getUsesAndReinits(const CFGBlock *Block, const ValueDecl *MovedVariable,
+                         llvm::SmallVectorImpl<const DeclRefExpr *> *Uses,
+                         llvm::SmallPtrSetImpl<const Stmt *> *Reinits);
+  void getDeclRefs(const CFGBlock *Block, const Decl *MovedVariable,
+                   llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs);
+  void getReinits(const CFGBlock *Block, const ValueDecl *MovedVariable,
+                  llvm::SmallPtrSetImpl<const Stmt *> *Stmts,
+                  llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs);
+
+  ASTContext *Context;
+  std::unique_ptr<ExprSequence> Sequence;
+  std::unique_ptr<StmtToBlockMap> BlockMap;
+  llvm::SmallPtrSet<const CFGBlock *, 8> Visited;
+};
+
+} // namespace
+
+// Returns the Stmt nodes that are parents of 'S', skipping any potential
+// intermediate non-Stmt nodes.
+//
+// In almost all cases, this function returns a single parent or no parents at
+// all.
+//
+// The case that a Stmt has multiple parents is rare but does actually occur in
+// the parts of the AST that we're interested in. Specifically, InitListExpr
+// nodes cause ASTContext::getParent() to return multiple parents for certain
+// nodes in their subtree because RecursiveASTVisitor visits both the syntactic
+// and semantic forms of InitListExpr, and the parent-child relationships are
+// different between the two forms.
+static SmallVector<const Stmt *, 1> getParentStmts(const Stmt *S,
+                                                   ASTContext *Context) {
+  SmallVector<const Stmt *, 1> Result;
+
+  ASTContext::DynTypedNodeList Parents = Context->getParents(*S);
+
+  SmallVector<ast_type_traits::DynTypedNode, 1> NodesToProcess(Parents.begin(),
+                                                               Parents.end());
+
+  while (!NodesToProcess.empty()) {
+    ast_type_traits::DynTypedNode Node = NodesToProcess.back();
+    NodesToProcess.pop_back();
+
+    if (const auto *S = Node.get<Stmt>()) {
+      Result.push_back(S);
+    } else {
+      Parents = Context->getParents(Node);
+      NodesToProcess.append(Parents.begin(), Parents.end());
+    }
+  }
+
+  return Result;
+}
+
+bool isDescendantOrEqual(const Stmt *Descendant, const Stmt *Ancestor,
+                         ASTContext *Context) {
+  if (Descendant == Ancestor)
+    return true;
+  for (const Stmt *Parent : getParentStmts(Descendant, Context)) {
+    if (isDescendantOrEqual(Parent, Ancestor, Context))
+      return true;
+  }
+
+  return false;
+}
+
+ExprSequence::ExprSequence(const CFG *TheCFG, ASTContext *TheContext)
+    : Context(TheContext) {
+  for (const auto &SyntheticStmt : TheCFG->synthetic_stmts()) {
+    SyntheticStmtSourceMap[SyntheticStmt.first] = SyntheticStmt.second;
+  }
+}
+
+bool ExprSequence::inSequence(const Stmt *Before, const Stmt *After) const {
+  Before = resolveSyntheticStmt(Before);
+  After = resolveSyntheticStmt(After);
+
+  // If 'After' is in the subtree of the siblings that follow 'Before' in the
+  // chain of successors, we know that 'After' is sequenced after 'Before'.
+  for (const Stmt *Successor = getSequenceSuccessor(Before); Successor;
+       Successor = getSequenceSuccessor(Successor)) {
+    if (isDescendantOrEqual(After, Successor, Context))
+      return true;
+  }
+
+  // If 'After' is a parent of 'Before' or is sequenced after one of these
+  // parents, we know that it is sequenced after 'Before'.
+  for (const Stmt *Parent : getParentStmts(Before, Context)) {
+    if (Parent == After || inSequence(Parent, After))
+      return true;
+  }
+
+  return false;
+}
+
+bool ExprSequence::potentiallyAfter(const Stmt *After,
+                                    const Stmt *Before) const {
+  return !inSequence(After, Before);
+}
+
+const Stmt *ExprSequence::getSequenceSuccessor(const Stmt *S) const {
+  for (const Stmt *Parent : getParentStmts(S, Context)) {
+    if (const auto *BO = dyn_cast<BinaryOperator>(Parent)) {
+      // Comma operator: Right-hand side is sequenced after the left-hand side.
+      if (BO->getLHS() == S && BO->getOpcode() == BO_Comma)
+        return BO->getRHS();
+    } else if (const auto *InitList = dyn_cast<InitListExpr>(Parent)) {
+      // Initializer list: Each initializer clause is sequenced after the
+      // clauses that precede it.
+      for (unsigned I = 1; I < InitList->getNumInits(); ++I) {
+        if (InitList->getInit(I - 1) == S)
+          return InitList->getInit(I);
+      }
+    } else if (const auto *Compound = dyn_cast<CompoundStmt>(Parent)) {
+      // Compound statement: Each sub-statement is sequenced after the
+      // statements that precede it.
+      const Stmt *Previous = nullptr;
+      for (const auto *Child : Compound->body()) {
+        if (Previous == S)
+          return Child;
+        Previous = Child;
+      }
+    } else if (const auto *TheDeclStmt = dyn_cast<DeclStmt>(Parent)) {
+      // Declaration: Every initializer expression is sequenced after the
+      // initializer expressions that precede it.
+      const Expr *PreviousInit = nullptr;
+      for (const Decl *TheDecl : TheDeclStmt->decls()) {
+        if (const auto *TheVarDecl = dyn_cast<VarDecl>(TheDecl)) {
+          if (const Expr *Init = TheVarDecl->getInit()) {
+            if (PreviousInit == S)
+              return Init;
+            PreviousInit = Init;
+          }
+        }
+      }
+    } else if (const auto *ForRange = dyn_cast<CXXForRangeStmt>(Parent)) {
+      // Range-based for: Loop variable declaration is sequenced before the
+      // body. (We need this rule because these get placed in the same
+      // CFGBlock.)
+      if (S == ForRange->getLoopVarStmt())
+        return ForRange->getBody();
+    } else if (const auto *TheIfStmt = dyn_cast<IfStmt>(Parent)) {
+      // If statement: If a variable is declared inside the condition, the
+      // expression used to initialize the variable is sequenced before the
+      // evaluation of the condition.
+      if (S == TheIfStmt->getConditionVariableDeclStmt())
+        return TheIfStmt->getCond();
+    }
+  }
+
+  return nullptr;
+}
+
+const Stmt *ExprSequence::resolveSyntheticStmt(const Stmt *S) const {
+  if (SyntheticStmtSourceMap.count(S))
+    return SyntheticStmtSourceMap.lookup(S);
+  else
+    return S;
+}
+
+StmtToBlockMap::StmtToBlockMap(const CFG *TheCFG, ASTContext *TheContext)
+    : Context(TheContext) {
+  for (const auto *B : *TheCFG) {
+    for (const auto &Elem : *B) {
+      if (Optional<CFGStmt> S = Elem.getAs<CFGStmt>())
+        Map[S->getStmt()] = B;
+    }
+  }
+}
+
+const CFGBlock *StmtToBlockMap::blockContainingStmt(const Stmt *S) const {
+  while (!Map.count(S)) {
+    SmallVector<const Stmt *, 1> Parents = getParentStmts(S, Context);
+    if (Parents.empty())
+      return nullptr;
+    S = Parents[0];
+  }
+
+  return Map.lookup(S);
+}
+
+// Matches nodes that are
+// - Part of a decltype argument or class template argument (we check this by
+//   seeing if they are children of a TypeLoc), or
+// - Part of a function template argument (we check this by seeing if they are
+//   children of a DeclRefExpr that references a function template).
+// DeclRefExprs that fulfill these conditions should not be counted as a use or
+// move.
+static StatementMatcher inDecltypeOrTemplateArg() {
+  return anyOf(hasAncestor(typeLoc()),
+               hasAncestor(declRefExpr(
+                   to(functionDecl(ast_matchers::isTemplateInstantiation())))));
+}
+
+UseAfterMoveFinder::UseAfterMoveFinder(ASTContext *TheContext)
+    : Context(TheContext) {}
+
+bool UseAfterMoveFinder::find(Stmt *FunctionBody, const Expr *MovingCall,
+                              const ValueDecl *MovedVariable,
+                              UseAfterMove *TheUseAfterMove) {
+  // Generate the CFG manually instead of through an AnalysisDeclContext because
+  // it seems the latter can't be used to generate a CFG for the body of a
+  // labmda.
+  //
+  // We include implicit and temporary destructors in the CFG so that
+  // destructors marked [[noreturn]] are handled correctly in the control flow
+  // analysis. (These are used in some styles of assertion macros.)
+  CFG::BuildOptions Options;
+  Options.AddImplicitDtors = true;
+  Options.AddTemporaryDtors = true;
+  std::unique_ptr<CFG> TheCFG =
+      CFG::buildCFG(nullptr, FunctionBody, Context, Options);
+  if (!TheCFG)
+    return false;
+
+  Sequence.reset(new ExprSequence(TheCFG.get(), Context));
+  BlockMap.reset(new StmtToBlockMap(TheCFG.get(), Context));
+  Visited.clear();
+
+  const CFGBlock *Block = BlockMap->blockContainingStmt(MovingCall);
+  if (!Block)
+    return false;
+
+  return findInternal(Block, MovingCall, MovedVariable, TheUseAfterMove);
+}
+
+bool UseAfterMoveFinder::findInternal(const CFGBlock *Block,
+                                      const Expr *MovingCall,
+                                      const ValueDecl *MovedVariable,
+                                      UseAfterMove *TheUseAfterMove) {
+  if (Visited.count(Block))
+    return false;
+
+  // Mark the block as visited (except if this is the block containing the
+  // std::move() and it's being visited the first time).
+  if (!MovingCall)
+    Visited.insert(Block);
+
+  // Get all uses and reinits in the block.
+  llvm::SmallVector<const DeclRefExpr *, 1> Uses;
+  llvm::SmallPtrSet<const Stmt *, 1> Reinits;
+  getUsesAndReinits(Block, MovedVariable, &Uses, &Reinits);
+
+  // Ignore all reinitializations where the move potentially comes after the
+  // reinit.
+  llvm::SmallVector<const Stmt *, 1> ReinitsToDelete;
+  for (const Stmt *Reinit : Reinits) {
+    if (MovingCall && Sequence->potentiallyAfter(MovingCall, Reinit))
+      ReinitsToDelete.push_back(Reinit);
+  }
+  for (const Stmt *Reinit : ReinitsToDelete) {
+    Reinits.erase(Reinit);
+  }
+
+  // Find all uses that potentially come after the move.
+  for (const DeclRefExpr *Use : Uses) {
+    if (!MovingCall || Sequence->potentiallyAfter(Use, MovingCall)) {
+      // Does the use have a saving reinit? A reinit is saving if it definitely
+      // comes before the use, i.e. if there's no potential that the reinit is
+      // after the use.
+      bool HaveSavingReinit = false;
+      for (const Stmt *Reinit : Reinits) {
+        if (!Sequence->potentiallyAfter(Reinit, Use))
+          HaveSavingReinit = true;
+      }
+
+      if (!HaveSavingReinit) {
+        TheUseAfterMove->DeclRef = Use;
+
+        // Is this a use-after-move that depends on order of evaluation?
+        // This is the case if the move potentially comes after the use (and we
+        // already know that use potentially comes after the move, which taken
+        // together tells us that the ordering is unclear).
+        TheUseAfterMove->EvaluationOrderUndefined =
+            MovingCall != nullptr &&
+            Sequence->potentiallyAfter(MovingCall, Use);
+
+        return true;
+      }
+    }
+  }
+
+  // If the object wasn't reinitialized, call ourselves recursively on all
+  // successors.
+  if (Reinits.empty()) {
+    for (const auto &Succ : Block->succs()) {
+      if (Succ && findInternal(Succ, nullptr, MovedVariable, TheUseAfterMove))
+        return true;
+    }
+  }
+
+  return false;
+}
+
+void UseAfterMoveFinder::getUsesAndReinits(
+    const CFGBlock *Block, const ValueDecl *MovedVariable,
+    llvm::SmallVectorImpl<const DeclRefExpr *> *Uses,
+    llvm::SmallPtrSetImpl<const Stmt *> *Reinits) {
+  llvm::SmallPtrSet<const DeclRefExpr *, 1> DeclRefs;
+  llvm::SmallPtrSet<const DeclRefExpr *, 1> ReinitDeclRefs;
+
+  getDeclRefs(Block, MovedVariable, &DeclRefs);
+  getReinits(Block, MovedVariable, Reinits, &ReinitDeclRefs);
+
+  // All references to the variable that aren't reinitializations are uses.
+  Uses->clear();
+  for (const DeclRefExpr *DeclRef : DeclRefs) {
+    if (!ReinitDeclRefs.count(DeclRef))
+      Uses->push_back(DeclRef);
+  }
+
+  // Sort the uses by their occurrence in the source code.
+  std::sort(Uses->begin(), Uses->end(),
+            [](const DeclRefExpr *D1, const DeclRefExpr *D2) {
+              return D1->getExprLoc() < D2->getExprLoc();
+            });
+}
+
+void UseAfterMoveFinder::getDeclRefs(
+    const CFGBlock *Block, const Decl *MovedVariable,
+    llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs) {
+  DeclRefs->clear();
+  for (const auto &Elem : *Block) {
+    Optional<CFGStmt> S = Elem.getAs<CFGStmt>();
+    if (!S)
+      continue;
+
+    SmallVector<BoundNodes, 1> Matches =
+        match(findAll(declRefExpr(hasDeclaration(equalsNode(MovedVariable)),
+                                  unless(inDecltypeOrTemplateArg()))
+                          .bind("declref")),
+              *S->getStmt(), *Context);
+
+    for (const auto &Match : Matches) {
+      const auto *DeclRef = Match.getNodeAs<DeclRefExpr>("declref");
+      if (DeclRef && BlockMap->blockContainingStmt(DeclRef) == Block)
+        DeclRefs->insert(DeclRef);
+    }
+  }
+}
+
+void UseAfterMoveFinder::getReinits(
+    const CFGBlock *Block, const ValueDecl *MovedVariable,
+    llvm::SmallPtrSetImpl<const Stmt *> *Stmts,
+    llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs) {
+  auto DeclRefMatcher =
+      declRefExpr(hasDeclaration(equalsNode(MovedVariable))).bind("declref");
+
+  auto StandardContainerTypeMatcher = hasType(cxxRecordDecl(
+      hasAnyName("::std::basic_string", "::std::vector", "::std::deque",
+                 "::std::forward_list", "::std::list", "::std::set",
+                 "::std::map", "::std::multiset", "::std::multimap",
+                 "::std::unordered_set", "::std::unordered_map",
+                 "::std::unordered_multiset", "::std::unordered_multimap")));
+
+  // Matches different types of reinitialization.
+  auto ReinitMatcher =
+      stmt(anyOf(
+               // Assignment. In addition to the overloaded assignment operator,
+               // test for built-in assignment as well, since template functions
+               // may be instantiated to use std::move() on built-in types.
+               binaryOperator(hasOperatorName("="), hasLHS(DeclRefMatcher)),
+               cxxOperatorCallExpr(hasOverloadedOperatorName("="),
+                                   hasArgument(0, DeclRefMatcher)),
+               // Declaration. We treat this as a type of reinitialization too,
+               // so we don't need to treat it separately.
+               declStmt(hasDescendant(equalsNode(MovedVariable))),
+               // clear() and assign() on standard containers.
+               cxxMemberCallExpr(
+                   on(allOf(DeclRefMatcher, StandardContainerTypeMatcher)),
+                   // To keep the matcher simple, we check for assign() calls
+                   // on all standard containers, even though only vector,
+                   // deque, forward_list and list have assign(). If assign()
+                   // is called on any of the other containers, this will be
+                   // flagged by a compile error anyway.
+                   callee(cxxMethodDecl(hasAnyName("clear", "assign")))),
+               // Passing variable to a function as a non-const pointer.
+               callExpr(forEachArgumentWithParam(
+                   unaryOperator(hasOperatorName("&"),
+                                 hasUnaryOperand(DeclRefMatcher)),
+                   unless(parmVarDecl(hasType(pointsTo(isConstQualified())))))),
+               // Passing variable to a function as a non-const lvalue reference
+               // (unless that function is std::move()).
+               callExpr(forEachArgumentWithParam(
+                            DeclRefMatcher,
+                            unless(parmVarDecl(hasType(
+                                references(qualType(isConstQualified())))))),
+                        unless(callee(functionDecl(hasName("::std::move")))))))
+          .bind("reinit");
+
+  Stmts->clear();
+  DeclRefs->clear();
+  for (const auto &Elem : *Block) {
+    Optional<CFGStmt> S = Elem.getAs<CFGStmt>();
+    if (!S)
+      continue;
+
+    SmallVector<BoundNodes, 1> Matches =
+        match(findAll(ReinitMatcher), *S->getStmt(), *Context);
+
+    for (const auto &Match : Matches) {
+      const auto *TheStmt = Match.getNodeAs<Stmt>("reinit");
+      const auto *TheDeclRef = Match.getNodeAs<DeclRefExpr>("declref");
+      if (TheStmt && BlockMap->blockContainingStmt(TheStmt) == Block) {
+        Stmts->insert(TheStmt);
+
+        // We count DeclStmts as reinitializations, but they don't have a
+        // DeclRefExpr associated with them -- so we need to check 'TheDeclRef'
+        // before adding it to the set.
+        if (TheDeclRef)
+          DeclRefs->insert(TheDeclRef);
+      }
+    }
+  }
+}
+
+static void emitDiagnostic(const Expr *MovingCall,
+                           const ValueDecl *MovedVariable,
+                           const UseAfterMove &Use, ClangTidyCheck *Check,
+                           ASTContext *Context) {
+  Check->diag(Use.DeclRef->getExprLoc(), "'%0' used after it was moved")
+      << MovedVariable->getName();
+  Check->diag(MovingCall->getExprLoc(), "move occurred here",
+              DiagnosticIDs::Note);
+  if (Use.EvaluationOrderUndefined) {
+    Check->diag(Use.DeclRef->getExprLoc(),
+                "the use and move are unsequenced, i.e. there is no guarantee "
+                "about the order in which they are evaluated",
+                DiagnosticIDs::Note);
+  }
+}
+
+void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  auto StandardSmartPointerTypeMatcher = hasType(
+      cxxRecordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr")));
+
+  auto CallMoveMatcher =
+      callExpr(
+          callee(functionDecl(hasName("::std::move"))), argumentCountIs(1),
+          hasArgument(
+              0,
+              declRefExpr(unless(StandardSmartPointerTypeMatcher)).bind("arg")),
+          anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
+                hasAncestor(functionDecl().bind("containing-func"))),
+          unless(inDecltypeOrTemplateArg()))
+          .bind("call-move");
+
+  Finder->addMatcher(
+      // To find the Stmt that we assume performs the actual move, we look for
+      // the direct ancestor of the std::move() that isn't one of the node
+      // types ignored by ignoringParenImpCasts().
+      stmt(forEach(expr(ignoringParenImpCasts(CallMoveMatcher))),
+           unless(expr(ignoringParenImpCasts(equalsBoundNode("call-move")))))
+          .bind("moving-call"),
+      this);
+}
+
+void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *ContainingLambda =
+      Result.Nodes.getNodeAs<LambdaExpr>("containing-lambda");
+  const auto *ContainingFunc =
+      Result.Nodes.getNodeAs<FunctionDecl>("containing-func");
+  const auto *CallMove = Result.Nodes.getNodeAs<CallExpr>("call-move");
+  const auto *MovingCall = Result.Nodes.getNodeAs<Expr>("moving-call");
+  const auto *Arg = Result.Nodes.getNodeAs<DeclRefExpr>("arg");
+
+  if (!MovingCall)
+    MovingCall = CallMove;
+
+  Stmt *FunctionBody = nullptr;
+  if (ContainingLambda)
+    FunctionBody = ContainingLambda->getBody();
+  else if (ContainingFunc)
+    FunctionBody = ContainingFunc->getBody();
+  else
+    return;
+
+  const ValueDecl *MovedVariable = Arg->getDecl();
+
+  // Ignore the std::move if the variable that was passed to it isn't a local
+  // variable.
+  if (!Arg->getDecl()->getDeclContext()->isFunctionOrMethod())
+    return;
+
+  UseAfterMoveFinder finder(Result.Context);
+  UseAfterMove Use;
+  if (finder.find(FunctionBody, MovingCall, MovedVariable, &Use))
+    emitDiagnostic(MovingCall, MovedVariable, Use, this, Result.Context);
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang

clang-tools-extra/trunk/docs/ReleaseNotes.rst

- New `cppcoreguidelines-special-member-functions
   Flags classes where some, but not all, special member functions are user-defined.
 
 - New `misc-move-forwarding-reference
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-move-forwarding-reference.html>`_ check
 
   Warns when ``std::move`` is applied to a forwarding reference instead of
   ``std::forward``.
 
+- New `misc-use-after-move
+  <http://clang.llvm.org/extra/clang-tidy/checks/misc-use-after-move.html>`_ check
+
+  Warns if an object is used after it has been moved, without an intervening
+  reinitialization.
+
 - New `mpi-buffer-deref
   <http://clang.llvm.org/extra/clang-tidy/checks/mpi-buffer-deref.html>`_ check
 
   Flags buffers which are insufficiently dereferenced when passed to an MPI function call.
 
 - New `mpi-type-mismatch
   <http://clang.llvm.org/extra/clang-tidy/checks/mpi-type-mismatch.html>`_ check
 

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

.. toctree::
    misc-throw-by-value-catch-by-reference
    misc-unconventional-assign-operator
    misc-undelegated-constructor
    misc-uniqueptr-reset-release
    misc-unused-alias-decls
    misc-unused-parameters
    misc-unused-raii
    misc-unused-using-decls
+   misc-use-after-move
    misc-virtual-near-miss
    modernize-avoid-bind
    modernize-deprecated-headers
    modernize-loop-convert
    modernize-make-shared
    modernize-make-unique
    modernize-pass-by-value
    modernize-raw-string-literal

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst

+.. title:: clang-tidy - misc-use-after-move
+
+misc-use-after-move
+===================
+
+Warns if an object is used after it has been moved, for example:
+
+  .. code-block:: c++
+
+    std::string str = "Hello, world!\n";
+    std::vector<std::string> messages;
+    messages.emplace_back(std::move(str));
+    std::cout << str;
+
+The last line will trigger a warning that ``str`` is used after it has been
+moved.
+
+The check does not trigger a warning if the object is reinitialized after the
+move and before the use. For example, no warning will be output for this code:
+
+  .. code-block:: c++
+
+    messages.emplace_back(std::move(str));
+    str = "Greetings, stranger!\n";
+    std::cout << str;
+
+The check takes control flow into account. A warning is only emitted if the use
+can be reached from the move. This means that the following code does not
+produce a warning:
+
+  .. code-block:: c++
+
+    if (condition) {
+      messages.emplace_back(std::move(str));
+    } else {
+      std::cout << str;
+    }
+
+On the other hand, the following code does produce a warning:
+
+  .. code-block:: c++
+
+    for (int i = 0; i < 10; ++i) {
+      std::cout << str;
+      messages.emplace_back(std::move(str));
+    }
+
+(The use-after-move happens on the second iteration of the loop.)
+
+In some cases, the check may not be able to detect that two branches are
+mutually exclusive. For example (assuming that ``i`` is an int):
+
+  .. code-block:: c++
+
+    if (i == 1) {
+      messages.emplace_back(std::move(str));
+    }
+    if (i == 2) {
+      std::cout << str;
+    }
+
+In this case, the check will erroneously produce a warning, even though it is
+not possible for both the move and the use to be executed.
+
+An erroneous warning can be silenced by reinitializing the object after the
+move:
+
+  .. code-block:: c++
+
+    if (i == 1) {
+      messages.emplace_back(std::move(str));
+      str = "";
+    }
+    if (i == 2) {
+      std::cout << str;
+    }
+
+No warnings are emitted for objects of type ``std::unique_ptr`` and
+``std::shared_ptr``, as they have defined move behavior. (Objects of these
+classes are guaranteed to be empty after they have been moved from.)
+
+Subsections below explain more precisely what exactly the check considers to be
+a move, use, and reinitialization.
+
+Unsequenced moves, uses, and reinitializations
+----------------------------------------------
+
+In many cases, C++ does not make any guarantees about the order in which
+sub-expressions of a statement are evaluated. This means that in code like the
+following, it is not guaranteed whether the use will happen before or after the
+move:
+
+  .. code-block:: c++
+
+    void f(int i, std::vector<int> v);
+    std::vector<int> v = { 1, 2, 3 };
+    f(v[1], std::move(v));
+
+In this kind of situation, the check will note that the use and move are
+unsequenced.
+
+The check will also take sequencing rules into account when reinitializations
+occur in the same statement as moves or uses. A reinitialization is only
+considered to reinitialize a variable if it is guaranteed to be evaluated after
+the move and before the use.
+
+Move
+----
+
+The check currently only considers calls of ``std::move`` on local variables or
+function parameters. It does not check moves of member variables or global
+variables.
+
+Any call of ``std::move`` on a variable is considered to cause a move of that
+variable, even if the result of ``std::move`` is not passed to an rvalue
+reference parameter.
+
+This means that the check will flag a use-after-move even on a type that does
+not define a move constructor or move assignment operator. This is intentional.
+Developers may use ``std::move`` on such a type in the expectation that the type
+will add move semantics in the future. If such a ``std::move`` has the potential
+to cause a use-after-move, we want to warn about it even if the type does not
+implement move semantics yet.
+
+Furthermore, if the result of ``std::move`` *is* passed to an rvalue reference
+parameter, this will always be considered to cause a move, even if the function
+that consumes this parameter does not move from it, or if it does so only
+conditionally. For example, in the following situation, the check will assume
+that a move always takes place:
+
+  .. code-block:: c++
+
+    std::vector<std::string> messages;
+    void f(std::string &&str) {
+      // Only remember the message if it isn't empty.
+      if (!str.empty()) {
+        messages.emplace_back(std::move(str));
+      }
+    }
+    std::string str = "";
+    f(std::move(str));
+
+The check will assume that the last line causes a move, even though, in this
+particular case, it does not. Again, this is intentional.
+
+When analyzing the order in which moves, uses and reinitializations happen (see
+section `Unsequenced moves, uses, and reinitializations`_), the move is assumed
+to occur in whichever function the result of the ``std::move`` is passed to.
+
+Use
+---
+
+Any occurrence of the moved variable that is not a reinitialization (see below)
+is considered to be a use.
+
+If multiple uses occur after a move, only the first of these is flagged.
+
+Reinitialization
+----------------
+
+The check considers a variable to be reinitialized in the following cases:
+
+  - The variable occurs on the left-hand side of an assignment.
+
+  - The variable is passed to a function as a non-const pointer or non-const
+    lvalue reference. (It is assumed that the variable may be an out-parameter
+    for the function.)
+
+  - ``clear()`` or ``assign()`` is called on the variable and the variable is of
+    one of the standard container types ``basic_string``, ``vector``, ``deque``,
+    ``forward_list``, ``list``, ``set``, ``map``, ``multiset``, ``multimap``,
+    ``unordered_set``, ``unordered_map``, ``unordered_multiset``,
+    ``unordered_multimap``.
+
+If the variable in question is a struct and an individual member variable of
+that struct is written to, the check does not consider this to be a
+reinitialization -- even if, eventually, all member variables of the struct are
+written to. For example:
+
+  .. code-block:: c++
+
+    struct S {
+      std::string str;
+      int i;
+    };
+    S s = { "Hello, world!\n", 42 };
+    S s_other = std::move(s);
+    s.str = "Lorem ipsum";
+    s.i = 99;
+
+The check will not consider ``s`` to be reinitialized after the last line;
+instead, the line that assigns to ``s.str`` will be flagged as a use-after-move.
+This is intentional as this pattern of reinitializing a struct is error-prone.
+For example, if an additional member variable is added to ``S``, it is easy to
+forget to add the reinitialization for this additional member. Instead, it is
+safer to assign to the entire struct in one go, and this will also avoid the
+use-after-move warning.

clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp

+// RUN: %check_clang_tidy %s misc-use-after-move %t
+
+typedef decltype(nullptr) nullptr_t;
+
+namespace std {
+typedef unsigned size_t;
+
+template <typename T>
+struct unique_ptr {
+  unique_ptr();
+  T *get() const;
+};
+
+template <typename T>
+struct shared_ptr {
+  shared_ptr();
+  T *get() const;
+};
+
+#define DECLARE_STANDARD_CONTAINER(name) \
+  template <typename T>                  \
+  struct name {                          \
+    name();                              \
+    void clear();                        \
+    bool empty();                        \
+  }
+
+#define DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(name) \
+  template <typename T>                              \
+  struct name {                                      \
+    name();                                          \
+    void clear();                                    \
+    bool empty();                                    \
+    void assign(size_t, const T &);                  \
+  }
+
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(basic_string);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(vector);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(deque);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(forward_list);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(list);
+DECLARE_STANDARD_CONTAINER(set);
+DECLARE_STANDARD_CONTAINER(map);
+DECLARE_STANDARD_CONTAINER(multiset);
+DECLARE_STANDARD_CONTAINER(multimap);
+DECLARE_STANDARD_CONTAINER(unordered_set);
+DECLARE_STANDARD_CONTAINER(unordered_map);
+DECLARE_STANDARD_CONTAINER(unordered_multiset);
+DECLARE_STANDARD_CONTAINER(unordered_multimap);
+
+typedef basic_string<char> string;
+
+template <typename>
+struct remove_reference;
+
+template <typename _Tp>
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template <typename _Tp>
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template <typename _Tp>
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept {
+  return static_cast<typename remove_reference<_Tp>::type &&>(__t);
+}
+
+} // namespace std
+
+class A {
+public:
+  A();
+  A(const A &);
+  A(A &&);
+
+  A &operator=(const A &);
+  A &operator=(A &&);
+
+  void foo() const;
+  int getInt() const;
+
+  operator bool() const;
+
+  int i;
+};
+
+////////////////////////////////////////////////////////////////////////////////
+// General tests.
+
+// Simple case.
+void simple() {
+  A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:15: note: move occurred here
+}
+
+// A warning should only be emitted for one use-after-move.
+void onlyFlagOneUseAfterMove() {
+  A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:15: note: move occurred here
+  a.foo();
+}
+
+void moveAfterMove() {
+  // Move-after-move also counts as a use.
+  {
+    A a;
+    std::move(a);
+    std::move(a);
+    // CHECK-MESSAGES: [[@LINE-1]]:15: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // This is also true if the move itself turns into the use on the second loop
+  // iteration.
+  {
+    A a;
+    for (int i = 0; i < 10; ++i) {
+      std::move(a);
+      // CHECK-MESSAGES: [[@LINE-1]]:17: warning: 'a' used after it was moved
+      // CHECK-MESSAGES: [[@LINE-2]]:7: note: move occurred here
+    }
+  }
+}
+
+// Checks also works on function parameters that have a use-after move.
+void parameters(A a) {
+  std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:3: note: move occurred here
+}
+
+void uniquePtrAndSharedPtr() {
+  // Use-after-moves on std::unique_ptr<> or std::shared_ptr<> aren't flagged.
+  {
+    std::unique_ptr<A> ptr;
+    std::move(ptr);
+    ptr.get();
+  }
+  {
+    std::shared_ptr<A> ptr;
+    std::move(ptr);
+    ptr.get();
+  }
+  // This is also true if the std::unique_ptr<> or std::shared_ptr<> is wrapped
+  // in a typedef.
+  {
+    typedef std::unique_ptr<A> PtrToA;
+    PtrToA ptr;
+    std::move(ptr);
+    ptr.get();
+  }
+  {
+    typedef std::shared_ptr<A> PtrToA;
+    PtrToA ptr;
+    std::move(ptr);
+    ptr.get();
+  }
+  // And it's also true if the template argument is a little more involved.
+  {
+    struct B {
+      typedef A AnotherNameForA;
+    };
+    std::unique_ptr<B::AnotherNameForA> ptr;
+    std::move(ptr);
+    ptr.get();
+  }
+}
+
+// The check also works in member functions.
+class Container {
+  void useAfterMoveInMemberFunction() {
+    A a;
+    std::move(a);
+    a.foo();
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+};
+
+// We see the std::move() if it's inside a declaration.
+void moveInDeclaration() {
+  A a;
+  A another_a(std::move(a));
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+}
+
+// We see the std::move if it's inside an initializer list. Initializer lists
+// are a special case because they cause ASTContext::getParents() to return
+// multiple parents for certain nodes in their subtree. This is because
+// RecursiveASTVisitor visits both the syntactic and semantic forms of
+// InitListExpr, and the parent-child relationships are different between the
+// two forms.
+void moveInInitList() {
+  struct S {
+    A a;
+  };
+  A a;
+  S s{std::move(a)};
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:6: note: move occurred here
+}
+
+void lambdas() {
+  // Use-after-moves inside a lambda should be detected.
+  {
+    A a;
+    auto lambda = [a] {
+      std::move(a);
+      a.foo();
+      // CHECK-MESSAGES: [[@LINE-1]]:7: warning: 'a' used after it was moved
+      // CHECK-MESSAGES: [[@LINE-3]]:7: note: move occurred here
+    };
+  }
+  // This is just as true if the variable was declared inside the lambda.
+  {
+    auto lambda = [] {
+      A a;
+      std::move(a);
+      a.foo();
+      // CHECK-MESSAGES: [[@LINE-1]]:7: warning: 'a' used after it was moved
+      // CHECK-MESSAGES: [[@LINE-3]]:7: note: move occurred here
+    };
+  }
+  // But don't warn if the move happened inside the lambda but the use happened
+  // outside -- because
+  // - the 'a' inside the lambda is a copy, and
+  // - we don't know when the lambda will get called anyway
+  {
+    A a;
+    auto lambda = [a] {
+      std::move(a);
+    };
+    a.foo();
+  }
+  // Warn if the use consists of a capture that happens after a move.
+  {
+    A a;
+    std::move(a);
+    auto lambda = [a]() { a.foo(); };
+    // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // ...even if the capture was implicit.
+  {
+    A a;
+    std::move(a);
+    auto lambda = [=]() { a.foo(); };
+    // CHECK-MESSAGES: [[@LINE-1]]:27: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // Same tests but for capture by reference.
+  {
+    A a;
+    std::move(a);
+    auto lambda = [&a]() { a.foo(); };
+    // CHECK-MESSAGES: [[@LINE-1]]:21: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  {
+    A a;
+    std::move(a);
+    auto lambda = [&]() { a.foo(); };
+    // CHECK-MESSAGES: [[@LINE-1]]:27: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // But don't warn if the move happened after the capture.
+  {
+    A a;
+    auto lambda = [a]() { a.foo(); };
+    std::move(a);
+  }
+  // ...and again, same thing with an implicit move.
+  {
+    A a;
+    auto lambda = [=]() { a.foo(); };
+    std::move(a);
+  }
+  // Same tests but for capture by reference.
+  {
+    A a;
+    auto lambda = [&a]() { a.foo(); };
+    std::move(a);
+  }
+  {
+    A a;
+    auto lambda = [&]() { a.foo(); };
+    std::move(a);
+  }
+}
+
+// Use-after-moves are detected in uninstantiated templates if the moved type
+// is not a dependent type.
+template <class T>
+void movedTypeIsNotDependentType() {
+  T t;
+  A a;
+  std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:3: note: move occurred here
+}
+
+// And if the moved type is a dependent type, the use-after-move is detected if
+// the template is instantiated.
+template <class T>
+void movedTypeIsDependentType() {
+  T t;
+  std::move(t);
+  t.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 't' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:3: note: move occurred here
+}
+template void movedTypeIsDependentType<A>();
+
+// Using decltype on an expression is not a use.
+void decltypeIsNotUse() {
+  A a;
+  std::move(a);
+  decltype(a) other_a;
+}
+
+// Ignore moves or uses that occur as part of template arguments.
+template <int>
+class ClassTemplate {
+public:
+  void foo(A a);
+};
+template <int>
+void functionTemplate(A a);
+void templateArgIsNotUse() {
+  {
+    // A pattern like this occurs in the EXPECT_EQ and ASSERT_EQ macros in
+    // Google Test.
+    A a;
+    ClassTemplate<sizeof(A(std::move(a)))>().foo(std::move(a));
+  }
+  {
+    A a;
+    functionTemplate<sizeof(A(std::move(a)))>(std::move(a));
+  }
+}
+
+// Ignore moves of global variables.
+A global_a;
+void ignoreGlobalVariables() {
+  std::move(global_a);
+  global_a.foo();
+}
+
+// Ignore moves of member variables.
+class IgnoreMemberVariables {
+  A a;
+  static A static_a;
+
+  void f() {
+    std::move(a);
+    a.foo();
+
+    std::move(static_a);
+    static_a.foo();
+  }
+};
+
+////////////////////////////////////////////////////////////////////////////////
+// Tests involving control flow.
+
+void useAndMoveInLoop() {
+  // Warn about use-after-moves if they happen in a later loop iteration than
+  // the std::move().
+  {
+    A a;
+    for (int i = 0; i < 10; ++i) {
+      a.foo();
+      // CHECK-MESSAGES: [[@LINE-1]]:7: warning: 'a' used after it was moved
+      // CHECK-MESSAGES: [[@LINE+1]]:7: note: move occurred here
+      std::move(a);
+    }
+  }
+  // However, this case shouldn't be flagged -- the scope of the declaration of
+  // 'a' is important.
+  {
+    for (int i = 0; i < 10; ++i) {
+      A a;
+      a.foo();
+      std::move(a);
+    }
+  }
+  // Same as above, except that we have an unrelated variable being declared in
+  // the same declaration as 'a'. This case is interesting because it tests that
+  // the synthetic DeclStmts generated by the CFG are sequenced correctly
+  // relative to the other statements.
+  {
+    for (int i = 0; i < 10; ++i) {
+      A a, other;
+      a.foo();
+      std::move(a);
+    }
+  }
+  // Don't warn if we return after the move.
+  {
+    A a;
+    for (int i = 0; i < 10; ++i) {
+      a.foo();
+      if (a.getInt() > 0) {
+        std::move(a);
+        return;
+      }
+    }
+  }
+}
+
+void differentBranches(int i) {
+  // Don't warn if the use is in a different branch from the move.
+  {
+    A a;
+    if (i > 0) {
+      std::move(a);
+    } else {
+      a.foo();
+    }
+  }
+  // Same thing, but with a ternary operator.
+  {
+    A a;
+    i > 0 ? (void)std::move(a) : a.foo();
+  }
+  // A variation on the theme above.
+  {
+    A a;
+    a.getInt() > 0 ? a.getInt() : A(std::move(a)).getInt();
+  }
+  // Same thing, but with a switch statement.
+  {
+    A a;
+    switch (i) {
+    case 1:
+      std::move(a);
+      break;
+    case 2:
+      a.foo();
+      break;
+    }
+  }
+  // However, if there's a fallthrough, we do warn.
+  {
+    A a;
+    switch (i) {
+    case 1:
+      std::move(a);
+    case 2:
+      a.foo();
+      // CHECK-MESSAGES: [[@LINE-1]]:7: warning: 'a' used after it was moved
+      // CHECK-MESSAGES: [[@LINE-4]]:7: note: move occurred here
+      break;
+    }
+  }
+}
+
+// False positive: A use-after-move is flagged even though the "if (b)" and
+// "if (!b)" are mutually exclusive.
+void mutuallyExclusiveBranchesFalsePositive(bool b) {
+  A a;
+  if (b) {
+    std::move(a);
+  }
+  if (!b) {
+    a.foo();
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-5]]:5: note: move occurred here
+  }
+}
+
+// Destructors marked [[noreturn]] are handled correctly in the control flow
+// analysis. (These are used in some styles of assertion macros.)
+class FailureLogger {
+public:
+  FailureLogger();
+  [[noreturn]] ~FailureLogger();
+  void log(const char *);
+};
+#define ASSERT(x) \
+  while (x)       \
+  FailureLogger().log(#x)
+bool operationOnA(A);
+void noreturnDestructor() {
+  A a;
+  // The while loop in the ASSERT() would ordinarily have the potential to cause
+  // a use-after-move because the second iteration of the loop would be using a
+  // variable that had been moved from in the first iteration. Check that the
+  // CFG knows that the second iteration of the loop is never reached because
+  // the FailureLogger destructor is marked [[noreturn]].
+  ASSERT(operationOnA(std::move(a)));
+}
+#undef ASSERT
+
+////////////////////////////////////////////////////////////////////////////////
+// Tests for reinitializations
+
+template <class T>
+void swap(T &a, T &b) {
+  T tmp = std::move(a);
+  a = std::move(b);
+  b = std::move(tmp);
+}
+void assignments(int i) {
+  // Don't report a use-after-move if the variable was assigned to in the
+  // meantime.
+  {
+    A a;
+    std::move(a);
+    a = A();
+    a.foo();
+  }
+  // The assignment should also be recognized if move, assignment and use don't
+  // all happen in the same block (but the assignment is still guaranteed to
+  // prevent a use-after-move).
+  {
+    A a;
+    if (i == 1) {
+      std::move(a);
+      a = A();
+    }
+    if (i == 2) {
+      a.foo();
+    }
+  }
+  {
+    A a;
+    if (i == 1) {
+      std::move(a);
+    }
+    if (i == 2) {
+      a = A();
+      a.foo();
+    }
+  }
+  // The built-in assignment operator should also be recognized as a
+  // reinitialization. (std::move() may be called on built-in types in template
+  // code.)
+  {
+    int a1 = 1, a2 = 2;
+    swap(a1, a2);
+  }
+  // A std::move() after the assignment makes the variable invalid again.
+  {
+    A a;
+    std::move(a);
+    a = A();
+    std::move(a);
+    a.foo();
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // Report a use-after-move if we can't be sure that the variable was assigned
+  // to.
+  {
+    A a;
+    std::move(a);
+    if (i < 10) {
+      a = A();
+    }
+    if (i > 5) {
+      a.foo();
+      // CHECK-MESSAGES: [[@LINE-1]]:7: warning: 'a' used after it was moved
+      // CHECK-MESSAGES: [[@LINE-7]]:5: note: move occurred here
+    }
+  }
+}
+
+// Passing the object to a function through a non-const pointer or refernce
+// counts as a re-initialization.
+void passByNonConstPointer(A *);
+void passByNonConstReference(A &);
+void passByNonConstPointerIsReinit() {
+  {
+    A a;
+    std::move(a);
+    passByNonConstPointer(&a);
+    a.foo();
+  }
+  {
+    A a;
+    std::move(a);
+    passByNonConstReference(a);
+    a.foo();
+  }
+}
+
+// Passing the object through a const pointer or reference counts as a use --
+// since the called function cannot reinitialize the object.
+void passByConstPointer(const A *);
+void passByConstReference(const A &);
+void passByConstPointerIsUse() {
+  {
+    // Declaring 'a' as const so that no ImplicitCastExpr is inserted into the
+    // AST -- we wouldn't want the check to rely solely on that to detect a
+    // const pointer argument.
+    const A a;
+    std::move(a);
+    passByConstPointer(&a);
+    // CHECK-MESSAGES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  const A a;
+  std::move(a);
+  passByConstReference(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:3: note: move occurred here
+}
+
+// Clearing a standard container using clear() is treated as a
+// re-initialization.
+void standardContainerClearIsReinit() {
+  {
+    std::string container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::vector<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::deque<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::forward_list<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::list<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::set<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::map<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::multiset<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::multimap<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::unordered_set<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::unordered_map<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::unordered_multiset<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  {
+    std::unordered_multimap<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  // This should also work for typedefs of standard containers.
+  {
+    typedef std::vector<int> IntVector;
+    IntVector container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  // But it shouldn't work for non-standard containers.
+  {
+    // This might be called "vector", but it's not in namespace "std".
+    struct vector {
+      void clear() {}
+    } container;
+    std::move(container);
+    container.clear();
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'container' used after it was
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // An intervening clear() on a different container does not reinitialize.
+  {
+    std::vector<int> container1, container2;
+    std::move(container1);
+    container2.clear();
+    container1.empty();
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'container1' used after it was
+    // CHECK-MESSAGES: [[@LINE-4]]:5: note: move occurred here
+  }
+}
+
+// Clearing a standard container using assign() is treated as a
+// re-initialization.
+void standardContainerAssignIsReinit() {
+  {
+    std::string container;
+    std::move(container);
+    container.assign(0, ' ');
+    container.empty();
+  }
+  {
+    std::vector<int> container;
+    std::move(container);
+    container.assign(0, 0);
+    container.empty();
+  }
+  {
+    std::deque<int> container;
+    std::move(container);
+    container.assign(0, 0);
+    container.empty();
+  }
+  {
+    std::forward_list<int> container;
+    std::move(container);
+    container.assign(0, 0);
+    container.empty();
+  }
+  {
+    std::list<int> container;
+    std::move(container);
+    container.clear();
+    container.empty();
+  }
+  // But it doesn't work for non-standard containers.
+  {
+    // This might be called "vector", but it's not in namespace "std".
+    struct vector {
+      void assign(std::size_t, int) {}
+    } container;
+    std::move(container);
+    container.assign(0, 0);
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'container' used after it was
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // An intervening assign() on a different container does not reinitialize.
+  {
+    std::vector<int> container1, container2;
+    std::move(container1);
+    container2.assign(0, 0);
+    container1.empty();
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'container1' used after it was
+    // CHECK-MESSAGES: [[@LINE-4]]:5: note: move occurred here
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Tests related to order of evaluation within expressions
+
+// Relative sequencing of move and use.
+void passByRvalueReference(int i, A &&a);
+void passByValue(int i, A a);
+void passByValue(A a, int i);
+A g(A, A &&);
+int intFromA(A &&);
+int intFromInt(int);
+void sequencingOfMoveAndUse() {
+  // This case is fine because the move only happens inside
+  // passByRvalueReference(). At this point, a.getInt() is guaranteed to have
+  // been evaluated.
+  {
+    A a;
+    passByRvalueReference(a.getInt(), std::move(a));
+  }
+  // However, if we pass by value, the move happens when the move constructor is
+  // called to create a temporary, and this happens before the call to
+  // passByValue(). Because the order in which arguments are evaluated isn't
+  // defined, the move may happen before the call to a.getInt().
+  //
+  // Check that we warn about a potential use-after move for both orderings of
+  // a.getInt() and std::move(a), independent of the order in which the
+  // arguments happen to get evaluated by the compiler.
+  {
+    A a;
+    passByValue(a.getInt(), std::move(a));
+    // CHECK-MESSAGES: [[@LINE-1]]:17: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-2]]:29: note: move occurred here
+    // CHECK-MESSAGES: [[@LINE-3]]:17: note: the use and move are unsequenced
+  }
+  {
+    A a;
+    passByValue(std::move(a), a.getInt());
+    // CHECK-MESSAGES: [[@LINE-1]]:31: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-2]]:17: note: move occurred here
+    // CHECK-MESSAGES: [[@LINE-3]]:31: note: the use and move are unsequenced
+  }
+  // An even more convoluted example.
+  {
+    A a;
+    g(g(a, std::move(a)), g(a, std::move(a)));
+    // CHECK-MESSAGES: [[@LINE-1]]:9: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-2]]:27: note: move occurred here
+    // CHECK-MESSAGES: [[@LINE-3]]:9: note: the use and move are unsequenced
+    // CHECK-MESSAGES: [[@LINE-4]]:29: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-5]]:7: note: move occurred here
+    // CHECK-MESSAGES: [[@LINE-6]]:29: note: the use and move are unsequenced
+  }
+  // This case is fine because the actual move only happens inside the call to
+  // operator=(). a.getInt(), by necessity, is evaluated before that call.
+  {
+    A a;
+    A vec[1];
+    vec[a.getInt()] = std::move(a);
+  }
+  // However, in the following case, the move happens before the assignment, and
+  // so the order of evaluation is not guaranteed.
+  {
+    A a;
+    int v[3];
+    v[a.getInt()] = intFromA(std::move(a));
+    // CHECK-MESSAGES: [[@LINE-1]]:7: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-2]]:21: note: move occurred here
+    // CHECK-MESSAGES: [[@LINE-3]]:7: note: the use and move are unsequenced
+  }
+  {
+    A a;
+    int v[3];
+    v[intFromA(std::move(a))] = intFromInt(a.i);
+    // CHECK-MESSAGES: [[@LINE-1]]:44: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-2]]:7: note: move occurred here
+    // CHECK-MESSAGES: [[@LINE-3]]:44: note: the use and move are unsequenced
+  }
+}
+
+// Relative sequencing of move and reinitialization. If the two are unsequenced,
+// we conservatively assume that the move happens after the reinitialization,
+// i.e. the that object does not get reinitialized after the move.
+A MutateA(A a);
+void passByValue(A a1, A a2);
+void sequencingOfMoveAndReinit() {
+  // Move and reinitialization as function arguments (which are indeterminately
+  // sequenced). Again, check that we warn for both orderings.
+  {
+    A a;
+    passByValue(std::move(a), (a = A()));
+    a.foo();
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:17: note: move occurred here
+  }
+  {
+    A a;
+    passByValue((a = A()), std::move(a));
+    a.foo();
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:28: note: move occurred here
+  }
+  // Common usage pattern: Move the object to a function that mutates it in some
+  // way, then reassign the result to the object. This pattern is fine.
+  {
+    A a;
+    a = MutateA(std::move(a));
+    a.foo();
+  }
+}
+
+// Relative sequencing of reinitialization and use. If the two are unsequenced,
+// we conservatively assume that the reinitialization happens after the use,
+// i.e. that the object is not reinitialized at the point in time when it is
+// used.
+void sequencingOfReinitAndUse() {
+  // Reinitialization and use in function arguments. Again, check both possible
+  // orderings.
+  {
+    A a;
+    std::move(a);
+    passByValue(a.getInt(), (a = A()));
+    // CHECK-MESSAGES: [[@LINE-1]]:17: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  {
+    A a;
+    std::move(a);
+    passByValue((a = A()), a.getInt());
+    // CHECK-MESSAGES: [[@LINE-1]]:28: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+}
+
+// The comma operator sequences its operands.
+void commaOperatorSequences() {
+  {
+    A a;
+    A(std::move(a))
+    , (a = A());
+    a.foo();
+  }
+  {
+    A a;
+    (a = A()), A(std::move(a));
+    a.foo();
+    // CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-3]]:16: note: move occurred here
+  }
+}
+
+// An initializer list sequences its initialization clauses.
+void initializerListSequences() {
+  {
+    struct S1 {
+      int i;
+      A a;
+    };
+    A a;
+    S1 s1{a.getInt(), std::move(a)};
+  }
+  {
+    struct S2 {
+      A a;
+      int i;
+    };
+    A a;
+    S2 s2{std::move(a), a.getInt()};
+    // CHECK-MESSAGES: [[@LINE-1]]:25: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-2]]:11: note: move occurred here
+  }
+}
+
+// A declaration statement containing multiple declarations sequences the
+// initializer expressions.
+void declarationSequences() {
+  {
+    A a;
+    A a1 = a, a2 = std::move(a);
+  }
+  {
+    A a;
+    A a1 = std::move(a), a2 = a;
+    // CHECK-MESSAGES: [[@LINE-1]]:31: warning: 'a' used after it was moved
+    // CHECK-MESSAGES: [[@LINE-2]]:12: note: move occurred here
+  }
+}
+
+// The logical operators && and || sequence their operands.
+void logicalOperatorsSequence() {
+  {
+    A a;
+    if (a.getInt() > 0 && A(std::move(a)).getInt() > 0) {
+      A().foo();
+    }
+  }
+  // A variation: Negate the result of the && (which pushes the && further down
+  // into the AST).
+  {
+    A a;
+    if (!(a.getInt() > 0 && A(std::move(a)).getInt() > 0)) {
+      A().foo();
+    }
+  }
+  {
+    A a;
+    if (A(std::move(a)).getInt() > 0 && a.getInt() > 0) {
+      // CHECK-MESSAGES: [[@LINE-1]]:41: warning: 'a' used after it was moved
+      // CHECK-MESSAGES: [[@LINE-2]]:9: note: move occurred here
+      A().foo();
+    }
+  }
+  {
+    A a;
+    if (a.getInt() > 0 || A(std::move(a)).getInt() > 0) {
+      A().foo();
+    }
+  }
+  {
+    A a;
+    if (A(std::move(a)).getInt() > 0 || a.getInt() > 0) {
+      // CHECK-MESSAGES: [[@LINE-1]]:41: warning: 'a' used after it was moved
+      // CHECK-MESSAGES: [[@LINE-2]]:9: note: move occurred here
+      A().foo();
+    }
+  }
+}
+
+// A range-based for sequences the loop variable declaration before the body.
+void forRangeSequences() {
+  A v[2] = {A(), A()};
+  for (A &a : v) {
+    std::move(a);
+  }
+}
+
+// If a variable is declared in an if statement, the declaration of the variable
+// (which is treated like a reinitialization by the check) is sequenced before
+// the evaluation of the condition (which constitutes a use).
+void ifStmtSequencesDeclAndCondition() {
+  for (int i = 0; i < 10; ++i) {
+    if (A a = A()) {
+      std::move(a);
+    }
+  }
+}