Author: Congcong Cai Date: 2025-02-27T19:36:24+08:00 New Revision: 56762b7ace0596404e5ae271f278cf7540b374f2
URL: https://github.com/llvm/llvm-project/commit/56762b7ace0596404e5ae271f278cf7540b374f2 DIFF: https://github.com/llvm/llvm-project/commit/56762b7ace0596404e5ae271f278cf7540b374f2.diff LOG: [clang-tidy] Add new check bugprone-unintended-char-ostream-output (#127720) It wants to find unintended character output from `uint8_t` and `int8_t` to an ostream. e.g. ```c++ uint8_t v = 9; std::cout << v; ``` --------- Co-authored-by: whisperity <whisper...@gmail.com> Added: clang-tools-extra/clang-tidy/bugprone/UnintendedCharOstreamOutputCheck.cpp clang-tools-extra/clang-tidy/bugprone/UnintendedCharOstreamOutputCheck.h clang-tools-extra/docs/clang-tidy/checks/bugprone/unintended-char-ostream-output.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/unintended-char-ostream-output-cast-type.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone/unintended-char-ostream-output.cpp Modified: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index c5f0b5b28418f..0a3376949b6e5 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -90,6 +90,7 @@ #include "UndelegatedConstructorCheck.h" #include "UnhandledExceptionAtNewCheck.h" #include "UnhandledSelfAssignmentCheck.h" +#include "UnintendedCharOstreamOutputCheck.h" #include "UniquePtrArrayMismatchCheck.h" #include "UnsafeFunctionsCheck.h" #include "UnusedLocalNonTrivialVariableCheck.h" @@ -147,6 +148,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-incorrect-enable-if"); CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>( "bugprone-incorrect-enable-shared-from-this"); + CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>( + "bugprone-unintended-char-ostream-output"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index cad6b456fc268..dab139b77c770 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -29,6 +29,7 @@ add_clang_library(clangTidyBugproneModule STATIC InaccurateEraseCheck.cpp IncorrectEnableIfCheck.cpp IncorrectEnableSharedFromThisCheck.cpp + UnintendedCharOstreamOutputCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp SwitchMissingDefaultCaseCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/UnintendedCharOstreamOutputCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnintendedCharOstreamOutputCheck.cpp new file mode 100644 index 0000000000000..7250e4ccb8c69 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnintendedCharOstreamOutputCheck.cpp @@ -0,0 +1,91 @@ +//===--- UnintendedCharOstreamOutputCheck.cpp - clang-tidy ----------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UnintendedCharOstreamOutputCheck.h" +#include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Tooling/FixIt.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { + +// check if the type is unsigned char or signed char +AST_MATCHER(Type, isNumericChar) { + return Node.isSpecificBuiltinType(BuiltinType::SChar) || + Node.isSpecificBuiltinType(BuiltinType::UChar); +} + +// check if the type is char +AST_MATCHER(Type, isChar) { + return Node.isSpecificBuiltinType(BuiltinType::Char_S) || + Node.isSpecificBuiltinType(BuiltinType::Char_U); +} + +} // namespace + +UnintendedCharOstreamOutputCheck::UnintendedCharOstreamOutputCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), CastTypeName(Options.get("CastTypeName")) { +} +void UnintendedCharOstreamOutputCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + if (CastTypeName.has_value()) + Options.store(Opts, "CastTypeName", CastTypeName.value()); +} + +void UnintendedCharOstreamOutputCheck::registerMatchers(MatchFinder *Finder) { + auto BasicOstream = + cxxRecordDecl(hasName("::std::basic_ostream"), + // only basic_ostream<char, Traits> has overload operator<< + // with char / unsigned char / signed char + classTemplateSpecializationDecl( + hasTemplateArgument(0, refersToType(isChar())))); + Finder->addMatcher( + cxxOperatorCallExpr( + hasOverloadedOperatorName("<<"), + hasLHS(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(cxxRecordDecl( + anyOf(BasicOstream, isDerivedFrom(BasicOstream)))))))), + hasRHS(hasType(hasUnqualifiedDesugaredType(isNumericChar())))) + .bind("x"), + this); +} + +void UnintendedCharOstreamOutputCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("x"); + const Expr *Value = Call->getArg(1); + const SourceRange SourceRange = Value->getSourceRange(); + + DiagnosticBuilder Builder = + diag(Call->getOperatorLoc(), + "%0 passed to 'operator<<' outputs as character instead of integer. " + "cast to 'unsigned int' to print numeric value or cast to 'char' to " + "print as character") + << Value->getType() << SourceRange; + + QualType T = Value->getType(); + const Type *UnqualifiedDesugaredType = T->getUnqualifiedDesugaredType(); + + llvm::StringRef CastType = CastTypeName.value_or( + UnqualifiedDesugaredType->isSpecificBuiltinType(BuiltinType::SChar) + ? "int" + : "unsigned int"); + + Builder << FixItHint::CreateReplacement( + SourceRange, ("static_cast<" + CastType + ">(" + + tooling::fixit::getText(*Value, *Result.Context) + ")") + .str()); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/UnintendedCharOstreamOutputCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnintendedCharOstreamOutputCheck.h new file mode 100644 index 0000000000000..61ea623d139ea --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/UnintendedCharOstreamOutputCheck.h @@ -0,0 +1,38 @@ +//===--- UnintendedCharOstreamOutputCheck.h - clang-tidy --------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNINTENDEDCHAROSTREAMOUTPUTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNINTENDEDCHAROSTREAMOUTPUTCHECK_H + +#include "../ClangTidyCheck.h" +#include <optional> + +namespace clang::tidy::bugprone { + +/// Finds unintended character output from `unsigned char` and `signed char` to +/// an ostream. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/unintended-char-ostream-output.html +class UnintendedCharOstreamOutputCheck : public ClangTidyCheck { +public: + UnintendedCharOstreamOutputCheck(StringRef Name, ClangTidyContext *Context); + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + +private: + const std::optional<StringRef> CastTypeName; +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNINTENDEDCHAROSTREAMOUTPUTCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b87ea491b3ad1..1a68a3d182e04 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -91,6 +91,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`bugprone-unintended-char-ostream-output + <clang-tidy/checks/bugprone/unintended-char-ostream-output>` check. + + Finds unintended character output from ``unsigned char`` and ``signed char`` to an + ``ostream``. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unintended-char-ostream-output.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unintended-char-ostream-output.rst new file mode 100644 index 0000000000000..ea1051847129b --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unintended-char-ostream-output.rst @@ -0,0 +1,45 @@ +.. title:: clang-tidy - bugprone-unintended-char-ostream-output + +bugprone-unintended-char-ostream-output +======================================= + +Finds unintended character output from ``unsigned char`` and ``signed char`` to an +``ostream``. + +Normally, when ``unsigned char (uint8_t)`` or ``signed char (int8_t)`` is used, it +is more likely a number than a character. However, when it is passed directly to +``std::ostream``'s ``operator<<``, the result is the character output instead +of the numeric value. This often contradicts the developer's intent to print +integer values. + +.. code-block:: c++ + + uint8_t v = 65; + std::cout << v; // output 'A' instead of '65' + +The check will suggest casting the value to an appropriate type to indicate the +intent, by default, it will cast to ``unsigned int`` for ``unsigned char`` and +``int`` for ``signed char``. + +.. code-block:: c++ + + std::cout << static_cast<unsigned int>(v); // when v is unsigned char + std::cout << static_cast<int>(v); // when v is signed char + +To avoid lengthy cast statements, add prefix ``+`` to the variable can also +suppress warnings because unary expression will promote the value to an ``int``. + +.. code-block:: c++ + + std::cout << +v; + +Or cast to char to explicitly indicate that output should be a character. + +.. code-block:: c++ + + std::cout << static_cast<char>(v); + +.. option:: CastTypeName + + When `CastTypeName` is specified, the fix-it will use `CastTypeName` as the + cast target type. Otherwise, fix-it will automatically infer the type. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 7b9b905ef7671..5f03ef72cc603 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -158,6 +158,7 @@ Clang-Tidy Checks :doc:`bugprone-undelegated-constructor <bugprone/undelegated-constructor>`, :doc:`bugprone-unhandled-exception-at-new <bugprone/unhandled-exception-at-new>`, :doc:`bugprone-unhandled-self-assignment <bugprone/unhandled-self-assignment>`, + :doc:`bugprone-unintended-char-ostream-output <bugprone/unintended-char-ostream-output>`, "Yes" :doc:`bugprone-unique-ptr-array-mismatch <bugprone/unique-ptr-array-mismatch>`, "Yes" :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`, :doc:`bugprone-unused-local-non-trivial-variable <bugprone/unused-local-non-trivial-variable>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unintended-char-ostream-output-cast-type.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unintended-char-ostream-output-cast-type.cpp new file mode 100644 index 0000000000000..faea4127ac44a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unintended-char-ostream-output-cast-type.cpp @@ -0,0 +1,45 @@ +// RUN: %check_clang_tidy %s bugprone-unintended-char-ostream-output %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: {bugprone-unintended-char-ostream-output.CastTypeName: "uint8_t"}}" + +namespace std { + +template <class _CharT, class _Traits = void> class basic_ostream { +public: + basic_ostream &operator<<(int); + basic_ostream &operator<<(unsigned int); +}; + +template <class CharT, class Traits> +basic_ostream<CharT, Traits> &operator<<(basic_ostream<CharT, Traits> &, CharT); +template <class CharT, class Traits> +basic_ostream<CharT, Traits> &operator<<(basic_ostream<CharT, Traits> &, char); +template <class _Traits> +basic_ostream<char, _Traits> &operator<<(basic_ostream<char, _Traits> &, char); +template <class _Traits> +basic_ostream<char, _Traits> &operator<<(basic_ostream<char, _Traits> &, + signed char); +template <class _Traits> +basic_ostream<char, _Traits> &operator<<(basic_ostream<char, _Traits> &, + unsigned char); + +using ostream = basic_ostream<char>; + +} // namespace std + +class A : public std::ostream {}; + +void origin_ostream(std::ostream &os) { + unsigned char unsigned_value = 9; + os << unsigned_value; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'unsigned char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<uint8_t>(unsigned_value); + + signed char signed_value = 9; + os << signed_value; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'signed char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<uint8_t>(signed_value); + + char char_value = 9; + os << char_value; +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unintended-char-ostream-output.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unintended-char-ostream-output.cpp new file mode 100644 index 0000000000000..0a5cdeb21c01e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unintended-char-ostream-output.cpp @@ -0,0 +1,131 @@ +// RUN: %check_clang_tidy %s bugprone-unintended-char-ostream-output %t + +namespace std { + +template <class _CharT, class _Traits = void> class basic_ostream { +public: + basic_ostream &operator<<(int); + basic_ostream &operator<<(unsigned int); +}; + +template <class CharT, class Traits> +basic_ostream<CharT, Traits> &operator<<(basic_ostream<CharT, Traits> &, CharT); +template <class CharT, class Traits> +basic_ostream<CharT, Traits> &operator<<(basic_ostream<CharT, Traits> &, char); +template <class _Traits> +basic_ostream<char, _Traits> &operator<<(basic_ostream<char, _Traits> &, char); +template <class _Traits> +basic_ostream<char, _Traits> &operator<<(basic_ostream<char, _Traits> &, + signed char); +template <class _Traits> +basic_ostream<char, _Traits> &operator<<(basic_ostream<char, _Traits> &, + unsigned char); + +using ostream = basic_ostream<char>; + +} // namespace std + +class A : public std::ostream {}; + +void origin_ostream(std::ostream &os) { + unsigned char unsigned_value = 9; + os << unsigned_value; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'unsigned char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<unsigned int>(unsigned_value); + + signed char signed_value = 9; + os << signed_value; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'signed char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<int>(signed_value); + + char char_value = 9; + os << char_value; +} + +void based_on_ostream(A &os) { + unsigned char unsigned_value = 9; + os << unsigned_value; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'unsigned char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<unsigned int>(unsigned_value); + + signed char signed_value = 9; + os << signed_value; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'signed char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<int>(signed_value); + + char char_value = 9; + os << char_value; +} + +void based_on_ostream(std::basic_ostream<unsigned char> &os) { + unsigned char unsigned_value = 9; + os << unsigned_value; + + signed char signed_value = 9; + os << signed_value; + + char char_value = 9; + os << char_value; +} + +template <class T> class B : public std::ostream {}; +void template_based_on_ostream(B<int> &os) { + unsigned char unsigned_value = 9; + os << unsigned_value; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'unsigned char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<unsigned int>(unsigned_value); +} + +template<class T> void template_fn_1(T &os) { + unsigned char unsigned_value = 9; + os << unsigned_value; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'unsigned char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<unsigned int>(unsigned_value); +} +template<class T> void template_fn_2(std::ostream &os) { + T unsigned_value = 9; + os << unsigned_value; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'unsigned char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<unsigned int>(unsigned_value); +} +template<class T> void template_fn_3(std::ostream &os) { + T unsigned_value = 9; + os << unsigned_value; +} +void call_template_fn() { + A a{}; + template_fn_1(a); + template_fn_2<unsigned char>(a); + template_fn_3<char>(a); +} + +using U8 = unsigned char; +void alias_unsigned_char(std::ostream &os) { + U8 v = 9; + os << v; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'U8' (aka 'unsigned char') passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<unsigned int>(v); +} + +using I8 = signed char; +void alias_signed_char(std::ostream &os) { + I8 v = 9; + os << v; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'I8' (aka 'signed char') passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<int>(v); +} + +using C8 = char; +void alias_char(std::ostream &os) { + C8 v = 9; + os << v; +} + + +#define MACRO_VARIANT_NAME a +void macro_variant_name(std::ostream &os) { + unsigned char MACRO_VARIANT_NAME = 9; + os << MACRO_VARIANT_NAME; + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: 'unsigned char' passed to 'operator<<' outputs as character instead of integer + // CHECK-FIXES: os << static_cast<unsigned int>(MACRO_VARIANT_NAME); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits