diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp --- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp +++ b/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 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; } diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/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 + ` + check by extending const usage analysis to include the type's members. + +- Improved :doc:`performance-inefficient-vector-operation + ` + check by extending const usage analysis to include the type's members. + - Improved :doc:`performance-move-const-arg ` check to warn when move special member functions are not available. @@ -445,6 +453,14 @@ ` checker that was causing false-positives when the move constructor or move assign operator were defaulted. +- Improved :doc:`performance-unnecessary-copy-initialization + ` + check by extending const usage analysis to include the type's members. + +- Improved :doc:`performance-unnecessary-value-param + ` + check by extending const usage analysis to include the type's members. + - Improved :doc:`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 --- 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 @@ // SS : createView(*ValueReturningIterator())) { } } + +void positiveConstMemberExpr() { + struct Struct { + Mutable Member; + }; + for (Struct SS : View>()) { + // CHECK-MESSAGES: [[@LINE-1]]:15: warning: loop variable is copied + // CHECK-FIXES: for (const Struct& SS : View>()) { + 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>()) { + SS.Member.setBool(true); + } + for (Struct SS : View>()) { + SS.Member[1]; + } + for (Struct SS : View>()) { + mutate(SS.Member); + } + for (Struct SS : View>()) { + 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 --- 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 instantiatePositiveSingleTemplateType() { positiveSingleTemplateType(); } + +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 --- a/clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp +++ b/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); }