JonasToth added a comment.
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.
Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:44
+for (const auto &Parent : Parent
JonasToth added a comment.
Related to this check:
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es45-avoid-magic-constants-use-symbolic-constants
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49114
_
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
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
Quuxplusone added a comment.
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 abandoned. //But//, it's just a
clang-tidy check — it's easy for peop
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
hfinkel added a comment.
> This version detects and report integers only. If there is interest of
> merging the tool I can add the functionality for floats as well.
FWIW: I think that the FP check would be interesting.
> Also I have seen coding guidelines suggesting "100" is grandfathered due t
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
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