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

2018-08-11 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-08-12 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-19 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-19 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-19 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-19 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-19 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-19 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-23 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-24 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-24 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-24 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-24 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-26 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-26 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-28 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-29 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-31 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-31 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-31 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-08-05 Thread Florin Iucha via Phabricator via cfe-commits
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: Add a clang-tidy check for "magic numbers"

2018-07-09 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- created this revision. 0x8000- added reviewers: Wizard, aaron.ballman, alexfh, hokein. Herald added subscribers: cfe-commits, mgorny. Add a clang-tidy check for "magic numbers", integers and floating point values embedded in the code instead of using symbols or constants. Bad exa

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

2018-07-09 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Perhaps M_PI wasn't the best example, as its value won't change soon, but other numbers should be defined in relation to constants. Also I have seen coding guidelines suggesting "100" is grandfathered due to 100% calculations. 2 and 10 due to logarithms, etc. Not su

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

2018-07-09 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In https://reviews.llvm.org/D49114#1156835, @hfinkel wrote: > I suspect that the check will be very noisy for powers of 2 and 10 that are > used as multiplicands. You might wish to exclude those. Good point. > Also, what happens for enums? Especially when initiali

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

2018-07-09 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In https://reviews.llvm.org/D49114#1156878, @Quuxplusone wrote: > The cult of "no magic numbers" is horrible and we should be trying to > //deprogram// its adherents, not create a whole new generation of them. I > would be happy if this clang-tidy patch were quickly

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

2018-07-09 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: test/clang-tidy/readability-magic-numbers.cpp:38 +public: + TwoIntContainer(int val) : anotherMember(val * val), yetAnotherMember(2), anotherConstant(val + val) {} + // CHECK-MESSAGES: :[[@LINE-1]]:73: warning: magic number intege

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

2018-07-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In https://reviews.llvm.org/D49114#1156987, @JonasToth wrote: > I think some of the logic you have in your check code could be done via > matchers. That is usually better for performance, because you analyze less > code. I have considered that approach, but had to

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

2018-07-10 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-10 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-10 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-11 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-11 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-11 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-11 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-11 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-11 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-11 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-13 Thread Florin Iucha via Phabricator via cfe-commits
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] D45444: [clang-tidy] implement new check for const-correctness

2018-07-14 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In https://reviews.llvm.org/D45444#1162715, @JonasToth wrote: > Time :/ Is this available as a public Git branch somewhere, or is downloading the diff the preferred way to interact with this code? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2018-07-17 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In https://reviews.llvm.org/D45444#1164671, @JonasToth wrote: > @0x8000- are you interested in working on the check? If i recall > correctly the branch for 7.0 will happen on 1. august. That would be the > timewindow in which it makes sense to give you the patch

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

2018-07-17 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-17 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-17 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-17 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-17 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-18 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-18 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-18 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-18 Thread Florin Iucha via Phabricator via cfe-commits
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"

2018-07-18 Thread Florin Iucha via Phabricator via cfe-commits
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] D45444: [clang-tidy] implement new check for const-correctness

2018-07-18 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Any plans for fix-its that will add the suggested 'const' qualifiers? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

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

2018-07-18 Thread Florin Iucha via Phabricator via cfe-commits
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] D69764: [clang-format] Add East/West Const fixer capability

2020-05-27 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. @MyDeveloperDay +1 from the trenches - I am in that same position and it took a lot of work to get the organization to _align_ on a consistent style, then enforce that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D6976

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-02-02 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. @aaron.ballman - can this land for Clang14, or does it have wait for 15? Are there any other reviewers that can approve this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 __

[PATCH] D67265: [clang-tidy] Magic number checker shouldn't warn on user defined string literals

2019-09-06 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- accepted this revision. 0x8000- added a comment. This revision is now accepted and ready to land. Looks good to me. Thank you for the fix. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67265/new/ https://reviews.llvm.org/D67265 __

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2019-12-31 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. F11163406: 0001-Add-extra-tests.patch shows a couple of false positives. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-03 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1803448 , @JonasToth wrote: > In D54943#1800182 , @0x8000- > wrote: > > > F11163406: 0001-Add-extra-tests.patch > > shows a couple of

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-03 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Indicated where the new test code should go. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template Please insert the this test code here: ``` struct IntWrapper {

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template JonasToth wrote: > JonasToth wrote: > > 0x8000- wrote: > > > Please insert the this test code here: >

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template 0x8000- wrote: > JonasToth wrote: > > JonasToth wrote: > > > 0x8000- wrote: > > > > Please insert

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. This patch does not build on top of current tree: /usr/bin/ld: lib/libclangTidyCppCoreGuidelinesModule.a(ConstCorrectnessCheck.cpp.o): in function `clang::tidy::cppcoreguidelines::ConstCorrectnessCheck::registerMatchers(clang::ast_matchers::MatchFinder*)': /ho

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1804291 , @JonasToth wrote: > In D54943#1804183 , @0x8000- > wrote: > > > This patch does not build on top of current tree: > > > > /usr/bin/ld: > > lib/libclangTidyCpp

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-04 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template JonasToth wrote: > 0x8000- wrote: > > 0x8000- wrote: > > > JonasToth wrote: > > > > JonasToth wro

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- marked an inline comment as done. 0x8000- added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608 +} + +template JonasToth wrote: > JonasToth wrote: > > 0x8000- wrote:

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. I can't build this on top of current llvm-project tree (6a6e6f04ec2cd2f4f07ec4943036c5c2d47ce0c7 ): /usr/bin/ld: lib/libclangTidyCppCoreGuidelinesModule.a(ConstCorrectnessCheck.cpp.o): in funct

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1804904 , @JonasToth wrote: > In D54943#1804890 , @0x8000- > wrote: > > > I can't build this on top of current llvm-project tree > > (6a6e6f04ec2cd2f4f07ec4943036c5c2d47c

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-05 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1804943 , @JonasToth wrote: > - Merge branch 'master' into feature_transform_const.patch > - link clangAnalysis to the cppcoreguidelines-module > - fix InitListExpr as well This last version too builds with RelWithD

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-06 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1805439 , @JonasToth wrote: > > This last version too builds with RelWithDebInfo and fails with Debug > > configuration (both clean builds). > > So it only fails in debug-build? Correct; I can build 'Release' or 'R

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Applied the fix-it on one of our smaller (but C++ code bases) and it builds and passes the tests. Comments: 1. I'm still seeing some false positives, where some locals that are const are flagged as could be made const - I'm working to create a minimal reproduction

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1815121 , @JonasToth wrote: > In D54943#1815073 , @0x8000- > wrote: > > > Applied the fix-it on one of our smaller (but C++ code bases) and it builds > > and passes the t

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-10 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Here is a minimal example that shows the problem: #include template struct DoGooder { DoGooder(void* accessor, SomeValue value) { } }; struct Bingus { static constexpr auto someRandomConstant = 99; }; template s

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1815473 , @JonasToth wrote: > In D54943#1815358 , @0x8000- > wrote: > > > Here is a minimal example that shows the problem: > > > I can not reproduce that case :( > It gi

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. In D54943#1815568 , @JonasToth wrote: > @0x8000- i did remove `dependentTypes` from matching. Locally that works > with your reported test-case (i was running clang-tidy wrong :(). > > I do understand the `auto &` issues n

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. I have built diff14 and tried to apply it on an internal code base with ~7500 translation units. $ python3 /opt/clang10/share/clang/run-clang-tidy.py -clang-tidy-binary /opt/clang10/bin/clang-tidy -clang-apply-replacements /opt/clang10/bin/clang-apply-replacemen

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. Summary of "misses" One: uint32_t counter = 0; BOOST_FOREACH(const Foo* foo, *theFoos) { if (foo->getType() == someType) { ++counter; } } The -fix makes the counter const :), so obviously it breaks compilation. Two: It marks a l

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-11 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. One more mis-constification that I can't explain, nor reduce to a small test case (when I extract the code, the check behaves as expected / desired) namespace util { struct StringIgnoreInitialHash : public std::unary_function { size_t operator()(cons

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2020-01-12 Thread Florin Iucha via Phabricator via cfe-commits
0x8000- added a comment. As an aside, once this is merged in, I dream of a "fix-it" for old style C code: int foo; ... /* page of code which does not use either foo or bar */ ... foo = 5; Moves the foo declaration to the line where it is actually first initialized. Then run the

  1   2   >