[PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:197
@@ +196,3 @@
+      this);
+}
+
----------------
What about this comment?

================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:203
@@ +202,3 @@
+  if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-constant")) {
+    diag(E->getLocStart(), "suspicious usage of 'sizeof(K)'; did you mean 'K'");
+  } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
----------------
nit: missing trailing question mark

================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:209
@@ +208,3 @@
+    diag(E->getLocStart(),
+         "suspicious usage of 'sizeof(char*)'; do you mean strlen?'");
+  } else if (const auto *E =
----------------
nit: 'strlen' should be in single quotes for consistency with other messages.

================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:257
@@ +256,3 @@
+                 Result.Nodes.getNodeAs<Expr>("sizeof-multiply-sizeof")) {
+    diag(E->getLocStart(), "suspicious multiplication of two 'sizeof'");
+  }
----------------
nit: I'd say `of 'sizeof' by 'sizeof'` instead of `of two 'sizeof'`.

================
Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:58
@@ +57,3 @@
+
+warning: suspicious usage of 'sizeof(A*)'
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
----------------
nit: the titles need to be consistent - either all have or not have `warning:`.

I'd also make the first letters capital.

================
Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:102
@@ +101,3 @@
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Multiplying ``sizeof`` expressions typically make no sense and is probably a
+logic error. In the following example, the programmer used ``*`` instead of
----------------
s/make/makes/


http://reviews.llvm.org/D19014