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
