[clang-tools-extra] r329073 - [clang-tidy] Check for sizeof that call functions
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 3 08:10:24 PDT 2018
Author: hokein
Date: Tue Apr 3 08:10:24 2018
New Revision: 329073
URL: http://llvm.org/viewvc/llvm-project?rev=329073&view=rev
Log:
[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
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp?rev=329073&r1=329072&r2=329073&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp Tue Apr 3 08:10:24 2018
@@ -62,12 +62,16 @@ SizeofExpressionCheck::SizeofExpressionC
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::registerMatc
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::registerMatc
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
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)'");
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h?rev=329073&r1=329072&r2=329073&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h Tue Apr 3 08:10:24 2018
@@ -29,6 +29,7 @@ public:
private:
const bool WarnOnSizeOfConstant;
+ const bool WarnOnSizeOfIntegerExpression;
const bool WarnOnSizeOfThis;
const bool WarnOnSizeOfCompareToConstant;
};
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst?rev=329073&r1=329072&r2=329073&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst Tue Apr 3 08:10:24 2018
@@ -22,6 +22,36 @@ programmer was probably to simply get th
char buf[BUFLEN];
memset(buf, 0, sizeof(BUFLEN)); // sizeof(42) ==> sizeof(int)
+Suspicious usage of 'sizeof(expr)'
+----------------------------------
+
+In cases, where there is an enum or integer to represent a type, a common
+mistake is to query the ``sizeof`` on the integer or enum that represents the
+type that should be used by ``sizeof``. This results in the size of the integer
+and not of the type the integer represents:
+
+.. code-block:: c++
+
+ enum data_type {
+ FLOAT_TYPE,
+ DOUBLE_TYPE
+ };
+
+ struct data {
+ data_type type;
+ void* buffer;
+ data_type get_type() {
+ return type;
+ }
+ };
+
+ void f(data d, int numElements) {
+ // should be sizeof(float) or sizeof(double), depending on d.get_type()
+ int numBytes = numElements * sizeof(d.get_type());
+ ...
+ }
+
+
Suspicious usage of 'sizeof(this)'
----------------------------------
@@ -142,6 +172,11 @@ Options
When non-zero, the check will warn on an expression like
``sizeof(CONSTANT)``. Default is `1`.
+.. option:: WarnOnSizeOfIntegerExpression
+
+ When non-zero, the check will warn on an expression like ``sizeof(expr)``
+ where the expression results in an integer. Default is `0`.
+
.. option:: WarnOnSizeOfThis
When non-zero, the check will warn on an expression like ``sizeof(this)``.
Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp?rev=329073&r1=329072&r2=329073&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp Tue Apr 3 08:10:24 2018
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t
+// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t -- -config="{CheckOptions: [{key: bugprone-sizeof-expression.WarnOnSizeOfIntegerExpression, value: 1}]}" --
class C {
int size() { return sizeof(this); }
@@ -14,6 +14,62 @@ extern short B[10];
#pragma pack(1)
struct S { char a, b, c; };
+enum E { E_VALUE = 0 };
+enum class EC { VALUE = 0 };
+
+bool AsBool() { return false; }
+int AsInt() { return 0; }
+E AsEnum() { return E_VALUE; }
+EC AsEnumClass() { return EC::VALUE; }
+S AsStruct() { return {}; }
+
+struct M {
+ int AsInt() { return 0; }
+ E AsEnum() { return E_VALUE; }
+ S AsStruct() { return {}; }
+};
+
+int ReturnOverload(int) { return {}; }
+S ReturnOverload(S) { return {}; }
+
+template <class T>
+T ReturnTemplate(T) { return {}; }
+
+template <class T>
+bool TestTrait1() {
+ return sizeof(ReturnOverload(T{})) == sizeof(A);
+}
+
+template <class T>
+bool TestTrait2() {
+ return sizeof(ReturnTemplate(T{})) == sizeof(A);
+}
+
+template <class T>
+bool TestTrait3() {
+ return sizeof(ReturnOverload(0)) == sizeof(T{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+}
+
+template <class T>
+bool TestTrait4() {
+ return sizeof(ReturnTemplate(0)) == sizeof(T{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+}
+
+bool TestTemplates() {
+ bool b = true;
+ b &= TestTrait1<int>();
+ b &= TestTrait1<S>();
+ b &= TestTrait2<int>();
+ b &= TestTrait2<S>();
+ b &= TestTrait3<int>();
+ b &= TestTrait3<S>();
+ b &= TestTrait4<int>();
+ b &= TestTrait4<S>();
+ return b;
+}
+
int Test1(const char* ptr) {
int sum = 0;
sum += sizeof(LEN);
@@ -22,6 +78,18 @@ int Test1(const char* ptr) {
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
sum += sizeof(sum, LEN);
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+ sum += sizeof(AsBool());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(AsInt());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(AsEnum());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(AsEnumClass());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(M{}.AsInt());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+ sum += sizeof(M{}.AsEnum());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
sum += sizeof(sizeof(X));
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
sum += sizeof(LEN + sizeof(X));
@@ -171,6 +239,8 @@ int ValidExpressions() {
if (sizeof(A) < 10)
sum += sizeof(A);
sum += sizeof(int);
+ sum += sizeof(AsStruct());
+ sum += sizeof(M{}.AsStruct());
sum += sizeof(A[sizeof(A) / sizeof(int)]);
sum += sizeof(&A[sizeof(A) / sizeof(int)]);
sum += sizeof(sizeof(0)); // Special case: sizeof size_t.
More information about the cfe-commits
mailing list