diff --git a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h --- a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.h @@ -38,6 +38,7 @@ const std::string &CastBindName) const; const std::string CharTypdefsToIgnoreList; + const bool DiagnoseSignedUnsignedCharComparisons; }; } // namespace bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp @@ -29,10 +29,14 @@ SignedCharMisuseCheck::SignedCharMisuseCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - CharTypdefsToIgnoreList(Options.get("CharTypdefsToIgnore", "")) {} + CharTypdefsToIgnoreList(Options.get("CharTypdefsToIgnore", "")), + DiagnoseSignedUnsignedCharComparisons( + Options.get("DiagnoseSignedUnsignedCharComparisons", true)) {} void SignedCharMisuseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "CharTypdefsToIgnore", CharTypdefsToIgnoreList); + Options.store(Opts, "DiagnoseSignedUnsignedCharComparisons", + DiagnoseSignedUnsignedCharComparisons); } // Create a matcher for char -> integer cast. @@ -92,16 +96,18 @@ Finder->addMatcher(Declaration, this); - // Catch signed char/unsigned char comparison. - const auto CompareOperator = - expr(binaryOperator(hasAnyOperatorName("==", "!="), - anyOf(allOf(hasLHS(SignedCharCastExpr), - hasRHS(UnSignedCharCastExpr)), - allOf(hasLHS(UnSignedCharCastExpr), - hasRHS(SignedCharCastExpr))))) - .bind("comparison"); - - Finder->addMatcher(CompareOperator, this); + if (DiagnoseSignedUnsignedCharComparisons) { + // Catch signed char/unsigned char comparison. + const auto CompareOperator = + expr(binaryOperator(hasAnyOperatorName("==", "!="), + anyOf(allOf(hasLHS(SignedCharCastExpr), + hasRHS(UnSignedCharCastExpr)), + allOf(hasLHS(UnSignedCharCastExpr), + hasRHS(SignedCharCastExpr))))) + .bind("comparison"); + + Finder->addMatcher(CompareOperator, this); + } // Catch array subscripts with signed char -> integer conversion. // Matcher for C arrays. diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -11,6 +11,7 @@ #include "../ClangTidyModuleRegistry.h" #include "../bugprone/BadSignalToKillThreadCheck.h" #include "../bugprone/ReservedIdentifierCheck.h" +#include "../bugprone/SignedCharMisuseCheck.h" #include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h" #include "../bugprone/UnhandledSelfAssignmentCheck.h" #include "../google/UnnamedNamespaceInHeaderCheck.h" @@ -108,6 +109,9 @@ // POS CheckFactories.registerCheck( "cert-pos44-c"); + // STR + CheckFactories.registerCheck( + "cert-str34-c"); } ClangTidyOptions getModuleOptions() override { @@ -115,6 +119,7 @@ ClangTidyOptions::OptionMap &Opts = Options.CheckOptions; Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU"; Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "0"; + Opts["cert-str34-c.DiagnoseSignedUnsignedCharComparisons"] = "0"; return Options; } }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -168,6 +168,11 @@ :doc:`bugprone-reserved-identifier ` was added. +- New alias :doc:`cert-str34-c + ` to + :doc:`bugprone-signed-char-misuse + ` was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-signed-char-misuse.rst @@ -3,6 +3,9 @@ bugprone-signed-char-misuse =========================== +`cert-str34-c` redirects here as an alias for this check. For the CERT alias, +the `DiagnoseSignedUnsignedCharComparisons` option is set to `0`. + Finds those ``signed char`` -> integer conversions which might indicate a programming error. The basic problem with the ``signed char``, that it might store the non-ASCII characters as negative values. This behavior can cause a @@ -108,3 +111,8 @@ check. This is useful when a typedef introduces an integer alias like ``sal_Int8`` or ``int8_t``. In this case, human misinterpretation is not an issue. + +.. option:: DiagnoseSignedUnsignedCharComparisons + + When nonzero, the check will warn on ``signed char``/``unsigned char`` comparisons, + otherwise these comparisons are ignored. By default, this option is set to ``1``. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-str34-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-str34-c.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert-str34-c.rst @@ -0,0 +1,10 @@ +.. title:: clang-tidy - cert-str34-c +.. meta:: + :http-equiv=refresh: 5;URL=bugprone-signed-char-misuse.html + +cert-str34-c +============ + +The cert-str34-c check is an alias, please see +`bugprone-signed-char-misuse `_ +for more information. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -320,6 +320,7 @@ `cert-oop11-cpp `_, `performance-move-constructor-init `_, "Yes" `cert-oop54-cpp `_, `bugprone-unhandled-self-assignment `_, `cert-pos44-c `_, `bugprone-bad-signal-to-kill-thread `_, + `cert-str34-c `_, `bugprone-signed-char-misuse `_, `clang-analyzer-core.CallAndMessage `_, `Clang Static Analyzer `_, `clang-analyzer-core.DivideZero `_, `Clang Static Analyzer `_, `clang-analyzer-core.NonNullParamChecker `_, `Clang Static Analyzer `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert-str34-c.cpp b/clang-tools-extra/test/clang-tidy/checkers/cert-str34-c.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cert-str34-c.cpp @@ -0,0 +1,18 @@ +// RUN: %check_clang_tidy %s cert-str34-c %t + +// Check whether alias is actually working. +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. [cert-str34-c] + + return NCharacter; +} + +// Check whether bugprone-signed-char-misuse.DiagnoseSignedUnsignedCharComparisons option is set correctly. +int SignedUnsignedCharEquality(signed char SCharacter) { + unsigned char USCharacter = 'a'; + if (SCharacter == USCharacter) // no warning + return 1; + return 0; +}