Index: clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp =================================================================== --- clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp +++ clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp @@ -28,12 +28,27 @@ std::string IntegerConversion = (llvm::Twine("::absl::ToInt64") + Scale).str(); + auto factory_matcher = cxxConstructExpr(hasArgument( + 0, + callExpr(callee(functionDecl(hasName(DurationFactory))), + hasArgument(0, ignoringImpCasts(integerLiteral(equals(1))))))); + + auto inverse_function_matcher = callExpr( + callee(functionDecl(hasAnyName(FloatConversion, IntegerConversion))), + hasArgument(0, expr().bind("arg"))); + + auto division_operator_matcher = cxxOperatorCallExpr( + hasOverloadedOperatorName("/"), hasArgument(0, expr().bind("arg")), + hasArgument(1, factory_matcher)); + + auto fdiv_matcher = callExpr( + callee(functionDecl(hasName("::absl::FDivDuration"))), + hasArgument(0, expr().bind("arg")), hasArgument(1, factory_matcher)); + Finder->addMatcher( - callExpr( - callee(functionDecl(hasName(DurationFactory))), - hasArgument(0, callExpr(callee(functionDecl(hasAnyName( - FloatConversion, IntegerConversion))), - hasArgument(0, expr().bind("arg"))))) + callExpr(callee(functionDecl(hasName(DurationFactory))), + hasArgument(0, anyOf(inverse_function_matcher, + division_operator_matcher, fdiv_matcher))) .bind("call"), this); } @@ -47,7 +62,8 @@ if (isInMacro(Result, OuterCall)) return; - diag(OuterCall->getBeginLoc(), "remove unnecessary absl::Duration conversions") + diag(OuterCall->getBeginLoc(), + "remove unnecessary absl::Duration conversions") << FixItHint::CreateReplacement( OuterCall->getSourceRange(), tooling::fixit::getText(*Arg, *Result.Context)); Index: docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst =================================================================== --- docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst +++ docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst @@ -6,7 +6,7 @@ Finds and fixes cases where ``absl::Duration`` values are being converted to numeric types and back again. -Examples: +Floating-point examples: .. code-block:: c++ @@ -17,6 +17,15 @@ // Suggestion - Remove unnecessary conversions absl::Duration d2 = d1; + // Original - Division to convert to double and back again + absl::Duration d2 = absl::Seconds(absl::FDivDuration(d1, absl::Seconds(1))); + + // Suggestion - Remove division and conversion + absl::Duration d2 = d1; + +Integer examples: + +.. code-block:: c++ // Original - Conversion to integer and back again absl::Duration d1; @@ -25,6 +34,12 @@ // Suggestion - Remove unnecessary conversions absl::Duration d2 = d1; + // Original - Integer division followed by conversion + absl::Duration d2 = absl::Seconds(d1 / absl::Seconds(1)); + + // Suggestion - Remove division and conversion + absl::Duration d2 = d1; + Note: Converting to an integer and back to an ``absl::Duration`` might be a truncating operation if the value is not aligned to the scale of conversion. In the rare case where this is the intended result, callers should use Index: test/clang-tidy/Inputs/absl/time/time.h =================================================================== --- test/clang-tidy/Inputs/absl/time/time.h +++ test/clang-tidy/Inputs/absl/time/time.h @@ -21,6 +21,7 @@ template Duration operator*(Duration lhs, T rhs); template Duration operator*(T lhs, Duration rhs); template Duration operator/(Duration lhs, T rhs); +int64_t operator/(Duration lhs, Duration rhs); class Time{}; @@ -86,4 +87,6 @@ inline Time operator-(Time lhs, Duration rhs); inline Duration operator-(Time lhs, Time rhs); +double FDivDuration(Duration num, Duration den); + } // namespace absl Index: test/clang-tidy/abseil-duration-unnecessary-conversion.cpp =================================================================== --- test/clang-tidy/abseil-duration-unnecessary-conversion.cpp +++ test/clang-tidy/abseil-duration-unnecessary-conversion.cpp @@ -45,6 +45,44 @@ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] // CHECK-FIXES: d1 + d2 = absl::Hours(d1 / absl::Hours(1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Minutes(d1 / absl::Minutes(1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Seconds(d1 / absl::Seconds(1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Milliseconds(d1 / absl::Milliseconds(1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Microseconds(d1 / absl::Microseconds(1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Nanoseconds(d1 / absl::Nanoseconds(1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + + d2 = absl::Hours(absl::FDivDuration(d1, absl::Hours(1))); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Minutes(absl::FDivDuration(d1, absl::Minutes(1))); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Seconds(absl::FDivDuration(d1, absl::Seconds(1))); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Milliseconds(absl::FDivDuration(d1, absl::Milliseconds(1))); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Microseconds(absl::FDivDuration(d1, absl::Microseconds(1))); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + d2 = absl::Nanoseconds(absl::FDivDuration(d1, absl::Nanoseconds(1))); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d1 + // As macro argument #define PLUS_FIVE_S(x) x + absl::Seconds(5) d2 = PLUS_FIVE_S(absl::Seconds(absl::ToInt64Seconds(d1))); @@ -66,4 +104,8 @@ d2 = absl::Seconds(absl::ToDoubleMilliseconds(d1)); d2 = absl::Seconds(4); int i = absl::ToInt64Milliseconds(d1); + d2 = absl::Hours(d1 / absl::Minutes(1)); + d2 = absl::Seconds(d1 / absl::Seconds(30)); + d2 = absl::Hours(absl::FDivDuration(d1, absl::Minutes(1))); + d2 = absl::Milliseconds(absl::FDivDuration(d1, absl::Milliseconds(20))); }