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

Jonas Toth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 26 06:23:47 PDT 2018


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) {
----------------
You move the `using namespace clang::ast_matchers;` up to shorten your signature.
Adding a using for `ast_type_traits` is possible, too .


================
Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:79
+bool MagicNumbersCheck::isConstant(
+    const clang::ast_matchers::MatchFinder::MatchResult &Result,
+    const clang::Expr &ExprResult) const {
----------------
is the `clang::` necessary? The code should be in that namespace already. I think shortening the really long type qualifier helps with readability. Similar on other places.


================
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:`IgnoredIntegerValues` option.  In addition,
----------------
-1 is not in the default list anymore.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114





More information about the cfe-commits mailing list