[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D44231#1039888, @hokein wrote:

> As this patch can catch some mistakes, I'm fine with checking it in. I agree that it is reasonable to write normal code like `sizeof(func_call())` (not false positive), maybe set the option to `false` by default?


I am okay with requiring people to enable this part of the check explicitly.



================
Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:220
+                 Result.Nodes.getNodeAs<Expr>("sizeof-integer-call")) {
+    diag(E->getLocStart(), "suspicious usage of 'sizeof(expr)' to an integer");
   } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
----------------
pfultz2 wrote:
> alexfh wrote:
> > I'm not sure I understand the message "suspicious usage of ... to an integer". Specifically, what does the "to an integer" part mean?
> That could probably be worded better. It means the `expr` is an integer type. Maybe I should say `suspicious usage of 'sizeof() on an expression that results in an integer`?
Missing a terminating quote around `sizeof`.


================
Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:28
+
+A common mistake is to query the ``sizeof`` on an integer or enum that
+represents the type, which will give the size of the integer and not of the
----------------
It's not obvious what "the type" refers to in this sentence. Also, it is incorrect to state that "A common mistake is to query the size of an integer" -- that's an incredibly common practice. You may need a bit more setup in the explanation to give the reader context.


https://reviews.llvm.org/D44231