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 : Parents) {
+
+      // the definition of const values themselves, as global or local 
variables
----------------
please remove this line.


================
Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:45
+
+      // the definition of const values themselves, as global or local 
variables
+      {
----------------
Please make the comments full sentences.


================
Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:46
+      // the definition of const values themselves, as global or local 
variables
+      {
+        const auto *AsVarDecl = Parent.get<clang::VarDecl>();
----------------
Every scope you introduce here should be a function, i think.


================
Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:11
+
+   double circleArea = 3.1415926535 * radius * radius;
+
----------------
This example is good, but right now the code only checks for integer literals. 
Maybe an integer example would be better?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to