[clang-tidy] Check for sizeof that call functions

Summary:
A common mistake that I have found in our codebase is calling a function to get an integer or enum that represents the type such as:

```
int numBytes = numElements * sizeof(x.GetType());
```

So this extends the `sizeof` check to check for these cases. There is also a `WarnOnSizeOfCall` option so it can be disabled.

Patch by Paul Fultz II!

Reviewers: hokein, alexfh, aaron.ballman, ilya-biryukov

Reviewed By: alexfh

Subscribers: lebedev.ri, xazax.hun, jkorous-apple, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D44231

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@329073 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 20 insertions, 0 deletions

diff --git a/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index d389ff15..f05a9006 100644
--- a/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -62,12 +62,16 @@ SizeofExpressionCheck::SizeofExpressionCheck(StringRef Name,
                                              ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       WarnOnSizeOfConstant(Options.get("WarnOnSizeOfConstant", 1) != 0),
+      WarnOnSizeOfIntegerExpression(
+          Options.get("WarnOnSizeOfIntegerExpression", 0) != 0),
       WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", 1) != 0),
       WarnOnSizeOfCompareToConstant(
           Options.get("WarnOnSizeOfCompareToConstant", 1) != 0) {}
 
 void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
+  Options.store(Opts, "WarnOnSizeOfIntegerExpression",
+                WarnOnSizeOfIntegerExpression);
   Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
   Options.store(Opts, "WarnOnSizeOfCompareToConstant",
                 WarnOnSizeOfCompareToConstant);
@@ -78,6 +82,9 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
   const auto ConstantExpr = expr(ignoringParenImpCasts(
       anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
             binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr)))));
+  const auto IntegerCallExpr = expr(ignoringParenImpCasts(
+      callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
+               unless(isInTemplateInstantiation()))));
   const auto SizeOfExpr =
       expr(anyOf(sizeOfExpr(has(type())), sizeOfExpr(has(expr()))));
   const auto SizeOfZero = expr(
@@ -94,6 +101,14 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
         this);
   }
 
+  // Detect sizeof(f())
+  if (WarnOnSizeOfIntegerExpression) {
+    Finder->addMatcher(
+        expr(sizeOfExpr(ignoringParenImpCasts(has(IntegerCallExpr))))
+            .bind("sizeof-integer-call"),
+        this);
+  }
+
   // Detect expression like: sizeof(this);
   if (WarnOnSizeOfThis) {
     Finder->addMatcher(
@@ -203,6 +218,10 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
   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-integer-call")) {
+    diag(E->getLocStart(), "suspicious usage of 'sizeof()' on an expression "
+                           "that results in an integer");
   } else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
     diag(E->getLocStart(),
          "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'");
diff --git a/clang-tidy/bugprone/SizeofExpressionCheck.h b/clang-tidy/bugprone/SizeofExpressionCheck.h
index 2811b5af..8e14c315 100644
--- a/clang-tidy/bugprone/SizeofExpressionCheck.h
+++ b/clang-tidy/bugprone/SizeofExpressionCheck.h
@@ -29,6 +29,7 @@ public:
 
 private:
   const bool WarnOnSizeOfConstant;
+  const bool WarnOnSizeOfIntegerExpression;
   const bool WarnOnSizeOfThis;
   const bool WarnOnSizeOfCompareToConstant;
 };