[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> Have you run this over any large code bases to see whether the check triggers in practice?

I'm still curious about this, btw.



================
Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:45
+                  anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)))))),
+              callee(namedDecl(hasName("data"))))
+              .bind("call")))),
----------------
Should this check apply equally to `std::string::c_str()` as well as `std::string::data()`?


================
Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:57
+  const auto *Member = Result.Nodes.getNodeAs<MemberExpr>("member");
+  auto DiagBuilder = diag(Member->getMemberLoc(), "redundant call to .data()");
+  if (Member->isArrow()) {
----------------
I think the word "redundant" isn't going to be immediately obvious to a user who would write code like that. Perhaps `"accessing an element of the container does not require a call to 'data()'; did you mean to use 'operator[]()' instead?"`


================
Comment at: docs/clang-tidy/checks/readability-redundant-data-call.rst:7
+This check finds and suggests removing redundant ``.data()`` calls. Currently
+this covers calling ``.data()`` and immediately doing array subscript operation
+to obtain a single element, in which case simply calling ``operator[]`` suffice.
----------------
doing array subscript -> doing an array subscript


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45702