flx created this revision.
flx added a reviewer: alexfh.
flx added a subscriber: cfe-commits.
flx set the repository for this revision to rL LLVM.
Move code shared between UnnecessaryCopyInitialization and ForRangeCopyCheck
into utilities files.
Add more test cases for UnnecessaryCopyInitialization and disable fixes inside
of macros.
Repository:
rL LLVM
http://reviews.llvm.org/D17488
Files:
clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tidy/performance/UnnecessaryCopyInitialization.h
clang-tidy/utils/CMakeLists.txt
clang-tidy/utils/DeclRefExprUtils.cpp
clang-tidy/utils/DeclRefExprUtils.h
clang-tidy/utils/FixItHintUtils.cpp
clang-tidy/utils/FixItHintUtils.h
test/clang-tidy/performance-unnecessary-copy-initialization.cpp
Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -4,6 +4,7 @@
ExpensiveToCopyType() {}
virtual ~ExpensiveToCopyType() {}
const ExpensiveToCopyType &reference() const { return *this; }
+ void nonConstMethod() {}
};
struct TrivialToCopyType {
@@ -20,6 +21,11 @@
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 @@
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 @@
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()
+}
Index: clang-tidy/utils/FixItHintUtils.h
===================================================================
--- /dev/null
+++ clang-tidy/utils/FixItHintUtils.h
@@ -0,0 +1,30 @@
+//===--- 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 fix_it_hint_utils {
+
+/// \brief Creates fix to make VarDecl a reference by adding '&'.
+FixItHint createReferenceFix(const VarDecl &Var, ASTContext &Context);
+
+/// \brief Creates fix to make VarDecl const qualified.
+FixItHint createConstFix(const VarDecl &Var);
+
+} // namespace fix_it_hint_utils
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H
Index: clang-tidy/utils/FixItHintUtils.cpp
===================================================================
--- /dev/null
+++ clang-tidy/utils/FixItHintUtils.cpp
@@ -0,0 +1,33 @@
+//===--- 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 fix_it_hint_utils {
+
+FixItHint createReferenceFix(const VarDecl &Var, ASTContext &Context) {
+ SourceLocation AmpLocation = Var.getLocation();
+ auto Token = lexer_utils::getPreviousNonCommentToken(Context, AmpLocation);
+ if (!Token.is(tok::unknown))
+ AmpLocation = Token.getLocation().getLocWithOffset(Token.getLength());
+ return FixItHint::CreateInsertion(AmpLocation, "&");
+}
+
+FixItHint createConstFix(const VarDecl &Var) {
+ return FixItHint::CreateInsertion(Var.getTypeSpecStartLoc(), "const ");
+}
+
+} // namespace fix_it_hint_utils
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/utils/DeclRefExprUtils.h
===================================================================
--- /dev/null
+++ clang-tidy/utils/DeclRefExprUtils.h
@@ -0,0 +1,31 @@
+//===--- 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
Index: clang-tidy/utils/DeclRefExprUtils.cpp
===================================================================
--- /dev/null
+++ clang-tidy/utils/DeclRefExprUtils.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
Index: clang-tidy/utils/CMakeLists.txt
===================================================================
--- clang-tidy/utils/CMakeLists.txt
+++ clang-tidy/utils/CMakeLists.txt
@@ -1,6 +1,8 @@
set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyUtils
+ DeclRefExprUtils.cpp
+ FixItHintUtils.cpp
HeaderGuard.cpp
HeaderFileExtensionsUtils.cpp
IncludeInserter.cpp
Index: clang-tidy/performance/UnnecessaryCopyInitialization.h
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -16,7 +16,7 @@
namespace tidy {
namespace performance {
-// A check that detects const local variable declarations that are copy
+// A check that 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.
Index: clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -9,25 +9,22 @@
#include "UnnecessaryCopyInitialization.h"
-#include "../utils/LexerUtils.h"
+#include "../utils/DeclRefExprUtils.h"
+#include "../utils/FixItHintUtils.h"
#include "../utils/Matchers.h"
namespace clang {
namespace tidy {
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(
+ 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
@@ -41,30 +38,44 @@
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 << fix_it_hint_utils::createReferenceFix(*Var, *Result.Context);
+ if (!IsConstQualified)
+ Diagnostic << fix_it_hint_utils::createConstFix(*Var);
}
} // namespace performance
Index: clang-tidy/performance/ForRangeCopyCheck.cpp
===================================================================
--- clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -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,35 +66,26 @@
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);
+ << fix_it_hint_utils::createReferenceFix(LoopVar, Context);
if (!LoopVar.getType().isConstQualified())
- Diagnostic << createConstFix(LoopVar);
+ Diagnostic << fix_it_hint_utils::createConstFix(LoopVar);
return true;
}
bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
const VarDecl &LoopVar, const CXXForRangeStmt &ForRange,
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);
+ << fix_it_hint_utils::createConstFix(LoopVar)
+ << fix_it_hint_utils::createReferenceFix(LoopVar, Context);
return true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
