[PATCH] D65912: [clang-tidy] Add new check for math constants

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:44-58
+            {{"M_E"}, {"std::math::e"}, 2.7182818284590452354},
+            {{"M_LOG2E"}, {"std::math::log2e"}, 1.4426950408889634074},
+            {{"M_LOG10E"}, {"std::math::log10e"}, 0.43429448190325182765},
+            {{"M_LN2"}, {"std::math::ln2"}, 0.69314718055994530942},
+            {{"M_LN10"}, {"std::math::ln10"}, 2.30258509299404568402},
+            {{"M_PI"}, {"std::math::pi"}, 3.14159265358979323846},
+            {{"M_PI_2"}, {}, 1.57079632679489661923},
----------------
I'm a bit uncomfortable putting the constant values in like this -- are you sure those values will be correct for all targets?


================
Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h:26
+
+/// FIXME: Write a short description.
+///
----------------
You should fix this FIXME. ;-)


================
Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h:42
+
+  struct MathConstantDescription
+  {
----------------
Formatting is off here, you should run the patch through clang-format, if you don't already.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65912/new/

https://reviews.llvm.org/D65912