[clang-tools-extra] [clang-tidy] Fix false positives in `bugprone-signed-char-misuse` (PR #149790)
Björn Svensson via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 22 08:09:49 PDT 2025
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.svensson at 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.svensson at 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.svensson at 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.svensson at 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.svensson at 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.svensson at 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.svensson at 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.svensson at 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;
+}
More information about the cfe-commits
mailing list