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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits