Quuxplusone added inline comments.
================ Comment at: clang-tidy/readability/MagicNumbersCheck.h:52 + + const std::vector<std::string> IngnoredValuesInput; + std::vector<int64_t> IngnoredValues; ---------------- `Ignored`. Please spellcheck throughout. ================ Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:33 + + return -3; // FILENOTFOUND + } ---------------- I notice you changed `-1` to `-3`. Is `return -1` not an example of the "no magic numbers" rule? If not, why not — and can you put something in the documentation about it? At least add a unit test proving that `return -1;` is accepted without any diagnostic, if that's the intended behavior. ================ Comment at: test/clang-tidy/readability-magic-numbers.cpp:16 +void BuggyFunction() { + int BadLocalInt = 6; + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: magic number integer literal 6 [readability-magic-numbers] ---------------- Please add test cases immediately following this one, for const int BadLocalConstInt = 6; constexpr int BadLocalConstexprInt = 6; static const int BadLocalStaticConstInt = 6; static constexpr int BadLocalStaticConstexprInt = 6; (except of course changing "Bad" to "Good" in any cases where 6 is acceptable as an initializer). Also std::vector<int> BadLocalVector(6); const std::vector<int> BadLocalConstVector(6); etc. etc. ================ Comment at: test/clang-tidy/readability-magic-numbers.cpp:84 +void SolidFunction() { + const int GoodLocalIntContant = 43; + ---------------- `Constant`. Please spellcheck throughout. ================ Comment at: test/clang-tidy/readability-magic-numbers.cpp:151 + +const geometry::Rectangle<double> mandelbrotCanvas{geometry::Point<double>{-2.5, -1}, geometry::Dimension<double>{3.5, 2}}; ---------------- Surely you should expect some diagnostics on this line! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits