[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