[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the patch and great discussion! I've commit in r339516. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. In https://reviews.llvm.org/D49114#1196480, @aaron.ballman wrote: > Alex has had plenty of time to respond, so I'm fine handling any concerns he > has post-commit. Do you need me to commit this on your behalf? Yes, please! Thank you! Author: "Florin Iucha " Repo

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Alex has had plenty of time to respond, so I'm fine handling any concerns he has post-commit. Do you need me to commit this on your behalf? Repository: rCTE Clang Tools Extra

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 160243. 0x8000- added a comment. Rebased on curent master. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readability/CMakeLists.txt clang-ti

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 159222. 0x8000- added a comment. Address several style comments. Rebase to current trunk (8.0). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added a comment. I think this is close. If @alexfh doesn't chime in on the open question in the next few days, I would say the check is ready to go in and we can address the open question in follow-up commits. Comment at: clang-tidy/readability/MagicNumbersCheck

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 158450. 0x8000- added a comment. Add reference to 5.1.1 Use symbolic names instead of literal values in code in the documentation. Repository: rCTE C

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 158425. 0x8000- added a comment. Add tests to show that we can detect, report and ignore floating point numbers represented using hexadecimal notation Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcore

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

JonasToth added a comment. In https://reviews.llvm.org/D49114#1182071, @0x8000- wrote: > The state of this patch: > > - user interface is as agreed-upon, giving end-users the capability to filter > out floats and ints as it makes sense for their code base > - code is clean > - documentation

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. The state of this patch: - user interface is as agreed-upon, giving end-users the capability to filter out floats and ints as it makes sense for their code base - code is clean - documentation is up to date - default ignore lists are sensible There is one outstandin

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157899. 0x8000- added a comment. Update the list of magic values ignored by default. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readability

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. > Based on this, I think the integer list should also include 2, 3, and 4 as > defaults -- those show up a lot more than I'd have expected. As for > floating-point values, 1.0 certainly jumps out at me, but none of the rest > seem particularly common. What do you th

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added a comment. In https://reviews.llvm.org/D49114#1179652, @0x8000- wrote: > Top 40 magic numbers in https://github.com/qt/qtbase > > 4859 2 > 2901 3 > 1855 4 >985 5 >968 8 >605 6 >600 7 >439 16 >432 10 >363 >356 32 >241 1.0f >217

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. Top 40 magic numbers in https://github.com/qt/qtbase 4859 2 2901 3 1855 4 985 5 968 8 605 6 600 7 439 16 432 10 363 356 32 241 1.0f 217 12 209 255 207 100 205 9 205 20 204 50 177 0.5 174 15 162 0x2 144 24

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &Inpu

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157889. 0x8000- added a comment. Add a flag to ignore all powers of two integral values. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readabi

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &InputV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:57 +const char DefaultIgnoredIntegerValues[] = "0;1;"; +const char DefaultIgnoredFloatingPointValues[] = "0.0;"; + aaron.ballman wrote: > 0x8000- wrote: > > aaron.ball

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:57 +const char DefaultIgnoredIntegerValues[] = "0;1;"; +const char DefaultIgnoredFloatingPointValues[] = "0.0;"; + 0x8000- wrote: > aaron.ballman wrote: > > 0x8000-0

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:57 +const char DefaultIgnoredIntegerValues[] = "0;1;"; +const char DefaultIgnoredFloatingPointValues[] = "0.0;"; + aaron.ballman wrote: > 0x8000- wrote: > > aaron.ball

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size()); + for (const auto &Inpu

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 2 inline comments as done. 0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86 + IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size()); + IgnoredDoublePointValues.reserve(IgnoredFloatingPointV

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:57 +const char DefaultIgnoredIntegerValues[] = "0;1;"; +const char DefaultIgnoredFloatingPointValues[] = "0.0;"; + 0x8000- wrote: > aaron.ballman wrote: > > I would

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 4 inline comments as done. 0x8000- added a comment. See inline comments. Basically we need two arrays because APFloats of different semantics don't compare well, and even if we coerce them, they sometimes are not equal. Comment at: clang-tidy/readabilit

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157886. 0x8000- added a comment. Indicate that `0` and `0.0` are accepted unconditionally (because it makes sense in the source code, and speeds-up many checks as 0s are very common and we don't want to spend log2(n) to find them at the beginning of

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added a comment. Thank you for working on this, I think it's getting closer! I'd use a slightly different approach to handling floating-point values, but if that turns out to be a clean implementation we may want to think about whether there are improvements from modelling integer

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157879. 0x8000- added a comment. Add support for ignoring specific floating point numbers. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/reada

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. Not trying to be difficult here - I have attempted to implement the straight-forward check. Added this to the MagicNumbersCheck::MagicNumbersCheck constructor: + // process set of ignored floating point values + const std::vector IgnoredFloatingPointValuesInpu

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable. ---

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable. -

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable. ---

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157871. 0x8000- added a comment. Add option to ignore all floating point values. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readability/CMa

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable.

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable. -

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. I will add one more option: IgnoreFloatingPointValues, to ignore all floats and doubles, because the FloatingLiteral does not distinguish between them (as implementation detail), and I don't see a good reason to be strict about doubles and lenient about floats, or v

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable. ---

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

lebedev.ri added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable.

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:61-63 +configuration for accepted floating point values, primarily because most +floating point comparisons are not exact, and some of the exact ones are not +portable. -

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 7 inline comments as done. 0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:53 + +const char DefaultIgnoredIntegerValues[] = "0;1;"; + aaron.ballman wrote: > Is the trailing semicolon after the `1` nec

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157863. 0x8000- added a comment. Address review comments to improve documentation and readability Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tid

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:45-46 "cppcoreguidelines-interfaces-global-init"); +CheckFactories.registerCheck( +"cppcoreguidelines-avoid-magic-numbers"); CheckFactories.regi

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157615. 0x8000- marked an inline comment as done. 0x8000- added a comment. Remove extra verbose namespaces and update documentation to indicate why -1 is accepted even when not explicitly called out. Repository: rCTE Clang Tools Extra https:/

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 3 inline comments as done. 0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:55 + +By default only `0`, `1` and `-1` integer values are accepted without a warning. +This can be overridden with the :option:`Igno

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

JonasToth added a comment. I did not find any major issue :) Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:20 +bool isUsedToInitializeAConstant( +const clang::ast_matchers::MatchFinder::MatchResult &Result, +const clang::ast_type_traits::DynTypedNode &Node) {

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157187. 0x8000- added a comment. Update links in documentation Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readability/CMakeLists.txt clan

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:56 +By default only `0`, `1` and `-1` integer values are accepted without a warning. +This can be overridden with the 'IgnoredIntegerValues' option. In addition, +the `0.0` fl

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157186. 0x8000- added a comment. Fix a few typos Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy/readability/CMakeLists.txt clang-tidy/readabi

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Eugene.Zelenko added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.h:42 +return false; + }; + bool isSyntheticValue(const clang::SourceManager *SourceManager, Unnecessary semicolon. Please also add empty line after. ===

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. @aaron.ballman , @JonasToth , @Eugene.Zelenko - would you be so kind to do one more review pass and let me know if there are any blockers or if this is ready to merge? I do not have commit privileges, so if it is alright, feel free to merge it. Repository: rCTE

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 157000. 0x8000- added a comment. Bail out early if a [grand-]parent node is a declaration, but not a constant declaration. Search for enumerations and declarations in the same tree traversal. Repository: rCTE Clang Tools Extra https://reviews.llv

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 156950. 0x8000- added a comment. Ignore literals implicitly added by the compiler, such as when using a range-for loop over a constant array. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguideline

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 156430. 0x8000- added a comment. Avoid parsing and reformatting the input literal - just print the original source code. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTi

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. ./tools/clang/tools/extra/clang-tidy/tool/run-clang-tidy.py -clang-tidy-binary ../llvm.rel/bin/clang-tidy -checks="-*,readability-magic-numbers" -j 12 -p ../llvm.rel -j 12 -quiet > /tmp/llvm.magic grep "warning:" /tmp/llvm.magic | cut -d: -f5 | cut -d" " -f2 | s

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 156425. 0x8000- added a comment. Filter out synthetic integers (such as _LINE_) from the report. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tidy

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 156422. 0x8000- added a comment. Add a (presently failing) test for not tripping up on __LINE__ through several layers of macro expansion (as in GoogleTest library). This creates a lot of false positives in the unit tests and needs to be fixed. Rep

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 156392. 0x8000- added a comment. Small refactoring and documentation update. Revert built-in acceptable integers to -1, 0 and 1 and document them. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/cppcoreguid

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 3 inline comments as done. 0x8000- added inline comments. 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 [readabili

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + 0x8000- wrote: > aaron.ballman wrote: > > lebedev.ri wrote: > > > aaron.ballman wrote: > > > > Why 2, 10

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked an inline comment as done. 0x8000- added inline comments. 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 [readabili

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 156203. 0x8000- added a comment. Herald added subscribers: kbarton, nemanjai. Significant refactoring to address review comments - mainly to reduce duplication and implement in functional style. cppcoreguidelines-avoid-magic-numbers is documented as

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. I think this requires a separate discussion - do we accept magic values only when they are used directly to initialize a constant of the same type? Or do we allow them generically when they are used to initialize any constant? Or do we only accept several layers of

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 8 inline comments as done. 0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:33 + + return -3; // FILENOTFOUND + } Quuxplusone wrote: > I notice you changed `-1` to `-3`. Is `return -1` n

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 17 inline comments as done. 0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:30-32 + for (const std::string &IgnoredValue : IngnoredValuesInput) { +IngnoredValues.push_back(std::stoll(IgnoredValue)); + }

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Quuxplusone added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.h:52 + + const std::vector IngnoredValuesInput; + std::vector IngnoredValues; `Ignored`. Please spellcheck throughout. Comment at: docs/clang-tidy/checks/

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + aaron.ballman wrote: > lebedev.ri wrote: > > aaron.ballman wrote: > > > Why 2, 10, and 100? > > These really s

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + lebedev.ri wrote: > aaron.ballman wrote: > > Why 2, 10, and 100? > These really should be a config option. T

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

lebedev.ri added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + aaron.ballman wrote: > Why 2, 10, and 100? These really should be a config option. Repository: rCTE Clang T

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + Why 2, 10, and 100? Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:30-32 + for

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked an inline comment as done. 0x8000- added a comment. florin@helios:~/tools/llvm$ find . -name .clang-format | sort ./.clang-format ./projects/compiler-rt/lib/asan/.clang-format ./projects/compiler-rt/lib/dfsan/.clang-format ./projects/compiler-rt/lib/hwasan/.clang-format ./pr

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: test/clang-tidy/readability-magic-numbers.cpp:124 + + explicit Point(T xval, T yval) noexcept : x{xval}, y{yval} { + } Eugene.Zelenko wrote: > 0x8000- wrote: > > Eu

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 155994. 0x8000- added a comment. Address several review comments. Create alias for cppcoreguidelines-avoid-magic-numbers . Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/readability/CMakeLists.txt clang-

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Eugene.Zelenko added inline comments. Comment at: test/clang-tidy/readability-magic-numbers.cpp:124 + + explicit Point(T xval, T yval) noexcept : x{xval}, y{yval} { + } 0x8000- wrote: > Eugene.Zelenko wrote: > > Please run Clang-format over test case. > I d

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 4 inline comments as done. 0x8000- added a comment. @Eugene.Zelenko thank you for suggesting the alias - I didn't know it is that easy, but a grep on "alias" led me to the right place. Comment at: clang-tidy/readability/MagicNumbersCheck.h:55 + const s

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Eugene.Zelenko added a comment. Adding C++ Core Guidelines alias is definitely low-hanging fruit which could be implemented within this patch. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:14 +#include "clang/ASTMatchers/ASTMatchFinder.h" + +#include --

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

JonasToth added a comment. In https://reviews.llvm.org/D49114#1164788, @0x8000- wrote: > @aaron.ballman , @JonasToth , @Eugene.Zelenko Is there anything missing from > this patch? What do I need to do to get it merged? This is my first > contribution to LLVM so I'm not quite sure. Thank you

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. @aaron.ballman , @JonasToth , @Eugene.Zelenko Is there anything missing from this patch? What do I need to do to get it merged? This is my first contribution to LLVM so I'm not quite sure. Thank you! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D4

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 155549. 0x8000- added a comment. Accept magic values arbitrarily deep in a constant initialization Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/MagicNum

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked an inline comment as done. 0x8000- added a comment. Remove extraneous .html. Sorry for not seeing it earlier. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 155089. 0x8000- added a comment. Fix typo Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/MagicNumbersCheck.cpp clang-tidy/readability/MagicNumbersCheck.

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:158 +- New :doc:`readability-magic-numbers + ` check. + .html is not needed. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 __

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 3 inline comments as done. 0x8000- added a comment. Thank you for your review @Eugene.Zelenko Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 155084. 0x8000- added a comment. Update documentation to clean up formatting Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/MagicNumbersCheck.cpp clang-

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:158 +- New :doc:`readability-magic-numbers + ` check. + Please make link short. See other links as examples.

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 155078. 0x8000- edited the summary of this revision. 0x8000- added a comment. Updated examples code and added several references Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Quuxplusone added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:11 + + double circleArea = 3.1415926535 * radius * radius; + 0x8000- wrote: > Quuxplusone wrote: > > 0x8000- wrote: > > > Eugene.Zelenko wrote: > > > > J

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:11 + + double circleArea = 3.1415926535 * radius * radius; + Quuxplusone wrote: > 0x8000- wrote: > > Eugene.Zelenko wrote: > > > JonasToth wrote: > > > > Thi

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Eugene.Zelenko added a comment. In https://reviews.llvm.org/D49114#1159327, @0x8000- wrote: > In https://reviews.llvm.org/D49114#1159211, @Eugene.Zelenko wrote: > > > C++ Core Guidelines contains ES.45: Avoid "magic constants"; use symbolic > > constants > >

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added a comment. In https://reviews.llvm.org/D49114#1159211, @Eugene.Zelenko wrote: > C++ Core Guidelines contains ES.45: Avoid "magic constants"; use symbolic > constants > , > so I think check

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Quuxplusone added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:11 + + double circleArea = 3.1415926535 * radius * radius; + 0x8000- wrote: > Eugene.Zelenko wrote: > > JonasToth wrote: > > > This example is good, but righ

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Eugene.Zelenko added a comment. C++ Core Guidelines contains ES.45: Avoid "magic constants"; use symbolic constants , so I think check should be moved into cppcoreguidelines module. Repository: rCTE Clan

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- updated this revision to Diff 154921. 0x8000- added a comment. Herald added a subscriber: mgrang. Incorporate review comments. Add support for floating point detection. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 Files: clang-tidy/readability/CMakeLis

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- marked 7 inline comments as done. 0x8000- added inline comments. Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:11 + + double circleArea = 3.1415926535 * radius * radius; + Eugene.Zelenko wrote: > JonasToth wrote: > > This exam

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

0x8000- added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:42 + +const auto &Parents = Result.Context->getParents(*MatchedDecl); +for (const auto &Parent : Parents) { Eugene.Zelenko wrote: > Please don't use auto where typ

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

Eugene.Zelenko added a comment. It's highly likely that this part of coding guidelines. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:42 + +const auto &Parents = Result.Context->getParents(*MatchedDecl); +for (const auto &Parent : Parents) {

  1   2   >