pfultz2 updated this revision to Diff 140790.
pfultz2 added a comment.
I have rebased.
https://reviews.llvm.org/D44231
Files:
clang-tidy/bugprone/SizeofExpressionCheck.cpp
clang-tidy/bugprone/SizeofExpressionCheck.h
docs/clang-tidy/checks/bugprone-sizeof-expression.rst
test/clang-tidy/bugprone-sizeof-expression.cpp
Index: test/clang-tidy/bugprone-sizeof-expression.cpp
===================================================================
--- test/clang-tidy/bugprone-sizeof-expression.cpp
+++ test/clang-tidy/bugprone-sizeof-expression.cpp
@@ -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,14 +14,82 @@
#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);
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
sum += sizeof(LEN + 1);
// 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 @@
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.
Index: docs/clang-tidy/checks/bugprone-sizeof-expression.rst
===================================================================
--- docs/clang-tidy/checks/bugprone-sizeof-expression.rst
+++ docs/clang-tidy/checks/bugprone-sizeof-expression.rst
@@ -22,6 +22,36 @@
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 @@
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)``.
Index: clang-tidy/bugprone/SizeofExpressionCheck.h
===================================================================
--- clang-tidy/bugprone/SizeofExpressionCheck.h
+++ clang-tidy/bugprone/SizeofExpressionCheck.h
@@ -29,6 +29,7 @@
private:
const bool WarnOnSizeOfConstant;
+ const bool WarnOnSizeOfIntegerExpression;
const bool WarnOnSizeOfThis;
const bool WarnOnSizeOfCompareToConstant;
};
Index: clang-tidy/bugprone/SizeofExpressionCheck.cpp
===================================================================
--- clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -62,12 +62,16 @@
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 @@
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 @@
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 @@
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)'");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
