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

Reply via email to