[clang-tools-extra] 1e51268 - [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.
Shivam Gupta via cfe-commits
cfe-commits at lists.llvm.org
Sun Jul 23 11:38:38 PDT 2023
Author: Shivam Gupta
Date: 2023-07-24T00:08:29+05:30
New Revision: 1e512688376c83d96f097e9b0ddb19132247a646
URL: https://github.com/llvm/llvm-project/commit/1e512688376c83d96f097e9b0ddb19132247a646
DIFF: https://github.com/llvm/llvm-project/commit/1e512688376c83d96f097e9b0ddb19132247a646.diff
LOG: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.
Until now when determining all the const uses of a VarDecl we only considered
how the variable itself was used. This change extends checking for const usages
of the type's members as well.
This increases the number of true positives for various performance checks that
share the same const usage analysis.
Path by Felix Berger
Reviewed By: njames93, PiotrZSL
Differential Revision: https://reviews.llvm.org/D97567
Added:
Modified:
clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/performance/for-range-copy.cpp
clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
index 891529997787db..2d73179150e8b8 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -43,14 +43,19 @@ constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
ASTContext &Context) {
auto DeclRefToVar =
declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef");
+ auto MemberExprOfVar = memberExpr(hasObjectExpression(DeclRefToVar));
+ auto DeclRefToVarOrMemberExprOfVar =
+ stmt(anyOf(DeclRefToVar, MemberExprOfVar));
auto ConstMethodCallee = callee(cxxMethodDecl(isConst()));
// Match method call expressions where the variable is referenced as the this
// implicit object argument and operator 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))))),
+ findAll(expr(anyOf(
+ cxxMemberCallExpr(ConstMethodCallee,
+ on(DeclRefToVarOrMemberExprOfVar)),
+ cxxOperatorCallExpr(ConstMethodCallee,
+ hasArgument(0, DeclRefToVarOrMemberExprOfVar))))),
Stmt, Context);
SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
extractNodesByIdTo(Matches, "declRef", DeclRefs);
@@ -62,22 +67,23 @@ constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
ConstReferenceOrValue,
substTemplateTypeParmType(hasReplacementType(ConstReferenceOrValue))));
auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
- DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValueOrReplaced)));
+ DeclRefToVarOrMemberExprOfVar,
+ parmVarDecl(hasType(ConstReferenceOrValueOrReplaced)));
Matches = match(findAll(invocation(UsedAsConstRefOrValueArg)), Stmt, Context);
extractNodesByIdTo(Matches, "declRef", DeclRefs);
// References and pointers to const assignments.
- Matches =
- match(findAll(declStmt(
- has(varDecl(hasType(qualType(matchers::isReferenceToConst())),
- hasInitializer(ignoringImpCasts(DeclRefToVar)))))),
- Stmt, Context);
+ Matches = match(
+ findAll(declStmt(has(varDecl(
+ hasType(qualType(matchers::isReferenceToConst())),
+ hasInitializer(ignoringImpCasts(DeclRefToVarOrMemberExprOfVar)))))),
+ Stmt, Context);
extractNodesByIdTo(Matches, "declRef", DeclRefs);
- Matches =
- match(findAll(declStmt(has(varDecl(
- hasType(qualType(matchers::isPointerToConst())),
- hasInitializer(ignoringImpCasts(unaryOperator(
- hasOperatorName("&"), hasUnaryOperand(DeclRefToVar)))))))),
- Stmt, Context);
+ Matches = match(findAll(declStmt(has(varDecl(
+ hasType(qualType(matchers::isPointerToConst())),
+ hasInitializer(ignoringImpCasts(unaryOperator(
+ hasOperatorName("&"),
+ hasUnaryOperand(DeclRefToVarOrMemberExprOfVar)))))))),
+ Stmt, Context);
extractNodesByIdTo(Matches, "declRef", DeclRefs);
return DeclRefs;
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 2cc0010884a7a0..53b987ccab42e7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -432,6 +432,14 @@ Changes in existing checks
`IgnoreTemplateInstantiations` option to optionally ignore virtual function
overrides that are part of template instantiations.
+- Improved :doc:`performance-for-range-copy
+ <clang-tidy/checks/performance/for-range-copy>`
+ check by extending const usage analysis to include the type's members.
+
+- Improved :doc:`performance-inefficient-vector-operation
+ <clang-tidy/checks/performance/inefficient-vector-operation>`
+ check by extending const usage analysis to include the type's members.
+
- Improved :doc:`performance-move-const-arg
<clang-tidy/checks/performance/move-const-arg>` check to warn when move
special member functions are not available.
@@ -445,6 +453,14 @@ Changes in existing checks
<clang-tidy/checks/performance/noexcept-move-constructor>` checker that was causing
false-positives when the move constructor or move assign operator were defaulted.
+- Improved :doc:`performance-unnecessary-copy-initialization
+ <clang-tidy/checks/performance/unnecessary-copy-initialization>`
+ check by extending const usage analysis to include the type's members.
+
+- Improved :doc:`performance-unnecessary-value-param
+ <clang-tidy/checks/performance/unnecessary-value-param>`
+ check by extending const usage analysis to include the type's members.
+
- Improved :doc:`readability-container-data-pointer
<clang-tidy/checks/readability/container-data-pointer>` check with new
`IgnoredContainers` option to ignore some containers.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/for-range-copy.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/for-range-copy.cpp
index 07b5116dfb1480..1a2eedc9e65c53 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/for-range-copy.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/for-range-copy.cpp
@@ -296,3 +296,38 @@ void positiveValueIteratorUsedElseWhere() {
// SS : createView(*ValueReturningIterator<S>())) {
}
}
+
+void positiveConstMemberExpr() {
+ struct Struct {
+ Mutable Member;
+ };
+ for (Struct SS : View<Iterator<Struct>>()) {
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: loop variable is copied
+ // CHECK-FIXES: for (const Struct& SS : View<Iterator<Struct>>()) {
+ auto MemberCopy = SS.Member;
+ const auto &ConstRef = SS.Member;
+ bool b = SS.Member.constMethod();
+ use(SS.Member);
+ useByConstValue(SS.Member);
+ useByValue(SS.Member);
+ }
+}
+
+void negativeNonConstMemberExpr() {
+ struct Struct {
+ Mutable Member;
+ };
+ for (Struct SS : View<Iterator<Struct>>()) {
+ SS.Member.setBool(true);
+ }
+ for (Struct SS : View<Iterator<Struct>>()) {
+ SS.Member[1];
+ }
+ for (Struct SS : View<Iterator<Struct>>()) {
+ mutate(SS.Member);
+ }
+ for (Struct SS : View<Iterator<Struct>>()) {
+ mutate(&SS.Member);
+ }
+}
+
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
index 2491aab86f1ad2..049ba64d6aedeb 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
@@ -843,3 +843,36 @@ void positiveSingleTemplateType() {
}
void instantiatePositiveSingleTemplateType() { positiveSingleTemplateType<ExpensiveToCopyType>(); }
+
+struct Struct {
+ ExpensiveToCopyType Member;
+};
+
+void positiveConstMemberExpr() {
+ Struct Orig;
+ auto UC = Orig;
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: local copy 'UC'
+ // CHECK-FIXES: const auto& UC = Orig;
+ const auto &ConstRef = UC.Member;
+ auto MemberCopy = UC.Member;
+ bool b = UC.Member.constMethod();
+ useByValue(UC.Member);
+ useAsConstReference(UC.Member);
+ useByValue(UC.Member);
+}
+
+void negativeNonConstMemberExpr() {
+ Struct Orig;
+ {
+ auto Copy = Orig;
+ Copy.Member.nonConstMethod();
+ }
+ {
+ auto Copy = Orig;
+ mutate(Copy.Member);
+ }
+ {
+ auto Copy = Orig;
+ mutate(&Copy.Member);
+ }
+}
diff --git a/clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp b/clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
index d47add410b2cb8..a653b11faad282 100644
--- a/clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
@@ -114,8 +114,8 @@ TEST(ConstReferenceDeclRefExprsTest, ConstValueVar) {
(void)*⌖
S copy1 = /*const*/target;
S copy2(/*const*/target);
- useInt(target.int_member);
- useIntConstRef(target.int_member);
+ useInt(/*const*/target.int_member);
+ useIntConstRef(/*const*/target.int_member);
useIntPtr(target.ptr_member);
useIntConstPtr(&target.int_member);
}
@@ -140,8 +140,8 @@ TEST(ConstReferenceDeclRefExprsTest, ConstRefVar) {
(void)*⌖
S copy1 = /*const*/target;
S copy2(/*const*/target);
- useInt(target.int_member);
- useIntConstRef(target.int_member);
+ useInt(/*const*/target.int_member);
+ useIntConstRef(/*const*/target.int_member);
useIntPtr(target.ptr_member);
useIntConstPtr(&target.int_member);
}
@@ -172,8 +172,8 @@ TEST(ConstReferenceDeclRefExprsTest, ValueVar) {
(void)*⌖
S copy1 = /*const*/target;
S copy2(/*const*/target);
- useInt(target.int_member);
- useIntConstRef(target.int_member);
+ useInt(/*const*/target.int_member);
+ useIntConstRef(/*const*/target.int_member);
useIntPtr(target.ptr_member);
useIntConstPtr(&target.int_member);
}
@@ -199,8 +199,8 @@ TEST(ConstReferenceDeclRefExprsTest, RefVar) {
(void)*⌖
S copy1 = /*const*/target;
S copy2(/*const*/target);
- useInt(target.int_member);
- useIntConstRef(target.int_member);
+ useInt(/*const*/target.int_member);
+ useIntConstRef(/*const*/target.int_member);
useIntPtr(target.ptr_member);
useIntConstPtr(&target.int_member);
}
More information about the cfe-commits
mailing list