Skip to content

Commit fdcfb27

Browse files
authored
[clang-tidy] Clarify diagnostics of bugprone-sizeof-expression (#95550)
… because they were strangely worded and in a few cases outright incorrect.
1 parent 2dd1406 commit fdcfb27

File tree

6 files changed

+98
-95
lines changed

6 files changed

+98
-95
lines changed

clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp

+12-9
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
296296
} else if (const auto *E =
297297
Result.Nodes.getNodeAs<Expr>("sizeof-integer-call")) {
298298
diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
299-
"that results in an integer")
299+
"of integer type")
300300
<< E->getSourceRange();
301301
} else if (const auto *E = Result.Nodes.getNodeAs<Expr>("sizeof-this")) {
302302
diag(E->getBeginLoc(),
@@ -314,7 +314,7 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
314314
<< E->getSourceRange();
315315
} else {
316316
diag(E->getBeginLoc(), "suspicious usage of 'sizeof()' on an expression "
317-
"that results in a pointer")
317+
"of pointer type")
318318
<< E->getSourceRange();
319319
}
320320
} else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
@@ -348,25 +348,28 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
348348
} else if (ElementSize > CharUnits::Zero() &&
349349
DenominatorSize > CharUnits::Zero() &&
350350
ElementSize != DenominatorSize) {
351-
diag(E->getOperatorLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
352-
" numerator is not a multiple of denominator")
351+
// FIXME: Apparently there are no testcases that cover this branch!
352+
diag(E->getOperatorLoc(),
353+
"suspicious usage of 'sizeof(array)/sizeof(...)';"
354+
" denominator differs from the size of array elements")
353355
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
354356
} else if (NumTy && DenomTy && NumTy == DenomTy) {
355-
// FIXME: This message is wrong, it should not refer to sizeof "pointer"
356-
// usage (and by the way, it would be to clarify all the messages).
357357
diag(E->getOperatorLoc(),
358-
"suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'")
358+
"suspicious usage of 'sizeof(...)/sizeof(...)'; both expressions "
359+
"have the same type")
359360
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
360361
} else if (!WarnOnSizeOfPointer) {
361362
// When 'WarnOnSizeOfPointer' is enabled, these messages become redundant:
362363
if (PointedTy && DenomTy && PointedTy == DenomTy) {
363364
diag(E->getOperatorLoc(),
364-
"suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'")
365+
"suspicious usage of 'sizeof(...)/sizeof(...)'; size of pointer "
366+
"is divided by size of pointed type")
365367
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
366368
} else if (NumTy && DenomTy && NumTy->isPointerType() &&
367369
DenomTy->isPointerType()) {
368370
diag(E->getOperatorLoc(),
369-
"suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'")
371+
"suspicious usage of 'sizeof(...)/sizeof(...)'; both expressions "
372+
"have pointer types")
370373
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
371374
}
372375
}

clang-tools-extra/docs/ReleaseNotes.rst

+4-4
Original file line numberDiff line numberDiff line change
@@ -241,10 +241,10 @@ Changes in existing checks
241241
false positives resulting from use of optionals in unevaluated context.
242242

243243
- Improved :doc:`bugprone-sizeof-expression
244-
<clang-tidy/checks/bugprone/sizeof-expression>` check by eliminating some
245-
false positives and adding a new (off-by-default) option
246-
`WarnOnSizeOfPointer` that reports all ``sizeof(pointer)`` expressions
247-
(except for a few that are idiomatic).
244+
<clang-tidy/checks/bugprone/sizeof-expression>` check by clarifying the
245+
diagnostics, eliminating some false positives and adding a new
246+
(off-by-default) option `WarnOnSizeOfPointer` that reports all
247+
``sizeof(pointer)`` expressions (except for a few that are idiomatic).
248248

249249
- Improved :doc:`bugprone-suspicious-include
250250
<clang-tidy/checks/bugprone/suspicious-include>` check by replacing the local

clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c

+6-6
Original file line numberDiff line numberDiff line change
@@ -34,24 +34,24 @@ int Test5() {
3434

3535
int sum = 0;
3636
sum += sizeof(&S);
37-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
37+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
3838
sum += sizeof(__typeof(&S));
3939
sum += sizeof(&TS);
40-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
40+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
4141
sum += sizeof(__typeof(&TS));
4242
sum += sizeof(STRKWD MyStruct*);
4343
sum += sizeof(__typeof(STRKWD MyStruct*));
4444
sum += sizeof(TypedefStruct*);
4545
sum += sizeof(__typeof(TypedefStruct*));
4646
sum += sizeof(PTTS);
47-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
47+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
4848
sum += sizeof(PMyStruct);
4949
sum += sizeof(PS);
50-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
50+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
5151
sum += sizeof(PS2);
52-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
52+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
5353
sum += sizeof(&A10);
54-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
54+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
5555

5656
#ifdef __cplusplus
5757
MyStruct &rS = S;

clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-any-pointer.cpp

+38-38
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,17 @@ int Test1(const char* ptr) {
3838
sum += sizeof(sum, LEN);
3939
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(..., ...)'
4040
sum += sizeof(AsBool());
41-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
41+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of integer type
4242
sum += sizeof(AsInt());
43-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
43+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of integer type
4444
sum += sizeof(AsEnum());
45-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
45+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of integer type
4646
sum += sizeof(AsEnumClass());
47-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
47+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of integer type
4848
sum += sizeof(M{}.AsInt());
49-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
49+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of integer type
5050
sum += sizeof(M{}.AsEnum());
51-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
51+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of integer type
5252
sum += sizeof(sizeof(X));
5353
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
5454
sum += sizeof(LEN + sizeof(X));
@@ -62,7 +62,7 @@ int Test1(const char* ptr) {
6262
sum += sizeof(LEN + - + -sizeof(X));
6363
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
6464
sum += sizeof(char) / sizeof(char);
65-
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
65+
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; both expressions have the same type
6666
sum += sizeof(A) / sizeof(S);
6767
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
6868
sum += sizeof(char) / sizeof(int);
@@ -72,21 +72,21 @@ int Test1(const char* ptr) {
7272
sum += sizeof(B[0]) / sizeof(A);
7373
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
7474
sum += sizeof(ptr) / sizeof(char);
75-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
75+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
7676
sum += sizeof(ptr) / sizeof(ptr[0]);
77-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
77+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
7878
sum += sizeof(ptr) / sizeof(char*);
79-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
79+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
8080
sum += sizeof(ptr) / sizeof(void*);
81-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
81+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
8282
sum += sizeof(ptr) / sizeof(const void volatile*);
83-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
83+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
8484
sum += sizeof(ptr) / sizeof(char);
85-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
85+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
8686
sum += sizeof(int) * sizeof(char);
8787
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious 'sizeof' by 'sizeof' multiplication
8888
sum += sizeof(ptr) * sizeof(ptr[0]);
89-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
89+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
9090
// CHECK-MESSAGES: :[[@LINE-2]]:22: warning: suspicious 'sizeof' by 'sizeof' multiplication
9191
sum += sizeof(int) * (2 * sizeof(char));
9292
// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: suspicious 'sizeof' by 'sizeof' multiplication
@@ -127,54 +127,54 @@ int Test5() {
127127

128128
int sum = 0;
129129
sum += sizeof(&S.arr);
130-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
130+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
131131
sum += sizeof(&kGlocalMyStruct.arr);
132-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
132+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
133133
sum += sizeof(&kGlocalMyStructPtr->arr);
134-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
134+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
135135
sum += sizeof(S.arr + 0);
136-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
136+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
137137
sum += sizeof(+ S.arr);
138-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
138+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
139139
sum += sizeof((int*)S.arr);
140-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
140+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
141141

142142
sum += sizeof(S.ptr);
143-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
143+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
144144
sum += sizeof(kGlocalMyStruct.ptr);
145-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
145+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
146146
sum += sizeof(kGlocalMyStructPtr->ptr);
147-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
147+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
148148

149149
sum += sizeof(&kGlocalMyStruct);
150-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
150+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
151151
sum += sizeof(&S);
152-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
152+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
153153
sum += sizeof(MyStruct*);
154154
sum += sizeof(PMyStruct);
155155
sum += sizeof(PS);
156-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
156+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
157157
sum += sizeof(PS2);
158-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
158+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
159159
sum += sizeof(&A10);
160-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
160+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
161161
sum += sizeof(PtrArray) / sizeof(PtrArray[1]);
162-
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
162+
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: suspicious usage of 'sizeof()' on an expression of pointer type
163163
sum += sizeof(A10) / sizeof(PtrArray[0]);
164164
sum += sizeof(PC) / sizeof(PtrArray[0]);
165-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
166-
// CHECK-MESSAGES: :[[@LINE-2]]:21: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
165+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
166+
// CHECK-MESSAGES: :[[@LINE-2]]:21: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; both expressions have the same type
167167
sum += sizeof(ArrayC) / sizeof(PtrArray[0]);
168168
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
169169

170170
sum += sizeof(PChar);
171-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
171+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
172172
sum += sizeof(PInt);
173-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
173+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
174174
sum += sizeof(PPInt);
175-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
175+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
176176
sum += sizeof(PPMyStruct);
177-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
177+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
178178

179179
return sum;
180180
}
@@ -200,9 +200,9 @@ void GenericFunctionTest() {
200200
// reported by the `sizeof(pointer)` checks, but this causes some false
201201
// positives, so it would be good to create an exception for them.
202202
some_generic_function(&IntPP, sizeof(IntP));
203-
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
203+
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: suspicious usage of 'sizeof()' on an expression of pointer type
204204
some_generic_function(&ClassPP, sizeof(ClassP));
205-
// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
205+
// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: suspicious usage of 'sizeof()' on an expression of pointer type
206206
}
207207

208208
int ValidExpressions() {
@@ -222,7 +222,7 @@ int ValidExpressions() {
222222
sum += sizeof(A[sizeof(A) / sizeof(int)]);
223223
// Here the outer sizeof is reported, but the inner ones are accepted:
224224
sum += sizeof(&A[sizeof(A) / sizeof(int)]);
225-
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in a pointer
225+
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression of pointer type
226226
sum += sizeof(sizeof(0)); // Special case: sizeof size_t.
227227
sum += sizeof(void*);
228228
sum += sizeof(void const *);

clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-warn-on-sizeof-pointer-to-aggregate.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ int Test5() {
6868
sum += sizeof(A10) / sizeof(PtrArray[0]);
6969
// No warning.
7070
sum += sizeof(PC) / sizeof(PtrArray[0]);
71-
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
71+
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; both expressions have the same type
7272
sum += sizeof(ArrayC) / sizeof(PtrArray[0]);
7373
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
7474

0 commit comments

Comments
 (0)