Index: clang-tidy/abseil/AbseilTidyModule.cpp =================================================================== --- clang-tidy/abseil/AbseilTidyModule.cpp +++ 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 @@ class AbseilModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "abseil-duration-division"); CheckFactories.registerCheck( "abseil-string-find-startswith"); } Index: clang-tidy/abseil/DurationDivisionCheck.h =================================================================== --- clang-tidy/abseil/DurationDivisionCheck.h +++ clang-tidy/abseil/DurationDivisionCheck.h @@ -0,0 +1,31 @@ +//===--- 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. +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_ Index: clang-tidy/abseil/DurationDivisionCheck.cpp =================================================================== --- clang-tidy/abseil/DurationDivisionCheck.cpp +++ clang-tidy/abseil/DurationDivisionCheck.cpp @@ -0,0 +1,54 @@ +//===--- 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 IsDuration = + expr(hasType(cxxRecordDecl(hasName("::absl::Duration")))); + finder->addMatcher( + implicitCastExpr( + hasSourceExpression(ignoringParenCasts(cxxOperatorCallExpr( + hasOverloadedOperatorName("/"), argumentCountIs(2), + hasArgument(0, expr(is_duration)), + hasArgument(1, expr(is_duration)), expr().bind("OpCall")))), + hasImplicitDestinationType(qualType(unless(isInteger()))), + unless(hasParent(cxxStaticCastExpr()))), + this); +} + +void DurationDivisionCheck::check(const MatchFinder::MatchResult &result) { + const auto *op = result.Nodes.getNodeAs("OpCall"); + diag(op->getOperatorLoc(), + "operator/ on absl::Duration objects performs integer division; " + "did you mean to use FDivDuration()?") + << FixItHint::CreateInsertion(op->getLocStart(), "absl::FDivDuration(") + << FixItHint::CreateReplacement( + SourceRange(op->getOperatorLoc(), op->getOperatorLoc()), ", ") + << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken( + result.SourceManager->getSpellingLoc(op->getLocEnd()), 0, + *result.SourceManager, result.Context->getLangOpts()), + ")"); +} + +} // namespace abseil +} // namespace tidy +} // namespace clang Index: docs/clang-tidy/checks/abseil-duration-division.rst =================================================================== --- docs/clang-tidy/checks/abseil-duration-division.rst +++ docs/clang-tidy/checks/abseil-duration-division.rst @@ -0,0 +1,36 @@ +.. title:: clang-tidy - abseil-duration-division + +abseil-duration-division +======================== + +``absl::Duration`` arithmetic works like it does with integers. That means that +division of two ``absl::Duration`` objects returns an ``int64`` with any fractional +component truncated toward 0. + +For example: + +.. code-block:: c++ + + absl::Duration d = absl::Seconds(3.5); + int64 sec1 = d / absl::Seconds(1); // Truncates toward 0. + int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division. + assert(sec1 == 3 && sec2 == 3); + + double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0. + assert(dsec == 3.0); + +If you want floating-point division, you should use either the +``absl::FDivDuration()`` function, or one of the unit conversion functions such +as ``absl::ToDoubleSeconds()``. For example: + +.. code-block:: c++ + + absl::Duration d = absl::Seconds(3.5); + double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation. + double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation. + assert(dsec1 == 3.5 && dsec2 == 3.5); + + +This check looks 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. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -4,6 +4,7 @@ ================= .. toctree:: + abseil-duration-division abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 Index: test/clang-tidy/abseil-duration-division.cpp =================================================================== --- test/clang-tidy/abseil-duration-division.cpp +++ test/clang-tidy/abseil-duration-division.cpp @@ -0,0 +1,44 @@ +// 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 TakesInt(int); +void TakesDouble(double); + +absl::Duration d; + +#define MACRO_EQ(x, y) (x == y) +#define MACRO_DIVEQ(x,y,z) (x/y == z) +#define CHECK(x) (x) + +void Positives() { + const double num = d/d; + // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division] + // CHECK-FIXES: const double num = absl::FDivDuration(d, d); + if (MACRO_EQ(d/d, 0.0)) {} + // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on 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 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 Duration objects + // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {} +} + +void Negatives() { + const int num = d/d; + + // Explicit cast should disable the warning. + const double num_d = static_cast(d/d); +}