flx updated this revision to Diff 352233.
flx marked an inline comment as done.
flx added a comment.
Directly examine initializer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103021/new/
https://reviews.llvm.org/D103021
Files:
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -1,9 +1,19 @@
// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
+template <typename T>
+struct Iterator {
+ void operator++();
+ const T &operator*() const;
+ bool operator!=(const Iterator &) const;
+ typedef const T &const_reference;
+};
+
struct ExpensiveToCopyType {
ExpensiveToCopyType();
virtual ~ExpensiveToCopyType();
const ExpensiveToCopyType &reference() const;
+ Iterator<ExpensiveToCopyType> begin() const;
+ Iterator<ExpensiveToCopyType> end() const;
void nonConstMethod();
bool constMethod() const;
};
@@ -508,3 +518,41 @@
// CHECK-FIXES: const auto& UnnecessaryCopy = Ref.reference();
Orig.constMethod();
}
+
+void negativeloopedOverObjectIsModified() {
+ ExpensiveToCopyType Orig;
+ for (const auto &Element : Orig) {
+ const auto Copy = Element;
+ Orig.nonConstMethod();
+ Copy.constMethod();
+ }
+
+ auto Lambda = []() {
+ ExpensiveToCopyType Orig;
+ for (const auto &Element : Orig) {
+ const auto Copy = Element;
+ Orig.nonConstMethod();
+ Copy.constMethod();
+ }
+ };
+}
+
+void negativeReferenceIsInitializedOutsideOfBlock() {
+ ExpensiveToCopyType Orig;
+ const auto &E2 = Orig;
+ if (1 != 2 * 3) {
+ const auto C2 = E2;
+ Orig.nonConstMethod();
+ C2.constMethod();
+ }
+
+ auto Lambda = []() {
+ ExpensiveToCopyType Orig;
+ const auto &E2 = Orig;
+ if (1 != 2 * 3) {
+ const auto C2 = E2;
+ Orig.nonConstMethod();
+ C2.constMethod();
+ }
+ };
+}
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
===================================================================
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_INITIALIZATION_H
#include "../ClangTidyCheck.h"
+#include "clang/AST/Decl.h"
namespace clang {
namespace tidy {
@@ -35,11 +36,12 @@
private:
void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt,
- bool IssueFix, const VarDecl *ObjectArg,
+ const FunctionDecl &Func, bool IssueFix,
+ const VarDecl *ObjectArg,
ASTContext &Context);
void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar,
- const Stmt &BlockStmt, bool IssueFix,
- ASTContext &Context);
+ const Stmt &BlockStmt, const FunctionDecl &Func,
+ bool IssueFix, ASTContext &Context);
const std::vector<std::string> AllowedTypes;
};
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -12,6 +12,7 @@
#include "../utils/FixItHintUtils.h"
#include "../utils/Matchers.h"
#include "../utils/OptionsUtils.h"
+#include "clang/AST/Decl.h"
#include "clang/Basic/Diagnostic.h"
namespace clang {
@@ -61,11 +62,11 @@
AST_MATCHER_FUNCTION(StatementMatcher, isInitializedFromReferenceToConst) {
auto OldVarDeclRef =
declRefExpr(to(varDecl(hasLocalStorage()).bind(OldVarDeclId)));
- return declStmt(has(varDecl(hasInitializer(
+ return expr(
anyOf(isConstRefReturningFunctionCall(), isConstRefReturningMethodCall(),
ignoringImpCasts(OldVarDeclRef),
- ignoringImpCasts(unaryOperator(
- hasOperatorName("&"), hasUnaryOperand(OldVarDeclRef))))))));
+ ignoringImpCasts(unaryOperator(hasOperatorName("&"),
+ hasUnaryOperand(OldVarDeclRef)))));
}
// This checks that the variable itself is only used as const, and also makes
@@ -82,6 +83,7 @@
// object arg or variable that is referenced is immutable as well.
static bool isInitializingVariableImmutable(const VarDecl &InitializingVar,
const Stmt &BlockStmt,
+ const FunctionDecl &Func,
ASTContext &Context) {
if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context))
return false;
@@ -92,28 +94,24 @@
if (!isa<ReferenceType, PointerType>(T))
return true;
- auto Matches =
- match(findAll(declStmt(has(varDecl(equalsNode(&InitializingVar))))
- .bind("declStmt")),
- BlockStmt, Context);
- // The reference or pointer is not initialized in the BlockStmt. We assume
- // its pointee is not modified then.
- if (Matches.empty())
+ // The reference or pointer is not declared and hence not initialized anywhere
+ // in the function. We assume its pointee is not modified then.
+ if (!InitializingVar.isLocalVarDecl() || !InitializingVar.hasInit()) {
return true;
+ }
- const auto *Initialization = selectFirst<DeclStmt>("declStmt", Matches);
- Matches =
- match(isInitializedFromReferenceToConst(), *Initialization, Context);
+ auto Matches = match(isInitializedFromReferenceToConst(),
+ *InitializingVar.getInit(), Context);
// The reference is initialized from a free function without arguments
// returning a const reference. This is a global immutable object.
if (selectFirst<CallExpr>(InitFunctionCallId, Matches) != nullptr)
return true;
// Check that the object argument is immutable as well.
if (const auto *OrigVar = selectFirst<VarDecl>(ObjectArgId, Matches))
- return isInitializingVariableImmutable(*OrigVar, BlockStmt, Context);
+ return isInitializingVariableImmutable(*OrigVar, BlockStmt, Func, Context);
// Check that the old variable we reference is immutable as well.
if (const auto *OrigVar = selectFirst<VarDecl>(OldVarDeclId, Matches))
- return isInitializingVariableImmutable(*OrigVar, BlockStmt, Context);
+ return isInitializingVariableImmutable(*OrigVar, BlockStmt, Func, Context);
return false;
}
@@ -131,6 +129,7 @@
return compoundStmt(
forEachDescendant(
declStmt(
+ hasAncestor(functionDecl().bind("containingFunc")),
has(varDecl(hasLocalStorage(),
hasType(qualType(
hasCanonicalType(allOf(
@@ -169,6 +168,7 @@
const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>(ObjectArgId);
const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
+ const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("containingFunc");
TraversalKindScope RAII(*Result.Context, TK_AsIs);
@@ -186,22 +186,22 @@
return;
if (OldVar == nullptr) {
- handleCopyFromMethodReturn(*NewVar, *BlockStmt, IssueFix, ObjectArg,
+ handleCopyFromMethodReturn(*NewVar, *BlockStmt, *Func, IssueFix, ObjectArg,
*Result.Context);
} else {
- handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, IssueFix,
+ handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, *Func, IssueFix,
*Result.Context);
}
}
void UnnecessaryCopyInitialization::handleCopyFromMethodReturn(
- const VarDecl &Var, const Stmt &BlockStmt, bool IssueFix,
- const VarDecl *ObjectArg, ASTContext &Context) {
+ const VarDecl &Var, const Stmt &BlockStmt, const FunctionDecl &Func,
+ bool IssueFix, const VarDecl *ObjectArg, ASTContext &Context) {
bool IsConstQualified = Var.getType().isConstQualified();
if (!IsConstQualified && !isOnlyUsedAsConst(Var, BlockStmt, Context))
return;
if (ObjectArg != nullptr &&
- !isInitializingVariableImmutable(*ObjectArg, BlockStmt, Context))
+ !isInitializingVariableImmutable(*ObjectArg, BlockStmt, Func, Context))
return;
auto Diagnostic =
@@ -216,9 +216,9 @@
void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
const VarDecl &NewVar, const VarDecl &OldVar, const Stmt &BlockStmt,
- bool IssueFix, ASTContext &Context) {
+ const FunctionDecl &Func, bool IssueFix, ASTContext &Context) {
if (!isOnlyUsedAsConst(NewVar, BlockStmt, Context) ||
- !isInitializingVariableImmutable(OldVar, BlockStmt, Context))
+ !isInitializingVariableImmutable(OldVar, BlockStmt, Func, Context))
return;
auto Diagnostic = diag(NewVar.getLocation(),
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
