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

[Aaron Ballman via Phabricator via cfe-commits]

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 <typename T>
----------------
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