[PATCH] D32346: [clang-tidy] New readability check for strlen argument

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

My thoughts on the check added.
Have you run it over a big codebase? What is the turnout?



================
Comment at: clang-tidy/readability/StrlenArgumentCheck.cpp:23
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(hasName("strlen"))),
+               hasAnyArgument(ignoringParenImpCasts(
----------------
please make it `::strlen` since its in the global namespace in c++. AFAIK it detects in C correctly.


================
Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:6
+
+This checker will detect addition in strlen() argument. Example code:
+
----------------
Could you please add a little more motivational text to it?
As I understand it, the wanted goal is to get the length of a substring, denoted as `char*`. Am I right?
You could give a more fully code example showing the equivalence.

`const char* = "Some super nice string"; ....`


================
Comment at: test/clang-tidy/readability-strlen-argument.cpp:1
+// RUN: %check_clang_tidy %s readability-strlen-argument %t
+
----------------
Same as documentation, maybe a little more telling examples to test on.
What happens with `char**`, an array of strings? Accessing those one by one would be possible with an addition or subscriptoperation.


Repository:
  rL LLVM

https://reviews.llvm.org/D32346