Prazek updated this revision to Diff 100216.
Prazek marked 8 inline comments as done.
Prazek added a comment.
- Thanks for the review Aaron, that is much better.


https://reviews.llvm.org/D33470

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/DefaultNumericsCheck.cpp
  clang-tidy/misc/DefaultNumericsCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-default-numerics.rst
  test/clang-tidy/misc-default-numerics.cpp

Index: test/clang-tidy/misc-default-numerics.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-default-numerics.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s misc-default-numerics %t
+
+namespace std {
+
+template <typename T>
+struct numeric_limit {
+  static T min() { return T(); }
+  static T max() { return T(); }
+};
+
+template <>
+struct numeric_limit<int> {
+  static int min() { return -1; }
+  static int max() { return 1; }
+};
+
+} // namespace std
+
+class MyType {};
+template <typename T>
+class MyTemplate {};
+
+void test() {
+  auto x = std::numeric_limit<int>::min();
+
+  auto y = std::numeric_limit<MyType>::min();
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::numeric_limits::min' called with type 'MyType'; no such specialization exists, so the default value for that type is returned
+
+  auto z = std::numeric_limit<MyTemplate<int>>::max();
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::numeric_limits::max' called with type 'MyTemplate<int>';
+}
+
+template <typename T>
+void fun() {
+  auto x = std::numeric_limit<T>::min();
+}
+
+void testTemplate() {
+  fun<int>();
+  // FIXME: This should generate warning with backtrace.
+  fun<MyType>;
+}
Index: docs/clang-tidy/checks/misc-default-numerics.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-default-numerics.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - misc-default-numerics
+
+misc-default-numerics
+=====================
+
+This check flags usages of ``std::numeric_limits<T>::{min,max}()`` for
+unspecialized types. It is dangerous because the calls return ``T()``
+in this case, which is unlikely to represent the minimum or maximum value for
+the type.
+
+Consider scenario:
+.. code-block:: c++
+
+  // 1. Have a:
+  typedef long long BigInt
+
+  // 2. Use
+  std::numeric_limits<BigInt>::min()
+
+  // 3. Replace the BigInt typedef with class implementing BigIntegers
+  class BigInt { ;;; };
+
+  // 4. Your code continues to compile, but the call to min() returns BigInt{}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -75,6 +75,7 @@
    misc-assert-side-effect
    misc-bool-pointer-implicit-conversion
    misc-dangling-handle
+   misc-default-numerics
    misc-definitions-in-headers
    misc-fold-init-type
    misc-forward-declaration-namespace
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -71,7 +71,11 @@
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-no-malloc.html>`_ check
 
   Allow custom memory management functions to be considered as well.
-  
+
+- New `misc-default-numerics
+  <http://clang.llvm.org/extra/clang-tidy/checks/misc-default-numerics.html>`_ check
+  Finds uses of ``std::numeric_limit<T>`` for unspecialized types
+
 - New `misc-forwarding-reference-overload
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-forwarding-reference-overload.html>`_ check
 
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -14,6 +14,7 @@
 #include "AssertSideEffectCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
 #include "DanglingHandleCheck.h"
+#include "DefaultNumericsCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
@@ -66,6 +67,7 @@
     CheckFactories.registerCheck<ArgumentCommentCheck>("misc-argument-comment");
     CheckFactories.registerCheck<AssertSideEffectCheck>(
         "misc-assert-side-effect");
+    CheckFactories.registerCheck<DefaultNumericsCheck>("misc-default-numerics");
     CheckFactories.registerCheck<ForwardingReferenceOverloadCheck>(
         "misc-forwarding-reference-overload");
     CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
Index: clang-tidy/misc/DefaultNumericsCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/DefaultNumericsCheck.h
@@ -0,0 +1,37 @@
+//===--- DefaultNumericsCheck.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_MISC_DEFAULT_NUMERICS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEFAULT_NUMERICS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This check flags usages of ``std::numeric_limits<T>::{min,max}()`` for
+/// unspecialized types. It is dangerous because it returns T(), which might is
+/// rarely minimum or maximum for this type.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-default-numerics.html
+class DefaultNumericsCheck : public ClangTidyCheck {
+public:
+  DefaultNumericsCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEFAULT_NUMERICS_H
Index: clang-tidy/misc/DefaultNumericsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/DefaultNumericsCheck.cpp
@@ -0,0 +1,51 @@
+//===--- DefaultNumericsCheck.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 "DefaultNumericsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void DefaultNumericsCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+  // FIXME: We should also warn in template intantiations, but it would require
+  // printing backtrace.
+  Finder->addMatcher(
+      callExpr(
+          callee(cxxMethodDecl(
+              hasAnyName("min", "max"),
+              ofClass(classTemplateSpecializationDecl(
+                  hasName("::std::numeric_limit"),
+                  unless(isExplicitTemplateSpecialization()),
+                  hasTemplateArgument(0, templateArgument().bind("type")))))),
+          unless(isInTemplateInstantiation()))
+          .bind("call"),
+      this);
+}
+
+void DefaultNumericsCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto &MatchedCall = *Result.Nodes.getNodeAs<CallExpr>("call");
+  const auto &NumericType = *Result.Nodes.getNodeAs<TemplateArgument>("type");
+  bool IsMax = MatchedCall.getCalleeDecl()->getAsFunction()->getName() == "max";
+
+  diag(MatchedCall.getLocStart(),
+       "'std::numeric_limits::%select{min|max}0' called with type %1; no such "
+       "specialization exists, so the default value for that type is returned")
+      << IsMax << NumericType;
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -3,6 +3,7 @@
 add_clang_library(clangTidyMiscModule
   ArgumentCommentCheck.cpp
   AssertSideEffectCheck.cpp
+  DefaultNumericsCheck.cpp
   ForwardingReferenceOverloadCheck.cpp
   MisplacedConstCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to