[PATCH] D33470: [clang-tidy] Add misc-default-numerics
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. Should the check be in "bugprone-" instead? https://reviews.llvm.org/D33470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
aaron.ballman added inline comments.
Comment at: test/clang-tidy/misc-default-numerics.cpp:28
+struct numeric_limits {
+ static int min() { return -1; }
+ static SpecializedType max() { return SpecializedType(); }
This is not a proper specialization according to the standard.
> A program may add a template specialization for any standard library template
> to namespace std only if the declaration depends on a user-defined type and
> the specialization meets the standard library requirements for the original
> template and is not explicitly prohibited.
The functions are not marked `constexpr` or `noexcept`, and `min()` must return
`SpecializedType`. Also the `is_specialized` member needs to be set to `true`.
There are other requirements missing as well, but I think fixing the function
signatures is the only important one.
https://reviews.llvm.org/D33470
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
Prazek updated this revision to Diff 104297.
Prazek added a comment.
fixed broken test
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,56 @@
+// RUN: %check_clang_tidy %s misc-default-numerics %t
+
+namespace std {
+
+template
+struct numeric_limits {
+ static T min() { return T(); }
+ static T max() { return T(); }
+};
+
+template <>
+struct numeric_limits {
+ static int min() { return -1; }
+ static int max() { return 1; }
+};
+
+} // namespace std
+
+class MyType {};
+template
+class MyTemplate {};
+
+class SpecializedType {};
+
+namespace std {
+template<>
+struct numeric_limits {
+ static int min() { return -1; }
+ static SpecializedType max() { return SpecializedType(); }
+
+};
+}
+
+void test() {
+ auto x = std::numeric_limits::min();
+
+ auto y = std::numeric_limits::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_limits::max();
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::numeric_limits::max' called with type 'MyTemplate';
+
+ auto a = std::numeric_limits::min();
+ auto b = std::numeric_limits::max();
+}
+
+template
+void fun() {
+ auto x = std::numeric_limits::min();
+}
+
+void testTemplate() {
+ fun();
+ // FIXME: This should generate warning with backtrace.
+ fun;
+}
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::{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::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
@@ -76,6 +76,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
@@ -78,6 +78,10 @@
Allow custom memory management functions to be considered as well.
+- New `misc-default-numerics
+ `_ check
+ Finds uses of ``std::numeric_limits`` for unspecialized types
+
- New `misc-forwarding-reference-overload
`_ 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"
@@ -67,6 +68,7 @@
CheckFactories.registerCheck("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck("misc-default-numerics");
CheckFactories.registerCheck(
"misc-forwarding-reference-overload");
CheckFactories.registerCheck(
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.
+//
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
Prazek updated this revision to Diff 104288.
Prazek added a comment.
Small fix
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,56 @@
+// RUN: %check_clang_tidy %s misc-default-numerics %t
+
+namespace std {
+
+template
+struct numeric_limits {
+ static T min() { return T(); }
+ static T max() { return T(); }
+};
+
+template <>
+struct numeric_limits {
+ static int min() { return -1; }
+ static int max() { return 1; }
+};
+
+} // namespace std
+
+class MyType {};
+template
+class MyTemplate {};
+
+class SpecializedMin {};
+
+namespace std {
+template<>
+struct numeric_limits {
+ static int min() { return -1; }
+};
+}
+
+void test() {
+ auto x = std::numeric_limits::min();
+
+ auto y = std::numeric_limits::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_limits::max();
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::numeric_limits::max' called with type 'MyTemplate';
+
+ auto a = std::numeric_limits::min();
+
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::numeric_limits::max' called with type 'SpecializedMin';
+ auto b = std::numeric_limits::max();
+}
+
+template
+void fun() {
+ auto x = std::numeric_limits::min();
+}
+
+void testTemplate() {
+ fun();
+ // FIXME: This should generate warning with backtrace.
+ fun;
+}
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::{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::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
@@ -76,6 +76,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
@@ -78,6 +78,10 @@
Allow custom memory management functions to be considered as well.
+- New `misc-default-numerics
+ `_ check
+ Finds uses of ``std::numeric_limits`` for unspecialized types
+
- New `misc-forwarding-reference-overload
`_ 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"
@@ -67,6 +68,7 @@
CheckFactories.registerCheck("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck("misc-default-numerics");
CheckFactories.registerCheck(
"misc-forwarding-reference-overload");
CheckFactories.registerCheck(
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
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
Prazek updated this revision to Diff 104281.
Prazek marked 2 inline comments as done.
Prazek added a comment.
Herald added a subscriber: JDevlieghere.
- fixed docs
- fixes
- Last fixes?
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,56 @@
+// RUN: %check_clang_tidy %s misc-default-numerics %t
+
+namespace std {
+
+template
+struct numeric_limits {
+ static T min() { return T(); }
+ static T max() { return T(); }
+};
+
+template <>
+struct numeric_limits {
+ static int min() { return -1; }
+ static int max() { return 1; }
+};
+
+} // namespace std
+
+class MyType {};
+template
+class MyTemplate {};
+
+class SpecializedMin {};
+
+namespace std {
+template<>
+struct numeric_limits {
+ static int min() { return -1; }
+};
+
+
+void test() {
+ auto x = std::numeric_limits::min();
+
+ auto y = std::numeric_limits::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_limits::max();
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::numeric_limits::max' called with type 'MyTemplate';
+
+ auto a = std::numeric_limits::min();
+
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::numeric_limits::max' called with type 'SpecializedMin';
+ auto b = std::numeric_limits::max();
+}
+
+template
+void fun() {
+ auto x = std::numeric_limits::min();
+}
+
+void testTemplate() {
+ fun();
+ // FIXME: This should generate warning with backtrace.
+ fun;
+}
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::{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::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
@@ -76,6 +76,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
@@ -78,6 +78,10 @@
Allow custom memory management functions to be considered as well.
+- New `misc-default-numerics
+ `_ check
+ Finds uses of ``std::numeric_limits`` for unspecialized types
+
- New `misc-forwarding-reference-overload
`_ 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"
@@ -67,6 +68,7 @@
CheckFactories.registerCheck("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck("misc-default-numerics");
CheckFactories.registerCheck(
"misc-forwarding-reference-overload");
CheckFactories.registerCheck(
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
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
Prazek added a comment. In https://reviews.llvm.org/D33470#790484, @aaron.ballman wrote: > In https://reviews.llvm.org/D33470#789791, @Prazek wrote: > > > In https://reviews.llvm.org/D33470#764846, @aaron.ballman wrote: > > > > > Once you fix the typo in the check, can you run it over some large C++ > > > code bases to see if it finds any results? > > > > > > I tried it on LLVM code base (after fixing bug with the numeric_limits > > name) and it didn't find anything suspisious. > > Unfortunatelly I don't have enough time to try it on different codebases, > > but I am weiling to fix any bug with this check if it would happen in the > > future. > > The release 5.0 is near, so I would like to push it upstream. Does it > > sound good to you? > > > My concern is: does this find any actual issues in real world code? This > seems like such a highly specific check -- not many people use numeric_limits > in the first place, let alone on non-builtin types, so does it justify > running this check when someone batch-includes all of the misc checks? > > I don't think this check is going to trigger a ton of false positives. I am > wondering more the opposite: will this check ever trigger on anything other > than compiler test cases? The check is based on the real world scenario that my friend had at work, so at least I know about one such case :) It probably won't be a very popular check in terms of fiding anything, but it should find real bugs. The matcher is very easy so the check should be fast. https://reviews.llvm.org/D33470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
aaron.ballman added a comment. In https://reviews.llvm.org/D33470#789791, @Prazek wrote: > In https://reviews.llvm.org/D33470#764846, @aaron.ballman wrote: > > > Once you fix the typo in the check, can you run it over some large C++ code > > bases to see if it finds any results? > > > I tried it on LLVM code base (after fixing bug with the numeric_limits name) > and it didn't find anything suspisious. > Unfortunatelly I don't have enough time to try it on different codebases, > but I am weiling to fix any bug with this check if it would happen in the > future. > The release 5.0 is near, so I would like to push it upstream. Does it sound > good to you? My concern is: does this find any actual issues in real world code? This seems like such a highly specific check -- not many people use numeric_limits in the first place, let alone on non-builtin types, so does it justify running this check when someone batch-includes all of the misc checks? I don't think this check is going to trigger a ton of false positives. I am wondering more the opposite: will this check ever trigger on anything other than compiler test cases? Comment at: clang-tidy/misc/DefaultNumericsCheck.h:21 +/// unspecialized types. It is dangerous because it returns T(), which rarely +/// might be minimum or maximum for this type. +/// the minimum or maximum (add the "the"). Comment at: test/clang-tidy/misc-default-numerics.cpp:32 +} + +template Can you add a test case where numeric_limits has been properly specialized for the type and the type is not a builtin? https://reviews.llvm.org/D33470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
Prazek added a comment. In https://reviews.llvm.org/D33470#764846, @aaron.ballman wrote: > Once you fix the typo in the check, can you run it over some large C++ code > bases to see if it finds any results? I tried it on LLVM code base (after fixing bug with the numeric_limits name) and it didn't find anything suspisious. Unfortunatelly I don't have enough time to try it on different codebases, but I am weiling to fix any bug with this check if it would happen in the future. The release 5.0 is near, so I would like to push it upstream. Does it sound good to you? https://reviews.llvm.org/D33470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
Prazek updated this revision to Diff 103837.
Prazek marked 4 inline comments as done.
Prazek added a comment.
- fixed docs
- fixes
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
+struct numeric_limits {
+ static T min() { return T(); }
+ static T max() { return T(); }
+};
+
+template <>
+struct numeric_limits {
+ static int min() { return -1; }
+ static int max() { return 1; }
+};
+
+} // namespace std
+
+class MyType {};
+template
+class MyTemplate {};
+
+void test() {
+ auto x = std::numeric_limits::min();
+
+ auto y = std::numeric_limits::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_limits::max();
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::numeric_limits::max' called with type 'MyTemplate';
+}
+
+template
+void fun() {
+ auto x = std::numeric_limits::min();
+}
+
+void testTemplate() {
+ fun();
+ // FIXME: This should generate warning with backtrace.
+ fun;
+}
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::{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::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
@@ -72,6 +72,10 @@
Allow custom memory management functions to be considered as well.
+- New `misc-default-numerics
+ `_ check
+ Finds uses of ``std::numeric_limits`` for unspecialized types
+
- New `misc-forwarding-reference-overload
`_ 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"
@@ -67,6 +68,7 @@
CheckFactories.registerCheck("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck("misc-default-numerics");
CheckFactories.registerCheck(
"misc-forwarding-reference-overload");
CheckFactories.registerCheck(
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
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
alexshap added inline comments.
Comment at: clang-tidy/misc/DefaultNumericsCheck.h:20
+/// This check flags usages of ``std::numeric_limits::{min,max}()`` for
+/// unspecialized types. It is dangerous because it returns T(), which might is
+/// rarely minimum or maximum for this type.
nit: (feel free to correct me)
replace
"which might is ..."
with
"which rarely might be minimum or maximum for this type"
https://reviews.llvm.org/D33470
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
aaron.ballman added a comment.
Once you fix the typo in the check, can you run it over some large C++ code
bases to see if it finds any results?
Comment at: clang-tidy/misc/DefaultNumericsCheck.cpp:30
+ ofClass(classTemplateSpecializationDecl(
+ hasName("::std::numeric_limit"),
+ unless(isExplicitTemplateSpecialization()),
This should be checking for `::std::numeric_limits` (plural).
Comment at: docs/ReleaseNotes.rst:77
+ `_
check
+ Finds uses of ``std::numeric_limit`` for unspecialized types
+
numeric_limits
Comment at: test/clang-tidy/misc-default-numerics.cpp:6
+template
+struct numeric_limit {
+ static T min() { return T(); }
numeric_limits (same elsewhere in this file).
https://reviews.llvm.org/D33470
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
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
+struct numeric_limit {
+ static T min() { return T(); }
+ static T max() { return T(); }
+};
+
+template <>
+struct numeric_limit {
+ static int min() { return -1; }
+ static int max() { return 1; }
+};
+
+} // namespace std
+
+class MyType {};
+template
+class MyTemplate {};
+
+void test() {
+ auto x = std::numeric_limit::min();
+
+ auto y = std::numeric_limit::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::max();
+ // CHECK-MESSAGES: [[@LINE-1]]:12: warning: 'std::numeric_limits::max' called with type 'MyTemplate';
+}
+
+template
+void fun() {
+ auto x = std::numeric_limit::min();
+}
+
+void testTemplate() {
+ fun();
+ // FIXME: This should generate warning with backtrace.
+ fun;
+}
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::{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::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 @@
`_ check
Allow custom memory management functions to be considered as well.
-
+
+- New `misc-default-numerics
+ `_ check
+ Finds uses of ``std::numeric_limit`` for unspecialized types
+
- New `misc-forwarding-reference-overload
`_ 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("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck("misc-default-numerics");
CheckFactories.registerCheck(
"misc-forwarding-reference-overload");
CheckFactories.registerCheck("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
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/DefaultNumericsCheck.cpp:37
+void DefaultNumericsCheck::check(const MatchFinder::MatchResult ) {
+
+ const auto *MatchedDecl = Result.Nodes.getNodeAs("call");
Can remove the spurious newline.
Comment at: clang-tidy/misc/DefaultNumericsCheck.cpp:41
+ diag(MatchedDecl->getLocStart(),
+ "called std::numeric_limit method on not specialized type");
+}
This diagnostic doesn't tell the user what's wrong with their code. I think
something along these lines might be better:
`'std::numeric_limits::%select{max|min}0' called with type %1; no such
specialization exists, so the default value for that type is returned`
Comment at: clang-tidy/misc/DefaultNumericsCheck.h:20
+/// This check flags usages of ``std::numeric_limits::{min,max}()`` for
+/// unspecialized types. It is dangerous because returns T(), which might is
+/// rarely minimum or maximum for this type.
because returns -> because it returns
Comment at: docs/clang-tidy/checks/misc-default-numerics.rst:7
+This check flags usages of ``std::numeric_limits::{min,max}()`` for
+unspecialized types. It is dangerous because returns T(), which might is rarely
+minimum or maximum for this type.
because returns... -> because the calls return ''T()'' in this case, which is
unlikely to represent the minimum or maximum value for the type.
Comment at: docs/clang-tidy/checks/misc-default-numerics.rst:19
+
+
+ // 3. Replace the BigInt typedef with class implementing BigIntegers
Remove spurious newline
Comment at: docs/clang-tidy/checks/misc-default-numerics.rst:24
+ // 4. Your code compiles silently and you a few years later you find an
+ // of by 9223372036854775808 error.
Is this meant to say "off by X error"? I think a better wording would be that
the code continues to compile, but the call to `min()` returns `BigInt{}`, or
something more explicit.
https://reviews.llvm.org/D33470
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
