[PATCH] D33470: [clang-tidy] Add misc-default-numerics
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 26 06:44:04 PDT 2017
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?
More information about the cfe-commits