xgupta updated this revision to Diff 543307.
xgupta added a comment.
Address comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97567/new/
https://reviews.llvm.org/D97567
Files:
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
Index: clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
@@ -114,8 +114,8 @@
(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 @@
(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 @@
(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 @@
(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);
}
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
@@ -843,3 +843,36 @@
}
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);
+ }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/performance/for-range-copy.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance/for-range-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance/for-range-copy.cpp
@@ -296,3 +296,38 @@
// 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);
+ }
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -432,6 +432,14 @@
`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 @@
<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.
Index: clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -43,14 +43,19 @@
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 @@
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
