=?utf-8?q?Björn?= Svensson <bjorn.a.svens...@est.tech> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/149...@github.com>
llvmbot wrote: <!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Björn Svensson (bjosv) <details> <summary>Changes</summary> Ignore false positives on C23 enums which allows setting the fixed underlying type to signed char. The AST tree for [C enums](https://godbolt.org/z/5snW9114E) includes a ImplicitCastExp (that was matched) but this is not the case for [C++ enums](https://godbolt.org/z/Yj5TaqzcE). Fixes #<!-- -->145651 --- Full diff: https://github.com/llvm/llvm-project/pull/149790.diff 3 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp (+21-5) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c (+91) ``````````diff 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 new file mode 100644 index 0000000000000..349843f42fbc7 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c @@ -0,0 +1,91 @@ +// 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; +enum EType1 es1_2 = EType1_1; +enum EType1 es1_3 = -128; + +void assign(enum EType1 *es) { + *es = EType1_M128; +} + + +typedef signed char int8_t; +typedef enum EType2 : int8_t { + EType2_M128 = -128, + EType2_1 = 1, +} EType2_t; + +EType2_t es2_1 = EType2_M128; +EType2_t es2_2 = EType2_1; +EType2_t es2_3 = -128; `````````` </details> https://github.com/llvm/llvm-project/pull/149790 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
