0x8000-0000 marked 17 inline comments as done. 0x8000-0000 added inline comments.
================ Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:30-32 + for (const std::string &IgnoredValue : IngnoredValuesInput) { + IngnoredValues.push_back(std::stoll(IgnoredValue)); + } ---------------- aaron.ballman wrote: > This can be replaced with `llvm::transform(IgnoredValuesInput, > IgnoredValues.begin(), std::stoll);` /home/florin/tools/llvm/tools/clang/tools/extra/clang-tidy/readability/MagicNumbersCheck.cpp:30:3: error: no matching function for call to 'transform' llvm::transform(IgnoredValuesInput, IgnoredIntegerValues.begin(), std::stoll); ^~~~~~~~~~~~~~~ /home/florin/tools/llvm/include/llvm/ADT/STLExtras.h:990:10: note: candidate template ignored: couldn't infer template argument 'UnaryPredicate' OutputIt transform(R &&Range, OutputIt d_first, UnaryPredicate P) { ^ 1 error generated. Shall I wrap it in a lambda? ================ Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:41-42 +void MagicNumbersCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(integerLiteral().bind("ii"), this); + Finder->addMatcher(floatLiteral().bind("ff"), this); +} ---------------- aaron.ballman wrote: > The names `ii` and `ff` could be a bit more user-friendly. Also, this can be > written using a single matcher, I think. > `anyOf(integerLiteral().bind("integer"), floatLiteral().bind("float"))` addMatcher(anyOf(...), this) is ambiguous. Is there any benefit over the two statements? ================ Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:69-70 "readability-inconsistent-declaration-parameter-name"); + CheckFactories.registerCheck<MagicNumbersCheck>( + "readability-magic-numbers"); CheckFactories.registerCheck<MisleadingIndentationCheck>( ---------------- aaron.ballman wrote: > I think this should also be registered as a C++ Core Guideline checker, as I > believe it covers at least the integer and floating-point parts of > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-magic I have it as an alias in my branch. I will check why it was not exported in the patch. 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