[PATCH] D65912: Add new check for math constants

Eugene Zelenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 7 17:46:00 PDT 2019


Eugene.Zelenko added a comment.

May be this check belongs to readability module?



================
Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:13
+#include "../../../clang/unittests/AST/Language.h"
+
+#include <cmath>
----------------
Unnecessary empty line.


================
Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:27
+
+constexpr char StdMathHeader[] = "math";
+constexpr char StdCmathHeader[] = "cmath";
----------------
Functions and constants should be static. See LLVM Coding Guidelines.


================
Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:70
+void MathConstantsCheck::registerMatchers(MatchFinder *Finder) {
+  // FIXME: Add matchers.
+  Finder->addMatcher(floatLiteral().bind(FloatType), this);
----------------
Is it needed?


================
Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:84
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<FloatingLiteral>(FloatType);
+  const auto MatchedFloatConstant = MatchedDecl->getValue().convertToDouble();
+
----------------
Please don't use auto when type is not spelled in same statement or in case of iterators. Same in other places.


================
Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:85
+  const auto MatchedFloatConstant = MatchedDecl->getValue().convertToDouble();
+
+  const auto Language = getLangOpts();
----------------
Unnecessary empty line.


================
Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h:13
+#include "../ClangTidyCheck.h"
+
+#include "../utils/IncludeInserter.h"
----------------
Please remove empty lines between headers.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:91
+
+  FIXME: add release notes.
+
----------------
Please add one sentence description. Should be same as first sentence of documentation.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst:6
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
----------------
Please add documentation.


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

https://reviews.llvm.org/D65912





More information about the cfe-commits mailing list