flx removed rL LLVM as the repository for this revision.
flx updated this revision to Diff 56573.
http://reviews.llvm.org/D19865
Files:
clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tidy/performance/UnnecessaryCopyInitialization.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
@@ -338,3 +338,10 @@
WeirdCopyCtorType neg_weird_1(orig, false);
WeirdCopyCtorType neg_weird_2(orig, true);
}
+
+void WarningOnlyMultiDeclStmt() {
+ ExpensiveToCopyType orig;
+ ExpensiveToCopyType copy = orig, copy2;
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy' of the variable 'orig' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
+ // CHECK-FIXES: ExpensiveToCopyType copy = orig, copy2;
+}
Index: clang-tidy/performance/UnnecessaryCopyInitialization.h
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -33,9 +33,10 @@
private:
void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt,
- ASTContext &Context);
+ bool IssueFix, ASTContext &Context);
void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar,
- const Stmt &BlockStmt, ASTContext &Context);
+ const Stmt &BlockStmt, bool IssueFix,
+ ASTContext &Context);
};
} // namespace performance
Index: clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -20,10 +20,6 @@
void recordFixes(const VarDecl &Var, ASTContext &Context,
DiagnosticBuilder &Diagnostic) {
- // Do not propose fixes in macros since we cannot place them correctly.
- if (Var.getLocation().isMacroID())
- return;
-
Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context);
if (!Var.getType().isLocalConstQualified())
Diagnostic << utils::fixit::changeVarDeclToConst(Var);
@@ -57,14 +53,16 @@
auto localVarCopiedFrom = [](const internal::Matcher<Expr> &CopyCtorArg) {
return compoundStmt(
forEachDescendant(
- varDecl(hasLocalStorage(),
- hasType(matchers::isExpensiveToCopy()),
- hasInitializer(cxxConstructExpr(
- hasDeclaration(cxxConstructorDecl(
- isCopyConstructor())),
- hasArgument(0, CopyCtorArg))
- .bind("ctorCall")))
- .bind("newVarDecl")))
+ declStmt(
+ has(varDecl(hasLocalStorage(),
+ hasType(matchers::isExpensiveToCopy()),
+ hasInitializer(
+ cxxConstructExpr(
+ hasDeclaration(cxxConstructorDecl(
+ isCopyConstructor())),
+ hasArgument(0, CopyCtorArg))
+ .bind("ctorCall")))
+ .bind("newVarDecl"))).bind("declStmt")))
.bind("blockStmt");
};
@@ -84,6 +82,11 @@
const auto *OldVar = Result.Nodes.getNodeAs<VarDecl>("oldVarDecl");
const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
+ // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros
+ // since we cannot place them correctly.
+ bool IssueFix =
+ Result.Nodes.getNodeAs<DeclStmt>("declStmt")->isSingleDecl() &&
+ !NewVar->getLocation().isMacroID();
// A constructor that looks like T(const T& t, bool arg = false) counts as a
// copy only when it is called with default arguments for the arguments after
@@ -93,14 +96,16 @@
return;
if (OldVar == nullptr) {
- handleCopyFromMethodReturn(*NewVar, *BlockStmt, *Result.Context);
+ handleCopyFromMethodReturn(*NewVar, *BlockStmt, IssueFix, *Result.Context);
} else {
- handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, *Result.Context);
+ handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, IssueFix,
+ *Result.Context);
}
}
void UnnecessaryCopyInitialization::handleCopyFromMethodReturn(
- const VarDecl &Var, const Stmt &BlockStmt, ASTContext &Context) {
+ const VarDecl &Var, const Stmt &BlockStmt, bool IssueFix,
+ ASTContext &Context) {
bool IsConstQualified = Var.getType().isConstQualified();
if (!IsConstQualified && !isOnlyUsedAsConst(Var, BlockStmt, Context))
return;
@@ -114,21 +119,23 @@
"const reference but is only used as const "
"reference; consider making it a const reference")
<< &Var;
- recordFixes(Var, Context, Diagnostic);
+ if (IssueFix)
+ recordFixes(Var, Context, Diagnostic);
}
void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
const VarDecl &NewVar, const VarDecl &OldVar, const Stmt &BlockStmt,
- ASTContext &Context) {
+ bool IssueFix, ASTContext &Context) {
if (!isOnlyUsedAsConst(NewVar, BlockStmt, Context) ||
!isOnlyUsedAsConst(OldVar, BlockStmt, Context))
return;
auto Diagnostic = diag(NewVar.getLocation(),
"local copy %0 of the variable %1 is never modified; "
"consider avoiding the copy")
<< &NewVar << &OldVar;
- recordFixes(NewVar, Context, Diagnostic);
+ if (IssueFix)
+ recordFixes(NewVar, Context, Diagnostic);
}
} // namespace performance
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
