flx created this revision.
flx added reviewers: aaron.ballman, sbenza.
Herald added a subscriber: xazax.hun.
flx requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D97567
Files:
clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
clang-tools-extra/test/clang-tidy/checkers/performance-for-range-copy.cpp
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,37 @@
// 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/clang-tidy/utils/DeclRefExprUtils.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -46,14 +46,19 @@
ASTContext &Context) {
auto DeclRefToVar =
declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef");
+ auto MemberExprOfVar = memberExpr(has(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 opertor 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);
@@ -65,22 +70,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
