[PATCH] D33470: [clang-tidy] Add misc-default-numerics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/misc/DefaultNumericsCheck.cpp:37
+void DefaultNumericsCheck::check(const MatchFinder::MatchResult &Result) {
+
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<CallExpr>("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<T>::{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<T>::{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