flx updated this revision to Diff 351425.
flx marked an inline comment as not done.
flx added a comment.
Use more efficient method to check for local variable declaration.
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 {
@@ -68,6 +69,16 @@
hasOperatorName("&"), hasUnaryOperand(OldVarDeclRef))))))));
}
+bool isDeclaredInFunction(const VarDecl &Var, const DeclContext &Func) {
+ const DeclContext *DC = Var.getDeclContext();
+ while (DC != nullptr) {
+ if (DC == &Func)
+ return true;
+ DC = DC->getLexicalParent();
+ }
+ return false;
+}
+
// This checks that the variable itself is only used as const, and also makes
// sure that it does not reference another variable that could be modified in
// the BlockStmt. It does this by checking the following:
@@ -82,6 +93,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,12 +104,20 @@
if (!isa<ReferenceType, PointerType>(T))
return true;
+ // The reference or pointer is not declared and hence not initialized anywhere
+ // in the function. We assume its pointee is not modified then.
+ if (!isDeclaredInFunction(InitializingVar, Func)) {
+ return true;
+ }
+
+ // Search the whole function body, not just the current block statement, for
+ // the decl statement of the initialization variable.
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.
+ *Func.getBody(), Context);
+ // The reference or pointer is not initialized anywhere witin the function. We
+ // assume its pointee is not modified then.
if (Matches.empty())
return true;
@@ -110,10 +130,10 @@
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 +151,7 @@
return compoundStmt(
forEachDescendant(
declStmt(
+ hasAncestor(functionDecl().bind("containingFunc")),
has(varDecl(hasLocalStorage(),
hasType(qualType(
hasCanonicalType(allOf(
@@ -169,6 +190,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 +208,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 +238,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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
