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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 16 05:36:20 PDT 2018

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.


More information about the cfe-commits mailing list