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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
