[clang-tools-extra] r262781 - [clang-tidy] Extend UnnecessaryCopyInitialization check to trigger on non-const copies that can be safely converted to const references.
Felix Berger via cfe-commits
cfe-commits at lists.llvm.org
Sat Mar 5 13:17:58 PST 2016
Author: flx
Date: Sat Mar 5 15:17:58 2016
New Revision: 262781
URL: http://llvm.org/viewvc/llvm-project?rev=262781&view=rev
Log:
[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
Added:
clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.h
clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.h
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
Modified:
clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h
clang-tools-extra/trunk/clang-tidy/utils/CMakeLists.txt
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp
Modified: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp?rev=262781&r1=262780&r2=262781&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp Sat Mar 5 15:17:58 2016
@@ -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::handleConstValue
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::handleCopyIsOnly
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;
}
Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp?rev=262781&r1=262780&r2=262781&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp Sat Mar 5 15:17:58 2016
@@ -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::regi
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
Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h?rev=262781&r1=262780&r2=262781&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h Sat Mar 5 15:17:58 2016
@@ -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,
Modified: clang-tools-extra/trunk/clang-tidy/utils/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/CMakeLists.txt?rev=262781&r1=262780&r2=262781&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/CMakeLists.txt Sat Mar 5 15:17:58 2016
@@ -1,6 +1,8 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyUtils
+ DeclRefExprUtils.cpp
+ FixItHintUtils.cpp
HeaderGuard.cpp
HeaderFileExtensionsUtils.cpp
IncludeInserter.cpp
Added: clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.cpp?rev=262781&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.cpp Sat Mar 5 15:17:58 2016
@@ -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
Added: clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.h?rev=262781&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.h (added)
+++ clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.h Sat Mar 5 15:17:58 2016
@@ -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
Added: clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp?rev=262781&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp Sat Mar 5 15:17:58 2016
@@ -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
Added: clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.h?rev=262781&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.h (added)
+++ clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.h Sat Mar 5 15:17:58 2016
@@ -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
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst?rev=262781&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst Sat Mar 5 15:17:58 2016
@@ -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");
+ }
Modified: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp?rev=262781&r1=262780&r2=262781&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp Sat Mar 5 15:17:58 2016
@@ -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 &TrivialTypeRefe
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 Expens
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()
+}
More information about the cfe-commits
mailing list