diff options
7 files changed, 181 insertions, 0 deletions
diff --git a/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp b/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp index 8e427f05873..061fc7c4d13 100644 --- a/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/abseil/AbseilTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "DurationDivisionCheck.h" #include "StringFindStartswithCheck.h" namespace clang { @@ -19,6 +20,8 @@ namespace abseil { class AbseilModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<DurationDivisionCheck>( + "abseil-duration-division"); CheckFactories.registerCheck<StringFindStartswithCheck>( "abseil-string-find-startswith"); } diff --git a/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt b/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt index dd59dcf0c20..a2478fd428e 100644 --- a/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/abseil/CMakeLists.txt @@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyAbseilModule AbseilTidyModule.cpp + DurationDivisionCheck.cpp StringFindStartswithCheck.cpp LINK_LIBS diff --git a/clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.cpp b/clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.cpp new file mode 100644 index 00000000000..5edc15875eb --- /dev/null +++ b/clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.cpp @@ -0,0 +1,59 @@ +//===--- DurationDivisionCheck.cpp - clang-tidy----------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "DurationDivisionCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +namespace clang { +namespace tidy { +namespace abseil { + +using namespace clang::ast_matchers; + +void DurationDivisionCheck::registerMatchers(MatchFinder *finder) { + if (!getLangOpts().CPlusPlus) + return; + + const auto DurationExpr = + expr(hasType(cxxRecordDecl(hasName("::absl::Duration")))); + finder->addMatcher( + implicitCastExpr( + hasSourceExpression(ignoringParenCasts( + cxxOperatorCallExpr(hasOverloadedOperatorName("/"), + hasArgument(0, DurationExpr), + hasArgument(1, DurationExpr)) + .bind("OpCall"))), + hasImplicitDestinationType(qualType(unless(isInteger()))), + unless(hasParent(cxxStaticCastExpr())), + unless(hasParent(cStyleCastExpr())), + unless(isInTemplateInstantiation())), + this); +} + +void DurationDivisionCheck::check(const MatchFinder::MatchResult &result) { + const auto *OpCall = result.Nodes.getNodeAs<CXXOperatorCallExpr>("OpCall"); + diag(OpCall->getOperatorLoc(), + "operator/ on absl::Duration objects performs integer division; " + "did you mean to use FDivDuration()?") + << FixItHint::CreateInsertion(OpCall->getBeginLoc(), + "absl::FDivDuration(") + << FixItHint::CreateReplacement( + SourceRange(OpCall->getOperatorLoc(), OpCall->getOperatorLoc()), + ", ") + << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken( + result.SourceManager->getSpellingLoc(OpCall->getEndLoc()), 0, + *result.SourceManager, result.Context->getLangOpts()), + ")"); +} + +} // namespace abseil +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.h b/clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.h new file mode 100644 index 00000000000..932d0296636 --- /dev/null +++ b/clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.h @@ -0,0 +1,35 @@ +//===--- DurationDivisionCheck.h - clang-tidy--------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONDIVISIONCHECK_H_ +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONDIVISIONCHECK_H_ + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace abseil { + +// Find potential incorrect uses of integer division of absl::Duration objects. +// +// For the user-facing documentation see: +// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-division.html + +class DurationDivisionCheck : public ClangTidyCheck { +public: + using ClangTidyCheck::ClangTidyCheck; + void registerMatchers(ast_matchers::MatchFinder *finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &result) override; +}; + +} // namespace abseil +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONDIVISIONCHECK_H_ diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 83d5e017cae..cea98b334e3 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -57,6 +57,13 @@ The improvements are... Improvements to clang-tidy -------------------------- +- New :doc:`abseil-duration-division + <clang-tidy/checks/abseil-duration-division>` check. + + Checks for uses of ``absl::Duration`` division that is done in a + floating-point context, and recommends the use of a function that + returns a floating-point value. + - New :doc:`readability-magic-numbers <clang-tidy/checks/readability-magic-numbers>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 25515670c42..696905b50d2 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -4,6 +4,7 @@ Clang-Tidy Checks ================= .. toctree:: + abseil-duration-division abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 diff --git a/clang-tools-extra/test/clang-tidy/abseil-duration-division.cpp b/clang-tools-extra/test/clang-tidy/abseil-duration-division.cpp new file mode 100644 index 00000000000..51d012f1538 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/abseil-duration-division.cpp @@ -0,0 +1,75 @@ +// RUN: %check_clang_tidy %s abseil-duration-division %t + +namespace absl { + +class Duration {}; + +int operator/(Duration lhs, Duration rhs); + +double FDivDuration(Duration num, Duration den); + +} // namespace absl + +void TakesDouble(double); + +#define MACRO_EQ(x, y) (x == y) +#define MACRO_DIVEQ(x,y,z) (x/y == z) +#define CHECK(x) (x) + +void Positives() { + absl::Duration d; + + const double num_double = d/d; + // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division] + // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d); + const float num_float = d/d; + // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects + // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d); + const auto SomeVal = 1.0 + d/d; + // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects + // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d); + if (MACRO_EQ(d/d, 0.0)) {} + // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects + // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {} + if (CHECK(MACRO_EQ(d/d, 0.0))) {} + // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects + // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {} + + // This one generates a message, but no fix. + if (MACRO_DIVEQ(d, d, 0.0)) {} + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects + // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {} + + TakesDouble(d/d); + // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects + // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d)); +} + +void TakesInt(int); +template <class T> +void TakesGeneric(T); + +void Negatives() { + absl::Duration d; + const int num_int = d/d; + const long num_long = d/d; + const short num_short = d/d; + const char num_char = d/d; + const auto num_auto = d/d; + const auto SomeVal = 1 + d/d; + + TakesInt(d/d); + TakesGeneric(d/d); + // Explicit cast should disable the warning. + const double num_cast1 = static_cast<double>(d/d); + const double num_cast2 = (double)(d/d); +} + +template <class T> +double DoubleDivision(T t1, T t2) {return t1/t2;} + +//This also won't trigger a warning +void TemplateDivision() { + absl::Duration d; + DoubleDivision(d, d); +} |
