[PATCH] D49114: Add a clang-tidy check for "magic numbers"
Florin Iucha via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 9 18:51:38 PDT 2018
0x8000-0000 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 initialized using expressions such as 1 << 5? Some tests might be useful in this regard.
Even better point!
I will implement those suggestions and update the review packet.
Thank you,
florin
In https://reviews.llvm.org/D49114#1156835, @hfinkel wrote:
> > 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 to 100% calculations. 2 and 10 due to logarithms, etc. Not sure where to draw the line there.
>
> 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.
>
> Also, what happens for enums? Especially when initialized using expressions such as 1 << 5? Some tests might be useful in this regard.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49114
More information about the cfe-commits
mailing list