[PATCH] D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.

[Shivam Gupta via Phabricator via cfe-commits]

xgupta updated this revision to Diff 543282.
xgupta added a comment.

Rebase and address one last comment.

I think now it will be fine to commit?


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/test/clang-tidy/checkers/performance/for-range-copy.cpp
  
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
@@ -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,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);
+  }
+}
\ No newline at end of file
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,18 @@
                            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)),
+      findAll(expr(anyOf(cxxMemberCallExpr(ConstMethodCallee,
+                            on(DeclRefToVarOrMemberExprOfVar)),
                          cxxOperatorCallExpr(ConstMethodCallee,
-                                             hasArgument(0, DeclRefToVar))))),
+                                             hasArgument(0, DeclRefToVarOrMemberExprOfVar))))),
       Stmt, Context);
   SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
@@ -62,21 +66,22 @@
       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)))))),
+                            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)))))))),
+                    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