[clang-tools-extra] r281453 - [clang-tidy] Add check 'misc-use-after-move'
Martin Böhme via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 14 06:36:23 PDT 2016
clang-tidy/misc/CMakeLists.txt was missing a dependency on clangAnalysis.
For some reason, the Linux build wasn't affected by this and still built
correctly. I'll submit a patch to correct this.
On 14 September 2016 at 15:20, Martin Böhme <mboehme at google.com> wrote:
> This has also caused the PPC build bot to fail:
>
> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-
> multistage/builds/2488/steps/build%20stage%201/logs/stdio
>
> It's complaining about undefined symbols from CFG. I hope it's an obvious
> missing dependency.
>
> On 14 September 2016 at 12:29, Martin Bohme via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: mboehme
>> Date: Wed Sep 14 05:29:32 2016
>> New Revision: 281453
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=281453&view=rev
>> Log:
>> [clang-tidy] Add check 'misc-use-after-move'
>>
>> Summary:
>> The check warns if an object is used after it has been moved, without an
>> intervening reinitialization.
>>
>> See user-facing documentation for details.
>>
>> Reviewers: sbenza, Prazek, alexfh
>>
>> Subscribers: beanz, mgorny, shadeware, omtcyfz, Eugene.Zelenko, Prazek,
>> fowles, ioeric, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D23353
>>
>> Added:
>> clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
>> clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h
>> clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-afte
>> r-move.rst
>> clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
>> clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
>> clang-tools-extra/trunk/docs/ReleaseNotes.rst
>> clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>>
>> Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy/misc/CMakeLists.txt?rev=281453&r1=281452&r2=281453&view=diff
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
>> +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Wed Sep 14
>> 05:29:32 2016
>> @@ -43,6 +43,7 @@ add_clang_library(clangTidyMiscModule
>> UnusedParametersCheck.cpp
>> UnusedRAIICheck.cpp
>> UnusedUsingDeclsCheck.cpp
>> + UseAfterMoveCheck.cpp
>> VirtualNearMissCheck.cpp
>>
>> LINK_LIBS
>>
>> Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy/misc/MiscTidyModule.cpp?rev=281453&r1=281452&r2=
>> 281453&view=diff
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
>> +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Wed Sep
>> 14 05:29:32 2016
>> @@ -51,6 +51,7 @@
>> #include "UnusedParametersCheck.h"
>> #include "UnusedRAIICheck.h"
>> #include "UnusedUsingDeclsCheck.h"
>> +#include "UseAfterMoveCheck.h"
>> #include "VirtualNearMissCheck.h"
>>
>> namespace clang {
>> @@ -139,6 +140,7 @@ public:
>> CheckFactories.registerCheck<UnusedRAIICheck>("misc-unused-raii");
>> CheckFactories.registerCheck<UnusedUsingDeclsCheck>(
>> "misc-unused-using-decls");
>> + CheckFactories.registerCheck<UseAfterMoveCheck>("misc-use-af
>> ter-move");
>> CheckFactories.registerCheck<VirtualNearMissCheck>(
>> "misc-virtual-near-miss");
>> }
>>
>> Added: clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy/misc/UseAfterMoveCheck.cpp?rev=281453&view=auto
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp (added)
>> +++ clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp Wed
>> Sep 14 05:29:32 2016
>> @@ -0,0 +1,643 @@
>> +//===--- 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(MovedVar
>> iable)),
>> + 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(po
>> intsTo(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(isConstQua
>> lified())))))),
>> + unless(callee(functionDecl(has
>> Name("::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(StandardSma
>> rtPointerTypeMatcher)).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
>>
>> Added: clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> clang-tidy/misc/UseAfterMoveCheck.h?rev=281453&view=auto
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h (added)
>> +++ clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h Wed Sep
>> 14 05:29:32 2016
>> @@ -0,0 +1,36 @@
>> +//===--- 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
>>
>> Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> docs/ReleaseNotes.rst?rev=281453&r1=281452&r2=281453&view=diff
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
>> +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Sep 14 05:29:32
>> 2016
>> @@ -73,6 +73,12 @@ Improvements to clang-tidy
>> 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-afte
>> r-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
>>
>>
>> Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> docs/clang-tidy/checks/list.rst?rev=281453&r1=281452&r2=281453&view=diff
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
>> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Sep 14
>> 05:29:32 2016
>> @@ -93,6 +93,7 @@ Clang-Tidy Checks
>> misc-unused-parameters
>> misc-unused-raii
>> misc-unused-using-decls
>> + misc-use-after-move
>> misc-virtual-near-miss
>> modernize-avoid-bind
>> modernize-deprecated-headers
>>
>> Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-afte
>> r-move.rst
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> docs/clang-tidy/checks/misc-use-after-move.rst?rev=281453&view=auto
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst
>> (added)
>> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst
>> Wed Sep 14 05:29:32 2016
>> @@ -0,0 +1,197 @@
>> +.. 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.
>>
>> Added: clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> test/clang-tidy/misc-use-after-move.cpp?rev=281453&view=auto
>> ============================================================
>> ==================
>> --- clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
>> (added)
>> +++ clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp Wed
>> Sep 14 05:29:32 2016
>> @@ -0,0 +1,1039 @@
>> +// 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);
>> + }
>> + }
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
>
> --
>
> Martin Böhme
>
> Software Engineer
>
> mboehme at google.com
>
> Google Germany GmbH
> Erika-Mann-Straße 33
> 80363 München
>
> Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
>
> Registergericht und -nummer: Hamburg, HRB 86891
>
> Sitz der Gesellschaft: Hamburg
>
> Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
> leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
> löschen Sie die E-Mail und alle Anhänge. Vielen Dank.
>
>
>
> This e-mail is confidential. If you are not the right addressee please do
> not forward it, please inform the sender, and please erase this e-mail
> including any attachments. Thanks.
>
--
Martin Böhme
Software Engineer
mboehme at google.com
Google Germany GmbH
Erika-Mann-Straße 33
80363 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.
This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160914/48204fbd/attachment-0001.html>
More information about the cfe-commits
mailing list