From 06fb72b3720e3c457fc672c38258474879006682 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Thu, 7 Nov 2024 15:21:48 +0800 Subject: [PATCH 1/3] [clang-tidy] fix bugprone-sizeof-expression when sizeof expression with template types Fixed: #115175. `dependent type` are not the same even pointers are the same. --- .../clang-tidy/bugprone/SizeofExpressionCheck.cpp | 4 +++- clang-tools-extra/docs/ReleaseNotes.rst | 3 ++- .../test/clang-tidy/checkers/bugprone/sizeof-expression.cpp | 5 +++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp index 628d30ce7f73f..7269683384f6b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp @@ -400,7 +400,9 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) { "suspicious usage of 'sizeof(array)/sizeof(...)';" " denominator differs from the size of array elements") << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange(); - } else if (NumTy && DenomTy && NumTy == DenomTy) { + } else if (NumTy && DenomTy && NumTy == DenomTy && + !NumTy->isDependentType()) { + // dependent type should not be compared. diag(E->getOperatorLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)'; both expressions " "have the same type") diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index abcdcc25705bf..4fa8292ff8974 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -175,7 +175,8 @@ Changes in existing checks - Improved :doc:`bugprone-sizeof-expression ` check to find suspicious usages of ``sizeof()``, ``alignof()``, and ``offsetof()`` when adding or - subtracting from a pointer directly or when used to scale a numeric value. + subtracting from a pointer directly or when used to scale a numeric value and + fix false positive when sizeof expression with template types. - Improved :doc:`bugprone-unchecked-optional-access ` to support diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp index 81efd87345c97..f3ab78c7355b2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp @@ -385,3 +385,8 @@ int ValidExpressions() { sum += sizeof(PtrArray) / sizeof(A[0]); return sum; } + +template +int ValidateTemplateTypeExpressions(T t) { + return sizeof(t.val) / sizeof(t.val[0]); +} From 2df4bc111ad45b014a0a8b5727cade71fa5d9868 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Sat, 9 Nov 2024 21:26:12 +0800 Subject: [PATCH 2/3] Update clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp Co-authored-by: whisperity --- clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp index 7269683384f6b..f3d4c2255d86e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp @@ -402,7 +402,7 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) { << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange(); } else if (NumTy && DenomTy && NumTy == DenomTy && !NumTy->isDependentType()) { - // dependent type should not be compared. + // Dependent type should not be compared. diag(E->getOperatorLoc(), "suspicious usage of 'sizeof(...)/sizeof(...)'; both expressions " "have the same type") From 4388d530aa24d4e079db1edb72fe1b5d6a1b2937 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Tue, 12 Nov 2024 11:36:16 +0800 Subject: [PATCH 3/3] add issue number in test --- .../test/clang-tidy/checkers/bugprone/sizeof-expression.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp index f3ab78c7355b2..5e6f394152e9d 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp @@ -386,7 +386,9 @@ int ValidExpressions() { return sum; } +namespace gh115175 { template int ValidateTemplateTypeExpressions(T t) { return sizeof(t.val) / sizeof(t.val[0]); } +} // namespace gh115175