Index: clang-tidy/abseil/AbseilTidyModule.cpp =================================================================== --- clang-tidy/abseil/AbseilTidyModule.cpp +++ clang-tidy/abseil/AbseilTidyModule.cpp @@ -13,6 +13,7 @@ #include "DurationComparisonCheck.h" #include "DurationConversionCastCheck.h" #include "DurationDivisionCheck.h" +#include "DurationDoubleConversionCheck.h" #include "DurationFactoryFloatCheck.h" #include "DurationFactoryScaleCheck.h" #include "DurationSubtractionCheck.h" @@ -39,6 +40,8 @@ "abseil-duration-conversion-cast"); CheckFactories.registerCheck( "abseil-duration-division"); + CheckFactories.registerCheck( + "abseil-duration-double-conversion"); CheckFactories.registerCheck( "abseil-duration-factory-float"); CheckFactories.registerCheck( Index: clang-tidy/abseil/CMakeLists.txt =================================================================== --- clang-tidy/abseil/CMakeLists.txt +++ clang-tidy/abseil/CMakeLists.txt @@ -6,6 +6,7 @@ DurationComparisonCheck.cpp DurationConversionCastCheck.cpp DurationDivisionCheck.cpp + DurationDoubleConversionCheck.cpp DurationFactoryFloatCheck.cpp DurationFactoryScaleCheck.cpp DurationRewriter.cpp Index: clang-tidy/abseil/DurationDoubleConversionCheck.h =================================================================== --- /dev/null +++ clang-tidy/abseil/DurationDoubleConversionCheck.h @@ -0,0 +1,35 @@ +//===--- DurationDoubleConversionCheck.h - clang-tidy -----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace abseil { + +/// Find and fix cases where `absl::Duration` values are being converted to +/// numeric types and back again. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-double-conversion.html +class DurationDoubleConversionCheck : public ClangTidyCheck { +public: + DurationDoubleConversionCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + 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_TIMEDOUBLECONVERSIONCHECK_H Index: clang-tidy/abseil/DurationDoubleConversionCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/abseil/DurationDoubleConversionCheck.cpp @@ -0,0 +1,58 @@ +//===--- DurationDoubleConversionCheck.cpp - clang-tidy +//-----------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DurationDoubleConversionCheck.h" +#include "DurationRewriter.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/FixIt.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace abseil { + +void DurationDoubleConversionCheck::registerMatchers(MatchFinder *Finder) { + constexpr std::array Scales = { + "Hours", "Minutes", "Seconds", + "Milliseconds", "Microseconds", "Nanoseconds"}; + + for (const auto &s : Scales) { + std::string DurationFactory = (llvm::Twine("::absl::") + s).str(); + std::string FloatConversion = (llvm::Twine("::absl::ToDouble") + s).str(); + std::string IntegerConversion = (llvm::Twine("::absl::ToInt64") + s).str(); + + Finder->addMatcher( + callExpr( + callee(functionDecl(hasName(DurationFactory))), + hasArgument(0, callExpr(callee(functionDecl(hasAnyName( + FloatConversion, IntegerConversion))), + hasArgument(0, expr().bind("arg"))))) + .bind("call"), + this); + } +} + +void DurationDoubleConversionCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *OuterCall = Result.Nodes.getNodeAs("call"); + const auto *Arg = Result.Nodes.getNodeAs("arg"); + + if (!isNotInMacro(Result, OuterCall)) return; + + diag(OuterCall->getBeginLoc(), "remove double conversion of absl::Duration") + << FixItHint::CreateReplacement( + OuterCall->getSourceRange(), + tooling::fixit::getText(*Arg, *Result.Context)); +} + +} // namespace abseil +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -79,6 +79,12 @@ Checks for casts of ``absl::Duration`` conversion functions, and recommends the right conversion function instead. +- New :doc:`abseil-duration-double-conversion + ` check. + + Find and fix cases where `absl::Duration` values are being converted to + numeric types and back again. + - New :doc:`google-readability-avoid-underscore-in-googletest-name ` check. Index: docs/clang-tidy/checks/abseil-duration-double-conversion.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/abseil-duration-double-conversion.rst @@ -0,0 +1,32 @@ +.. title:: clang-tidy - abseil-duration-double-conversion + +abseil-duration-double-conversion +================================= + +Find and fix cases where `absl::Duration` values are being converted to +numeric types and back again. + +Examples: + +.. code-block:: c++ + + // Original - Conversion to double and back again + absl::Duration d1; + absl::Duration d2 = absl::Seconds(absl::ToDoubleSeconds(d1)); + + // Suggestion - Remove double conversion + absl::Duration d2 = d1; + + + // Original - Conversion to integer and back again + absl::Duration d1; + absl::Duration d2 = absl::Hours(absl::ToInt64Hours(d1)); + + // Suggestion - Remove double 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 +`absl::Trunc` to truncate explicitly. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -8,6 +8,7 @@ abseil-duration-comparison abseil-duration-conversion-cast abseil-duration-division + abseil-duration-double-conversion abseil-duration-factory-float abseil-duration-factory-scale abseil-duration-subtraction 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 @@ -14,6 +14,8 @@ Duration &operator/=(float r); Duration &operator/=(double r); template Duration &operator/=(T r); + + Duration &operator+(Duration d); }; template Duration operator*(Duration lhs, T rhs); Index: test/clang-tidy/abseil-duration-double-conversion.cpp =================================================================== --- /dev/null +++ test/clang-tidy/abseil-duration-double-conversion.cpp @@ -0,0 +1,69 @@ +// RUN: %check_clang_tidy %s abseil-duration-double-conversion %t -- -- -I%S/Inputs + +#include "absl/time/time.h" + +void f() { + absl::Duration d1, d2; + + // Floating point + d2 = absl::Hours(absl::ToDoubleHours(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Minutes(absl::ToDoubleMinutes(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Seconds(absl::ToDoubleSeconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Milliseconds(absl::ToDoubleMilliseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Microseconds(absl::ToDoubleMicroseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Nanoseconds(absl::ToDoubleNanoseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + + // Integer point + d2 = absl::Hours(absl::ToInt64Hours(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Minutes(absl::ToInt64Minutes(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Seconds(absl::ToInt64Seconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Milliseconds(absl::ToInt64Milliseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Microseconds(absl::ToInt64Microseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: d1 + d2 = absl::Nanoseconds(absl::ToInt64Nanoseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-double-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))); + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: remove double conversion of absl::Duration [abseil-duration-double-conversion] + // CHECK-FIXES: PLUS_FIVE_S(d1) +#undef PLUS_FIVE_S + + // Split by macro: should not change +#define TOSECONDS(x) absl::Seconds(x) + d2 = TOSECONDS(absl::ToInt64Seconds(d1)); +#undef TOSECONDS + + // Don't change something inside a macro definition +#define VALUE(x) absl::Hours(absl::ToInt64Hours(x)); + d2 = VALUE(d1); +#undef VALUE + + // These should not match + d2 = absl::Seconds(absl::ToDoubleMilliseconds(d1)); + d2 = absl::Seconds(4); + int i = absl::ToInt64Milliseconds(d1); +}