BRevzin updated this revision to Diff 128768. BRevzin added a comment. Updates based on review comments - and rebased off of latest so as to get the ReleaseNotes right. Added options so that the user can provide the list of stream types and int typedef types, as desired, defaulting to just `basic_ostream` and `int8_t`/`uint8_t`.
https://reviews.llvm.org/D41740 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/StreamInt8Check.cpp clang-tidy/bugprone/StreamInt8Check.h docs/ReleaseNotes.rst docs/clang-tidy/checks/bugprone-stream-int8.rst docs/clang-tidy/checks/list.rst test/clang-tidy/bugprone-stream-int8.cpp
Index: test/clang-tidy/bugprone-stream-int8.cpp =================================================================== --- /dev/null +++ test/clang-tidy/bugprone-stream-int8.cpp @@ -0,0 +1,46 @@ +// RUN: %check_clang_tidy %s bugprone-stream-int8 %t -- \ +// RUN: -config="{CheckOptions: \ +// RUN: [{key: bugprone-stream-int8.AliasTypes, \ +// RUN: value: 'int8_t; uint8_t; widget'}]}" \ +// RUN: -- -std=c++11 + +using int8_t = signed char; +using uint8_t = unsigned char; +using widget = unsigned char; + +namespace std { + template <typename CharT> + struct basic_ostream; + + using ostream = basic_ostream<char>; + + basic_ostream<char>& operator<<(basic_ostream<char>&, char ); + basic_ostream<char>& operator<<(basic_ostream<char>&, unsigned char ); + basic_ostream<char>& operator<<(basic_ostream<char>&, signed char ); + basic_ostream<char>& operator<<(basic_ostream<char>&, int ); +} + +void BadStreaming(std::ostream& os, uint8_t i) { + // CHECK-MESSAGES: :[[@LINE+1]]:11: warning: streaming uint8_t; did you mean to stream an int? [bugprone-stream-int8] + os << i; + + // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: streaming uint8_t; did you mean to stream an int? [bugprone-stream-int8] + os << 4 << i; + + // CHECK-MESSAGES: :[[@LINE+1]]:11: warning: streaming int8_t; did you mean to stream an int? [bugprone-stream-int8] + os << int8_t{}; + + using Num = int8_t; + // CHECK-MESSAGES: :[[@LINE+1]]:11: warning: streaming int8_t; did you mean to stream an int? [bugprone-stream-int8] + os << Num{}; + + // CHECK-MESSAGES: :[[@LINE+1]]:11: warning: streaming widget; did you mean to stream an int? [bugprone-stream-int8] + os << widget{}; +} + +void GoodStreaming(std::ostream& os, uint8_t i) { + using UC = unsigned char; + + unsigned char uc; + os << +i << 4 << uc << UC{}; +} Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -29,6 +29,7 @@ bugprone-misplaced-operator-in-strlen-in-alloc bugprone-move-forwarding-reference bugprone-multiple-statement-macro + bugprone-stream-int8 bugprone-string-constructor bugprone-suspicious-memset-usage bugprone-undefined-memory-manipulation Index: docs/clang-tidy/checks/bugprone-stream-int8.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/bugprone-stream-int8.rst @@ -0,0 +1,29 @@ +.. title:: clang-tidy - bugprone-stream-int8 + +bugprone-stream-int8 +==================== + +Checks that objects of type ``int8_t`` or ``uint8_t`` aren't streamed, if those +are typedefs to ``signed char`` or ``unsigned char``, as this likely intends to +be streaming these types as ``int`` s instead. + +Examples: + +.. code-block:: c++ + + uint8_t value = 0; + std::cout << "Value is " << value; // prints ^@ instead of likely intended 0 + +Options +------- + +.. option:: StreamTypes + + A semicolon-separated list of class names that should be treated as streams. + By default only ``std::basic_ostream`` is considered. + +.. option:: AliasTypes + + A semicolon-separated list of alias names that should be treated as integer + types to check against. By default only ``int8_t`` and ``uint8_t`` are + considered. Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,7 +57,12 @@ Improvements to clang-tidy -------------------------- -- ... +- New `bugprone-stream-int8_t + <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-stream-int8_t.html>`_ check + + Checks that objects of type ``int8_t`` or ``uint8_t`` aren't streamed, if those + are typedefs to ``signed char`` or ``unsigned char``, as this likely intends to + be streaming these types as ``int`` s instead. Improvements to include-fixer ----------------------------- Index: clang-tidy/bugprone/StreamInt8Check.h =================================================================== --- /dev/null +++ clang-tidy/bugprone/StreamInt8Check.h @@ -0,0 +1,43 @@ +//===--- StreamInt8Check.h - clang-tidy-----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_STREAMUINT8_TCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_STREAMUINT8_TCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Checks that objects of types int8_t or uint8_t aren't streamed to ostreams, +/// where the intended behavior is likely to stream them as ints +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-stream-int8.html +class StreamInt8Check : public ClangTidyCheck { +public: + StreamInt8Check(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + bool isBugproneAlias(StringRef Name) const; + + const std::vector<std::string> StreamTypes; + const std::vector<std::string> AliasTypes; + const ast_matchers::internal::Matcher<Decl> IsAStream; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_STREAMUINT8_TCHECK_H Index: clang-tidy/bugprone/StreamInt8Check.cpp =================================================================== --- /dev/null +++ clang-tidy/bugprone/StreamInt8Check.cpp @@ -0,0 +1,79 @@ +//===--- StreamInt8Check.cpp - clang-tidy----------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "StreamInt8Check.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +StreamInt8Check::StreamInt8Check(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + StreamTypes(utils::options::parseStringList(Options.get( + "StreamTypes", + "std::basic_ostream"))), + AliasTypes(utils::options::parseStringList(Options.get( + "AliasTypes", + "int8_t;uint8_t"))), + IsAStream(cxxRecordDecl(hasAnyName(std::vector<StringRef>( + StreamTypes.begin(), StreamTypes.end())))) +{ } + +void StreamInt8Check::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "StreamTypes", + utils::options::serializeStringList(StreamTypes)); + Options.store(Opts, "AliasTypes", + utils::options::serializeStringList(AliasTypes)); +} + +void StreamInt8Check::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + cxxOperatorCallExpr( + hasOverloadedOperatorName("<<"), + hasArgument(0, expr(hasType(qualType(hasCanonicalType( + hasDeclaration(IsAStream)))))), + hasArgument(1, expr(hasType(hasCanonicalType( + anyOf(asString("signed char"), + asString("const signed char"), + asString("unsigned char"), + asString("const unsigned char"))))) + .bind("offender")) + ), this); +} + +bool StreamInt8Check::isBugproneAlias(StringRef Name) const { + return std::find(AliasTypes.begin(), AliasTypes.end(), Name) + != AliasTypes.end(); +} + +void StreamInt8Check::check(const MatchFinder::MatchResult &Result) { + const auto *Offender = Result.Nodes.getNodeAs<Expr>("offender"); + + for (auto Typedef = Offender->getType().getTypePtr()->getAs<TypedefType>(); Typedef; ) { + auto Name = Typedef->getDecl()->getNameAsString(); + if (isBugproneAlias(Name)) { + diag(Offender->getLocStart(), + "streaming %0; did you mean to stream an int?") + << Name << Offender->getSourceRange(); + break; + } + + QualType Underlying = Typedef->getDecl()->getUnderlyingType(); + Typedef = Underlying.getTypePtr()->getAs<TypedefType>(); + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tidy/bugprone/CMakeLists.txt +++ clang-tidy/bugprone/CMakeLists.txt @@ -14,6 +14,7 @@ MisplacedOperatorInStrlenInAllocCheck.cpp MoveForwardingReferenceCheck.cpp MultipleStatementMacroCheck.cpp + StreamInt8Check.cpp StringConstructorCheck.cpp SuspiciousMemsetUsageCheck.cpp UndefinedMemoryManipulationCheck.cpp Index: clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tidy/bugprone/BugproneTidyModule.cpp @@ -22,6 +22,7 @@ #include "MisplacedOperatorInStrlenInAllocCheck.h" #include "MoveForwardingReferenceCheck.h" #include "MultipleStatementMacroCheck.h" +#include "StreamInt8Check.h" #include "StringConstructorCheck.h" #include "SuspiciousMemsetUsageCheck.h" #include "UndefinedMemoryManipulationCheck.h" @@ -59,6 +60,8 @@ "bugprone-move-forwarding-reference"); CheckFactories.registerCheck<MultipleStatementMacroCheck>( "bugprone-multiple-statement-macro"); + CheckFactories.registerCheck<StreamInt8Check>( + "bugprone-stream-int8"); CheckFactories.registerCheck<StringConstructorCheck>( "bugprone-string-constructor"); CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits