forked from OSchip/llvm-project
[clang-tidy] Extend UnnecessaryCopyInitialization check to trigger on non-const copies that can be safely converted to const references.
Summary: Move code shared between UnnecessaryCopyInitialization and ForRangeCopyCheck into utilities files. Add more test cases for UnnecessaryCopyInitialization and disable fixes inside of macros. Reviewers: alexfh Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D17488 llvm-svn: 262781
This commit is contained in:
parent
ac25f2c073
commit
adfdc14832
|
@ -8,92 +8,15 @@
|
|||
//===----------------------------------------------------------------------===//
|
||||
|
||||
#include "ForRangeCopyCheck.h"
|
||||
#include "../utils/LexerUtils.h"
|
||||
#include "../utils/DeclRefExprUtils.h"
|
||||
#include "../utils/FixItHintUtils.h"
|
||||
#include "../utils/TypeTraits.h"
|
||||
#include "clang/Lex/Lexer.h"
|
||||
#include "llvm/ADT/SmallPtrSet.h"
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace performance {
|
||||
|
||||
using namespace ::clang::ast_matchers;
|
||||
using llvm::SmallPtrSet;
|
||||
|
||||
namespace {
|
||||
|
||||
template <typename S> bool isSetDifferenceEmpty(const S &S1, const S &S2) {
|
||||
for (const auto &E : S1)
|
||||
if (S2.count(E) == 0)
|
||||
return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
// Extracts all Nodes keyed by ID from Matches and inserts them into Nodes.
|
||||
template <typename Node>
|
||||
void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
|
||||
SmallPtrSet<const Node *, 16> &Nodes) {
|
||||
for (const auto &Match : Matches)
|
||||
Nodes.insert(Match.getNodeAs<Node>(ID));
|
||||
}
|
||||
|
||||
// Finds all DeclRefExprs to VarDecl in Stmt.
|
||||
SmallPtrSet<const DeclRefExpr *, 16>
|
||||
declRefExprs(const VarDecl &VarDecl, const Stmt &Stmt, ASTContext &Context) {
|
||||
auto Matches = match(
|
||||
findAll(declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef")),
|
||||
Stmt, Context);
|
||||
SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
|
||||
extractNodesByIdTo(Matches, "declRef", DeclRefs);
|
||||
return DeclRefs;
|
||||
}
|
||||
|
||||
// Finds all DeclRefExprs where a const method is called on VarDecl or VarDecl
|
||||
// is the a const reference or value argument to a CallExpr or CXXConstructExpr.
|
||||
SmallPtrSet<const DeclRefExpr *, 16>
|
||||
constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
|
||||
ASTContext &Context) {
|
||||
auto DeclRefToVar =
|
||||
declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef");
|
||||
auto ConstMethodCallee = callee(cxxMethodDecl(isConst()));
|
||||
// Match method call expressions where the variable is referenced as the this
|
||||
// implicit object argument and opertor call expression for member operators
|
||||
// where the variable is the 0-th argument.
|
||||
auto Matches = match(
|
||||
findAll(expr(anyOf(cxxMemberCallExpr(ConstMethodCallee, on(DeclRefToVar)),
|
||||
cxxOperatorCallExpr(ConstMethodCallee,
|
||||
hasArgument(0, DeclRefToVar))))),
|
||||
Stmt, Context);
|
||||
SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
|
||||
extractNodesByIdTo(Matches, "declRef", DeclRefs);
|
||||
auto ConstReferenceOrValue =
|
||||
qualType(anyOf(referenceType(pointee(qualType(isConstQualified()))),
|
||||
unless(anyOf(referenceType(), pointerType()))));
|
||||
auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
|
||||
DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValue)));
|
||||
Matches = match(findAll(callExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
|
||||
extractNodesByIdTo(Matches, "declRef", DeclRefs);
|
||||
Matches =
|
||||
match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
|
||||
extractNodesByIdTo(Matches, "declRef", DeclRefs);
|
||||
return DeclRefs;
|
||||
}
|
||||
|
||||
// Modifies VarDecl to be a reference.
|
||||
FixItHint createAmpersandFix(const VarDecl &VarDecl, ASTContext &Context) {
|
||||
SourceLocation AmpLocation = VarDecl.getLocation();
|
||||
auto Token = lexer_utils::getPreviousNonCommentToken(Context, AmpLocation);
|
||||
if (!Token.is(tok::unknown))
|
||||
AmpLocation = Token.getLocation().getLocWithOffset(Token.getLength());
|
||||
return FixItHint::CreateInsertion(AmpLocation, "&");
|
||||
}
|
||||
|
||||
// Modifies VarDecl to be const.
|
||||
FixItHint createConstFix(const VarDecl &VarDecl) {
|
||||
return FixItHint::CreateInsertion(VarDecl.getTypeSpecStartLoc(), "const ");
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
ForRangeCopyCheck::ForRangeCopyCheck(StringRef Name, ClangTidyContext *Context)
|
||||
: ClangTidyCheck(Name, Context),
|
||||
|
@ -143,9 +66,9 @@ bool ForRangeCopyCheck::handleConstValueCopy(const VarDecl &LoopVar,
|
|||
diag(LoopVar.getLocation(),
|
||||
"the loop variable's type is not a reference type; this creates a "
|
||||
"copy in each iteration; consider making this a reference")
|
||||
<< createAmpersandFix(LoopVar, Context);
|
||||
<< utils::create_fix_it::changeVarDeclToReference(LoopVar, Context);
|
||||
if (!LoopVar.getType().isConstQualified())
|
||||
Diagnostic << createConstFix(LoopVar);
|
||||
Diagnostic << utils::create_fix_it::changeVarDeclToConst(LoopVar);
|
||||
return true;
|
||||
}
|
||||
|
||||
|
@ -154,24 +77,15 @@ bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
|
|||
ASTContext &Context) {
|
||||
llvm::Optional<bool> Expensive =
|
||||
type_traits::isExpensiveToCopy(LoopVar.getType(), Context);
|
||||
if (LoopVar.getType().isConstQualified() || !Expensive || !*Expensive) {
|
||||
if (LoopVar.getType().isConstQualified() || !Expensive || !*Expensive)
|
||||
return false;
|
||||
}
|
||||
// Collect all DeclRefExprs to the loop variable and all CallExprs and
|
||||
// CXXConstructExprs where the loop variable is used as argument to a const
|
||||
// reference parameter.
|
||||
// If the difference is empty it is safe for the loop variable to be a const
|
||||
// reference.
|
||||
auto AllDeclRefs = declRefExprs(LoopVar, *ForRange.getBody(), Context);
|
||||
auto ConstReferenceDeclRefs =
|
||||
constReferenceDeclRefExprs(LoopVar, *ForRange.getBody(), Context);
|
||||
if (AllDeclRefs.empty() ||
|
||||
!isSetDifferenceEmpty(AllDeclRefs, ConstReferenceDeclRefs))
|
||||
if (!decl_ref_expr_utils::isOnlyUsedAsConst(LoopVar, *ForRange.getBody(), Context))
|
||||
return false;
|
||||
diag(LoopVar.getLocation(),
|
||||
"loop variable is copied but only used as const reference; consider "
|
||||
"making it a const reference")
|
||||
<< createConstFix(LoopVar) << createAmpersandFix(LoopVar, Context);
|
||||
<< utils::create_fix_it::changeVarDeclToConst(LoopVar)
|
||||
<< utils::create_fix_it::changeVarDeclToReference(LoopVar, Context);
|
||||
return true;
|
||||
}
|
||||
|
||||
|
|
|
@ -9,7 +9,8 @@
|
|||
|
||||
#include "UnnecessaryCopyInitialization.h"
|
||||
|
||||
#include "../utils/LexerUtils.h"
|
||||
#include "../utils/DeclRefExprUtils.h"
|
||||
#include "../utils/FixItHintUtils.h"
|
||||
#include "../utils/Matchers.h"
|
||||
|
||||
namespace clang {
|
||||
|
@ -18,17 +19,13 @@ namespace performance {
|
|||
|
||||
using namespace ::clang::ast_matchers;
|
||||
|
||||
namespace {
|
||||
AST_MATCHER(QualType, isPointerType) { return Node->isPointerType(); }
|
||||
} // namespace
|
||||
|
||||
void UnnecessaryCopyInitialization::registerMatchers(
|
||||
ast_matchers::MatchFinder *Finder) {
|
||||
auto ConstReference = referenceType(pointee(qualType(isConstQualified())));
|
||||
auto ConstOrConstReference =
|
||||
allOf(anyOf(ConstReference, isConstQualified()),
|
||||
unless(allOf(isPointerType(), unless(pointerType(pointee(qualType(
|
||||
isConstQualified())))))));
|
||||
unless(allOf(pointerType(), unless(pointerType(pointee(
|
||||
qualType(isConstQualified())))))));
|
||||
// Match method call expressions where the this argument is a const
|
||||
// type or const reference. This returned const reference is highly likely to
|
||||
// outlive the local const reference of the variable being declared.
|
||||
|
@ -41,30 +38,44 @@ void UnnecessaryCopyInitialization::registerMatchers(
|
|||
callExpr(callee(functionDecl(returns(ConstReference))),
|
||||
unless(callee(cxxMethodDecl())));
|
||||
Finder->addMatcher(
|
||||
varDecl(
|
||||
hasLocalStorage(), hasType(isConstQualified()),
|
||||
hasType(matchers::isExpensiveToCopy()),
|
||||
hasInitializer(cxxConstructExpr(
|
||||
hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
|
||||
hasArgument(0, anyOf(ConstRefReturningFunctionCall,
|
||||
compoundStmt(
|
||||
forEachDescendant(
|
||||
varDecl(
|
||||
hasLocalStorage(), hasType(matchers::isExpensiveToCopy()),
|
||||
hasInitializer(cxxConstructExpr(
|
||||
hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
|
||||
hasArgument(
|
||||
0, anyOf(ConstRefReturningFunctionCall,
|
||||
ConstRefReturningMethodCallOfConstParam)))))
|
||||
.bind("varDecl"),
|
||||
.bind("varDecl"))).bind("blockStmt"),
|
||||
this);
|
||||
}
|
||||
|
||||
void UnnecessaryCopyInitialization::check(
|
||||
const ast_matchers::MatchFinder::MatchResult &Result) {
|
||||
const auto *Var = Result.Nodes.getNodeAs<VarDecl>("varDecl");
|
||||
SourceLocation AmpLocation = Var->getLocation();
|
||||
auto Token = lexer_utils::getPreviousNonCommentToken(*Result.Context,
|
||||
Var->getLocation());
|
||||
if (!Token.is(tok::unknown)) {
|
||||
AmpLocation = Token.getLocation().getLocWithOffset(Token.getLength());
|
||||
}
|
||||
diag(Var->getLocation(),
|
||||
"the const qualified variable '%0' is copy-constructed from a "
|
||||
"const reference; consider making it a const reference")
|
||||
<< Var->getName() << FixItHint::CreateInsertion(AmpLocation, "&");
|
||||
const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
|
||||
bool IsConstQualified = Var->getType().isConstQualified();
|
||||
if (!IsConstQualified &&
|
||||
!decl_ref_expr_utils::isOnlyUsedAsConst(*Var, *BlockStmt,
|
||||
*Result.Context))
|
||||
return;
|
||||
DiagnosticBuilder Diagnostic =
|
||||
diag(Var->getLocation(),
|
||||
IsConstQualified ? "the const qualified variable '%0' is "
|
||||
"copy-constructed from a const reference; "
|
||||
"consider making it a const reference"
|
||||
: "the variable '%0' is copy-constructed from a "
|
||||
"const reference but is only used as const "
|
||||
"reference; consider making it a const reference")
|
||||
<< Var->getName();
|
||||
// Do not propose fixes in macros since we cannot place them correctly.
|
||||
if (Var->getLocStart().isMacroID())
|
||||
return;
|
||||
Diagnostic << utils::create_fix_it::changeVarDeclToReference(*Var,
|
||||
*Result.Context);
|
||||
if (!IsConstQualified)
|
||||
Diagnostic << utils::create_fix_it::changeVarDeclToConst(*Var);
|
||||
}
|
||||
|
||||
} // namespace performance
|
||||
|
|
|
@ -16,10 +16,10 @@ namespace clang {
|
|||
namespace tidy {
|
||||
namespace performance {
|
||||
|
||||
// A check that detects const local variable declarations that are copy
|
||||
// initialized with the const reference of a function call or the const
|
||||
// reference of a method call whose object is guaranteed to outlive the
|
||||
// variable's scope and suggests to use a const reference.
|
||||
// The check detects local variable declarations that are copy initialized with
|
||||
// the const reference of a function call or the const reference of a method
|
||||
// call whose object is guaranteed to outlive the variable's scope and suggests
|
||||
// to use a const reference.
|
||||
//
|
||||
// The check currently only understands a subset of variables that are
|
||||
// guaranteed to outlive the const reference returned, namely: const variables,
|
||||
|
|
|
@ -1,6 +1,8 @@
|
|||
set(LLVM_LINK_COMPONENTS support)
|
||||
|
||||
add_clang_library(clangTidyUtils
|
||||
DeclRefExprUtils.cpp
|
||||
FixItHintUtils.cpp
|
||||
HeaderGuard.cpp
|
||||
HeaderFileExtensionsUtils.cpp
|
||||
IncludeInserter.cpp
|
||||
|
|
|
@ -0,0 +1,98 @@
|
|||
//===--- DeclRefExprUtils.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 "DeclRefExprUtils.h"
|
||||
#include "clang/AST/ASTContext.h"
|
||||
#include "clang/AST/DeclCXX.h"
|
||||
#include "clang/ASTMatchers/ASTMatchFinder.h"
|
||||
#include "llvm/ADT/SmallPtrSet.h"
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace decl_ref_expr_utils {
|
||||
|
||||
using namespace ::clang::ast_matchers;
|
||||
using llvm::SmallPtrSet;
|
||||
|
||||
namespace {
|
||||
|
||||
template <typename S> bool isSetDifferenceEmpty(const S &S1, const S &S2) {
|
||||
for (const auto &E : S1)
|
||||
if (S2.count(E) == 0)
|
||||
return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
// Extracts all Nodes keyed by ID from Matches and inserts them into Nodes.
|
||||
template <typename Node>
|
||||
void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
|
||||
SmallPtrSet<const Node *, 16> &Nodes) {
|
||||
for (const auto &Match : Matches)
|
||||
Nodes.insert(Match.getNodeAs<Node>(ID));
|
||||
}
|
||||
|
||||
// Finds all DeclRefExprs to VarDecl in Stmt.
|
||||
SmallPtrSet<const DeclRefExpr *, 16>
|
||||
declRefExprs(const VarDecl &VarDecl, const Stmt &Stmt, ASTContext &Context) {
|
||||
auto Matches = match(
|
||||
findAll(declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef")),
|
||||
Stmt, Context);
|
||||
SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
|
||||
extractNodesByIdTo(Matches, "declRef", DeclRefs);
|
||||
return DeclRefs;
|
||||
}
|
||||
|
||||
// Finds all DeclRefExprs where a const method is called on VarDecl or VarDecl
|
||||
// is the a const reference or value argument to a CallExpr or CXXConstructExpr.
|
||||
SmallPtrSet<const DeclRefExpr *, 16>
|
||||
constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
|
||||
ASTContext &Context) {
|
||||
auto DeclRefToVar =
|
||||
declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef");
|
||||
auto ConstMethodCallee = callee(cxxMethodDecl(isConst()));
|
||||
// Match method call expressions where the variable is referenced as the this
|
||||
// implicit object argument and opertor call expression for member operators
|
||||
// where the variable is the 0-th argument.
|
||||
auto Matches = match(
|
||||
findAll(expr(anyOf(cxxMemberCallExpr(ConstMethodCallee, on(DeclRefToVar)),
|
||||
cxxOperatorCallExpr(ConstMethodCallee,
|
||||
hasArgument(0, DeclRefToVar))))),
|
||||
Stmt, Context);
|
||||
SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
|
||||
extractNodesByIdTo(Matches, "declRef", DeclRefs);
|
||||
auto ConstReferenceOrValue =
|
||||
qualType(anyOf(referenceType(pointee(qualType(isConstQualified()))),
|
||||
unless(anyOf(referenceType(), pointerType()))));
|
||||
auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
|
||||
DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValue)));
|
||||
Matches = match(findAll(callExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
|
||||
extractNodesByIdTo(Matches, "declRef", DeclRefs);
|
||||
Matches =
|
||||
match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
|
||||
extractNodesByIdTo(Matches, "declRef", DeclRefs);
|
||||
return DeclRefs;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
|
||||
ASTContext &Context) {
|
||||
// Collect all DeclRefExprs to the loop variable and all CallExprs and
|
||||
// CXXConstructExprs where the loop variable is used as argument to a const
|
||||
// reference parameter.
|
||||
// If the difference is empty it is safe for the loop variable to be a const
|
||||
// reference.
|
||||
auto AllDeclRefs = declRefExprs(Var, Stmt, Context);
|
||||
auto ConstReferenceDeclRefs = constReferenceDeclRefExprs(Var, Stmt, Context);
|
||||
return isSetDifferenceEmpty(AllDeclRefs, ConstReferenceDeclRefs);
|
||||
}
|
||||
|
||||
} // namespace decl_ref_expr_utils
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
|
@ -0,0 +1,32 @@
|
|||
//===--- DeclRefExprUtils.h - clang-tidy-------------------------*- C++ -*-===//
|
||||
//
|
||||
// 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_UTILS_DECLREFEXPRUTILS_H
|
||||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_DECLREFEXPRUTILS_H
|
||||
|
||||
#include "clang/AST/ASTContext.h"
|
||||
#include "clang/AST/Type.h"
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace decl_ref_expr_utils {
|
||||
|
||||
/// \brief Returns true if all DeclRefExpr to the variable within Stmt do not
|
||||
/// modify it.
|
||||
///
|
||||
/// Returns true if only const methods or operators are called on the variable
|
||||
/// or the variable is a const reference or value argument to a callExpr().
|
||||
bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
|
||||
ASTContext &Context);
|
||||
|
||||
} // namespace decl_ref_expr_utils
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
||||
|
||||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_DECLREFEXPRUTILS_H
|
|
@ -0,0 +1,36 @@
|
|||
//===--- FixItHintUtils.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 "FixItHintUtils.h"
|
||||
#include "LexerUtils.h"
|
||||
#include "clang/AST/ASTContext.h"
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace utils {
|
||||
namespace create_fix_it {
|
||||
|
||||
FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context) {
|
||||
SourceLocation AmpLocation = Var.getLocation();
|
||||
auto Token = lexer_utils::getPreviousNonCommentToken(Context, AmpLocation);
|
||||
if (!Token.is(tok::unknown))
|
||||
AmpLocation = Lexer::getLocForEndOfToken(Token.getLocation(), 0,
|
||||
Context.getSourceManager(),
|
||||
Context.getLangOpts());
|
||||
return FixItHint::CreateInsertion(AmpLocation, "&");
|
||||
}
|
||||
|
||||
FixItHint changeVarDeclToConst(const VarDecl &Var) {
|
||||
return FixItHint::CreateInsertion(Var.getTypeSpecStartLoc(), "const ");
|
||||
}
|
||||
|
||||
} // namespace create_fix_it
|
||||
} // namespace utils
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
|
@ -0,0 +1,32 @@
|
|||
//===--- FixItHintUtils.h - clang-tidy---------------------------*- C++ -*-===//
|
||||
//
|
||||
// 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_UTILS_FIXITHINTUTILS_H
|
||||
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H
|
||||
|
||||
#include "clang/AST/ASTContext.h"
|
||||
#include "clang/AST/Decl.h"
|
||||
|
||||
namespace clang {
|
||||
namespace tidy {
|
||||
namespace utils {
|
||||
namespace create_fix_it {
|
||||
|
||||
/// \brief Creates fix to make VarDecl a reference by adding '&'.
|
||||
FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context);
|
||||
|
||||
/// \brief Creates fix to make VarDecl const qualified.
|
||||
FixItHint changeVarDeclToConst(const VarDecl &Var);
|
||||
|
||||
} // namespace create_fix_it
|
||||
} // utils
|
||||
} // namespace tidy
|
||||
} // namespace clang
|
||||
|
||||
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H
|
|
@ -0,0 +1,34 @@
|
|||
.. title:: clang-tidy - performance-unnecessary-copy-initialization
|
||||
|
||||
performance-unnecessary-copy-initialization
|
||||
===========================================
|
||||
|
||||
Finds local variable declarations that are initialized using the copy
|
||||
constructor of a non-trivially-copyable type but it would suffice to obtain a
|
||||
const reference.
|
||||
|
||||
The check is only applied if it is safe to replace the copy by a const
|
||||
reference. This is the case when the variable is const qualified or when it is
|
||||
only used as a const, i.e. only const methods or operators are invoked on it, or
|
||||
it is used as const reference or value argument in constructors or function
|
||||
calls.
|
||||
|
||||
Example:
|
||||
|
||||
.. code-block:: c++
|
||||
|
||||
const string& constReference() {
|
||||
}
|
||||
void function() {
|
||||
// The warning will suggest making this a const reference.
|
||||
const string UnnecessaryCopy = constReference();
|
||||
}
|
||||
|
||||
struct Foo {
|
||||
const string& name() const;
|
||||
}
|
||||
void Function(const Foo& foo) {
|
||||
// The warning will suggest making this a const reference.
|
||||
string UnnecessaryCopy = foo.name();
|
||||
UnnecessaryCopy.find("bar");
|
||||
}
|
|
@ -4,6 +4,7 @@ struct ExpensiveToCopyType {
|
|||
ExpensiveToCopyType() {}
|
||||
virtual ~ExpensiveToCopyType() {}
|
||||
const ExpensiveToCopyType &reference() const { return *this; }
|
||||
void nonConstMethod() {}
|
||||
};
|
||||
|
||||
struct TrivialToCopyType {
|
||||
|
@ -20,6 +21,11 @@ const TrivialToCopyType &TrivialTypeReference() {
|
|||
return *Type;
|
||||
}
|
||||
|
||||
void mutate(ExpensiveToCopyType &);
|
||||
void mutate(ExpensiveToCopyType *);
|
||||
void useAsConstReference(const ExpensiveToCopyType &);
|
||||
void useByValue(ExpensiveToCopyType);
|
||||
|
||||
void PositiveFunctionCall() {
|
||||
const auto AutoAssigned = ExpensiveTypeReference();
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
|
||||
|
@ -114,11 +120,53 @@ void NegativeStaticLocalVar(const ExpensiveToCopyType &Obj) {
|
|||
static const auto StaticVar = Obj.reference();
|
||||
}
|
||||
|
||||
void NegativeFunctionCallExpensiveTypeNonConstVariable() {
|
||||
void PositiveFunctionCallExpensiveTypeNonConstVariable() {
|
||||
auto AutoAssigned = ExpensiveTypeReference();
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'AutoAssigned' is copy-constructed from a const reference but is only used as const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
|
||||
// CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference();
|
||||
auto AutoCopyConstructed(ExpensiveTypeReference());
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable
|
||||
// CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveTypeReference());
|
||||
ExpensiveToCopyType VarAssigned = ExpensiveTypeReference();
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:23: warning: the variable
|
||||
// CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = ExpensiveTypeReference();
|
||||
ExpensiveToCopyType VarCopyConstructed(ExpensiveTypeReference());
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:23: warning: the variable
|
||||
// CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(ExpensiveTypeReference());
|
||||
}
|
||||
|
||||
void positiveNonConstVarInCodeBlock(const ExpensiveToCopyType &Obj) {
|
||||
{
|
||||
auto Assigned = Obj.reference();
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:10: warning: the variable
|
||||
// CHECK-FIXES: const auto& Assigned = Obj.reference();
|
||||
Assigned.reference();
|
||||
useAsConstReference(Assigned);
|
||||
useByValue(Assigned);
|
||||
}
|
||||
}
|
||||
|
||||
void negativeNonConstVarWithNonConstUse(const ExpensiveToCopyType &Obj) {
|
||||
{
|
||||
auto NonConstInvoked = Obj.reference();
|
||||
// CHECK-FIXES: auto NonConstInvoked = Obj.reference();
|
||||
NonConstInvoked.nonConstMethod();
|
||||
}
|
||||
{
|
||||
auto Reassigned = Obj.reference();
|
||||
// CHECK-FIXES: auto Reassigned = Obj.reference();
|
||||
Reassigned = ExpensiveToCopyType();
|
||||
}
|
||||
{
|
||||
auto MutatedByReference = Obj.reference();
|
||||
// CHECK-FIXES: auto MutatedByReference = Obj.reference();
|
||||
mutate(MutatedByReference);
|
||||
}
|
||||
{
|
||||
auto MutatedByPointer = Obj.reference();
|
||||
// CHECK-FIXES: auto MutatedByPointer = Obj.reference();
|
||||
mutate(&MutatedByPointer);
|
||||
}
|
||||
}
|
||||
|
||||
void NegativeMethodCallNonConstRef(ExpensiveToCopyType &Obj) {
|
||||
|
@ -146,11 +194,32 @@ void NegativeObjIsNotParam() {
|
|||
ExpensiveToCopyType Obj;
|
||||
const auto AutoAssigned = Obj.reference();
|
||||
const auto AutoCopyConstructed(Obj.reference());
|
||||
const ExpensiveToCopyType VarAssigned = Obj.reference();
|
||||
const ExpensiveToCopyType VarCopyConstructed(Obj.reference());
|
||||
ExpensiveToCopyType VarAssigned = Obj.reference();
|
||||
ExpensiveToCopyType VarCopyConstructed(Obj.reference());
|
||||
}
|
||||
|
||||
struct NegativeConstructor {
|
||||
NegativeConstructor(const ExpensiveToCopyType &Obj) : Obj(Obj) {}
|
||||
ExpensiveToCopyType Obj;
|
||||
};
|
||||
|
||||
#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE) \
|
||||
void functionWith##TYPE(const TYPE& T) { \
|
||||
auto AssignedInMacro = T.reference(); \
|
||||
} \
|
||||
// Ensure fix is not applied.
|
||||
// CHECK-FIXES: auto AssignedInMacro = T.reference();
|
||||
|
||||
|
||||
UNNECESSARY_COPY_INIT_IN_MACRO_BODY(ExpensiveToCopyType)
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:1: warning: the variable 'AssignedInMacro' is copy-constructed
|
||||
|
||||
#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT) \
|
||||
ARGUMENT
|
||||
|
||||
void PositiveMacroArgument(const ExpensiveToCopyType &Obj) {
|
||||
UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(auto CopyInMacroArg = Obj.reference());
|
||||
// CHECK-MESSAGES: [[@LINE-1]]:48: warning: the variable 'CopyInMacroArg' is copy-constructed
|
||||
// Ensure fix is not applied.
|
||||
// CHECK-FIXES: auto CopyInMacroArg = Obj.reference()
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue