https://github.com/bjosv updated https://github.com/llvm/llvm-project/pull/149790
From c0b8835f8aeebe6523c88b32163fbe6c77d9ce83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svens...@est.tech> Date: Thu, 17 Jul 2025 12:14:48 +0200 Subject: [PATCH 1/4] Add C23 tests for bugprone-signed-char-misuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Björn Svensson <bjorn.a.svens...@est.tech> --- .../bugprone/signed-char-misuse-c23.c | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c new file mode 100644 index 0000000000000..1ae0e69c30416 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c @@ -0,0 +1,94 @@ +// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t -- -- -std=c23 + +/////////////////////////////////////////////////////////////////// +/// Test cases correctly caught by the check. + +int SimpleVarDeclaration() { + signed char CCharacter = -5; + int NCharacter = CCharacter; + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] + + return NCharacter; +} + +int SimpleAssignment() { + signed char CCharacter = -5; + int NCharacter; + NCharacter = CCharacter; + // CHECK-MESSAGES: [[@LINE-1]]:16: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] + + return NCharacter; +} + +int NegativeConstValue() { + const signed char CCharacter = -5; + int NCharacter = CCharacter; + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] + + return NCharacter; +} + +int CharPointer(signed char *CCharacter) { + int NCharacter = *CCharacter; + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] + + return NCharacter; +} + +int SignedUnsignedCharEquality(signed char SCharacter) { + unsigned char USCharacter = 'a'; + if (SCharacter == USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse] + return 1; + return 0; +} + +int SignedUnsignedCharIneqiality(signed char SCharacter) { + unsigned char USCharacter = 'a'; + if (SCharacter != USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse] + return 1; + return 0; +} + +int CompareWithNonAsciiConstant(unsigned char USCharacter) { + const signed char SCharacter = -5; + if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse] + return 1; + return 0; +} + +int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) { + const unsigned char USCharacter = 128; + if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse] + return 1; + return 0; +} + +/////////////////////////////////////////////////////////////////// +/// Test cases correctly ignored by the check. + +enum EType1 : signed char { + EType1_M128 = -128, + EType1_1 = 1, +}; + +enum EType1 es1_1 = EType1_M128; +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'signed char' to 'enum EType1' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] +enum EType1 es1_2 = EType1_1; +enum EType1 es1_3 = -128; + +void assign(enum EType1 *es) { + *es = EType1_M128; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'signed char' to 'enum EType1' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] +} + + +typedef signed char int8_t; +typedef enum EType2 : int8_t { + EType2_M128 = -128, + EType2_1 = 1, +} EType2_t; + +EType2_t es2_1 = EType2_M128; +// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'signed char' to 'EType2_t' (aka 'enum EType2') conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] +EType2_t es2_2 = EType2_1; +EType2_t es2_3 = -128; From 0436c2fd306812c29aef789887b3107b7cf696f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svens...@est.tech> Date: Fri, 18 Jul 2025 09:55:36 +0200 Subject: [PATCH 2/4] Correcting bugprone-signed-char-misuse for C23 enums MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Björn Svensson <bjorn.a.svens...@est.tech> --- .../bugprone/SignedCharMisuseCheck.cpp | 26 +++++++++++++++---- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../bugprone/signed-char-misuse-c23.c | 3 --- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp index ea3b8b8e9df4f..82276628203dd 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp @@ -16,6 +16,17 @@ using namespace clang::ast_matchers::internal; namespace clang::tidy::bugprone { +namespace { + +AST_MATCHER(Stmt, isC23Stmt) { + return Finder->getASTContext().getLangOpts().C23; +} +AST_MATCHER(Decl, isC23Decl) { + return Finder->getASTContext().getLangOpts().C23; +} + +} // anonymous namespace + static constexpr int UnsignedASCIIUpperBound = 127; SignedCharMisuseCheck::SignedCharMisuseCheck(StringRef Name, @@ -75,16 +86,21 @@ void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) { const auto UnSignedCharCastExpr = charCastExpression(false, IntegerType, "unsignedCastExpression"); - // Catch assignments with signed char -> integer conversion. + // Catch assignments with signed char -> integer conversion. Ignore false + // positives on C23 enums with the fixed underlying type of signed char. const auto AssignmentOperatorExpr = expr(binaryOperator(hasOperatorName("="), hasLHS(hasType(IntegerType)), - hasRHS(SignedCharCastExpr))); + hasRHS(SignedCharCastExpr)), + unless(allOf(isC23Stmt(), binaryOperator(hasLHS(hasType( + hasCanonicalType(enumType()))))))); Finder->addMatcher(AssignmentOperatorExpr, this); - // Catch declarations with signed char -> integer conversion. - const auto Declaration = varDecl(isDefinition(), hasType(IntegerType), - hasInitializer(SignedCharCastExpr)); + // Catch declarations with signed char -> integer conversion. Ignore false + // positives on C23 enums with the fixed underlying type of signed char. + const auto Declaration = varDecl( + isDefinition(), hasType(IntegerType), hasInitializer(SignedCharCastExpr), + unless(allOf(isC23Decl(), hasType(hasCanonicalType(enumType()))))); Finder->addMatcher(Declaration, this); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 9eb3835fe8340..af7192c4d649a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -204,6 +204,10 @@ Changes in existing checks <clang-tidy/checks/bugprone/optional-value-conversion>` check to detect conversion in argument of ``std::make_optional``. +- Improved :doc:`bugprone-signed-char-misuse + <clang-tidy/checks/bugprone/signed-char-misuse>` check by fixing + false positives on C23 enums with the fixed underlying type of signed char. + - Improved :doc:`bugprone-sizeof-expression <clang-tidy/checks/bugprone/sizeof-expression>` check by adding `WarnOnSizeOfInLoopTermination` option to detect misuses of ``sizeof`` diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c index 1ae0e69c30416..349843f42fbc7 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c @@ -72,13 +72,11 @@ enum EType1 : signed char { }; enum EType1 es1_1 = EType1_M128; -// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'signed char' to 'enum EType1' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] enum EType1 es1_2 = EType1_1; enum EType1 es1_3 = -128; void assign(enum EType1 *es) { *es = EType1_M128; -// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'signed char' to 'enum EType1' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] } @@ -89,6 +87,5 @@ typedef enum EType2 : int8_t { } EType2_t; EType2_t es2_1 = EType2_M128; -// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'signed char' to 'EType2_t' (aka 'enum EType2') conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse] EType2_t es2_2 = EType2_1; EType2_t es2_3 = -128; From 66babe4ce7528966227b6b8a9f9079882097e434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svens...@est.tech> Date: Tue, 22 Jul 2025 01:17:16 +0200 Subject: [PATCH 3/4] fixup: simplify matcher for C23 handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Björn Svensson <bjorn.a.svens...@est.tech> --- .../bugprone/SignedCharMisuseCheck.cpp | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp index 82276628203dd..dfd3cbfcd664a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp @@ -16,17 +16,6 @@ using namespace clang::ast_matchers::internal; namespace clang::tidy::bugprone { -namespace { - -AST_MATCHER(Stmt, isC23Stmt) { - return Finder->getASTContext().getLangOpts().C23; -} -AST_MATCHER(Decl, isC23Decl) { - return Finder->getASTContext().getLangOpts().C23; -} - -} // anonymous namespace - static constexpr int UnsignedASCIIUpperBound = 127; SignedCharMisuseCheck::SignedCharMisuseCheck(StringRef Name, @@ -85,14 +74,16 @@ void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) { charCastExpression(true, IntegerType, "signedCastExpression"); const auto UnSignedCharCastExpr = charCastExpression(false, IntegerType, "unsignedCastExpression"); + const bool IsC23 = getLangOpts().C23; // Catch assignments with signed char -> integer conversion. Ignore false // positives on C23 enums with the fixed underlying type of signed char. const auto AssignmentOperatorExpr = expr(binaryOperator(hasOperatorName("="), hasLHS(hasType(IntegerType)), hasRHS(SignedCharCastExpr)), - unless(allOf(isC23Stmt(), binaryOperator(hasLHS(hasType( - hasCanonicalType(enumType()))))))); + IsC23 ? unless(binaryOperator( + hasLHS(hasType(hasCanonicalType(enumType()))))) + : Matcher<Stmt>(anything())); Finder->addMatcher(AssignmentOperatorExpr, this); @@ -100,7 +91,8 @@ void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) { // positives on C23 enums with the fixed underlying type of signed char. const auto Declaration = varDecl( isDefinition(), hasType(IntegerType), hasInitializer(SignedCharCastExpr), - unless(allOf(isC23Decl(), hasType(hasCanonicalType(enumType()))))); + IsC23 ? unless(hasType(hasCanonicalType(enumType()))) + : Matcher<VarDecl>(anything())); Finder->addMatcher(Declaration, this); From b0beac901f19beea92768926c36083a3a9ff8659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svens...@est.tech> Date: Tue, 22 Jul 2025 17:07:15 +0200 Subject: [PATCH 4/4] fixup: add additional tests with cases ignored by the check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Björn Svensson <bjorn.a.svens...@est.tech> --- .../bugprone/signed-char-misuse-c23.c | 112 +++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c index 349843f42fbc7..ad6e31d873e5a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c @@ -66,6 +66,7 @@ int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) { /////////////////////////////////////////////////////////////////// /// Test cases correctly ignored by the check. +// Enum with a fixed underlying type of signed char. enum EType1 : signed char { EType1_M128 = -128, EType1_1 = 1, @@ -79,7 +80,7 @@ void assign(enum EType1 *es) { *es = EType1_M128; } - +// Type aliased enum with a fixed underlying type of signed char. typedef signed char int8_t; typedef enum EType2 : int8_t { EType2_M128 = -128, @@ -89,3 +90,112 @@ typedef enum EType2 : int8_t { EType2_t es2_1 = EType2_M128; EType2_t es2_2 = EType2_1; EType2_t es2_3 = -128; + +// Enum with a fixed underlying type of unsigned char. +enum EType3 : unsigned char { + EType3_1 = 1, + EType3_128 = 128, +}; + +enum EType3 es3_1 = EType3_1; +enum EType3 es3_2 = EType3_128; +enum EType3 es3_3 = 128; + + +int UnsignedCharCast() { + unsigned char CCharacter = 'a'; + int NCharacter = CCharacter; + + return NCharacter; +} + +int PositiveConstValue() { + const signed char CCharacter = 5; + int NCharacter = CCharacter; + + return NCharacter; +} + +// signed char -> integer cast is not the direct child of declaration expression. +int DescendantCast() { + signed char CCharacter = 'a'; + int NCharacter = 10 + CCharacter; + + return NCharacter; +} + +// signed char -> integer cast is not the direct child of assignment expression. +int DescendantCastAssignment() { + signed char CCharacter = 'a'; + int NCharacter; + NCharacter = 10 + CCharacter; + + return NCharacter; +} + +// bool is an integer type in clang; make sure to ignore it. +bool BoolVarDeclaration() { + signed char CCharacter = 'a'; + bool BCharacter = CCharacter == 'b'; + + return BCharacter; +} + +// bool is an integer type in clang; make sure to ignore it. +bool BoolAssignment() { + signed char CCharacter = 'a'; + bool BCharacter; + BCharacter = CCharacter == 'b'; + + return BCharacter; +} + +// char is an integer type in clang; make sure to ignore it. +unsigned char CharToCharCast() { + signed char SCCharacter = 'a'; + unsigned char USCharacter; + USCharacter = SCCharacter; + + return USCharacter; +} + +int SameCharTypeComparison(signed char SCharacter) { + signed char SCharacter2 = 'a'; + if (SCharacter == SCharacter2) + return 1; + return 0; +} + +int SameCharTypeComparison2(unsigned char USCharacter) { + unsigned char USCharacter2 = 'a'; + if (USCharacter == USCharacter2) + return 1; + return 0; +} + +int CharIntComparison(signed char SCharacter) { + int ICharacter = 10; + if (SCharacter == ICharacter) + return 1; + return 0; +} + +int CompareWithAsciiLiteral(unsigned char USCharacter) { + if (USCharacter == 'x') + return 1; + return 0; +} + +int CompareWithAsciiConstant(unsigned char USCharacter) { + const signed char SCharacter = 'a'; + if (USCharacter == SCharacter) + return 1; + return 0; +} + +int CompareWithUnsignedAsciiConstant(signed char SCharacter) { + const unsigned char USCharacter = 'a'; + if (USCharacter == SCharacter) + return 1; + return 0; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits