Index: clang-tidy/abseil/AbseilTidyModule.cpp =================================================================== --- clang-tidy/abseil/AbseilTidyModule.cpp +++ clang-tidy/abseil/AbseilTidyModule.cpp @@ -14,6 +14,7 @@ #include "DurationDivisionCheck.h" #include "DurationFactoryFloatCheck.h" #include "DurationFactoryScaleCheck.h" +#include "DurationSubtractionCheck.h" #include "FasterStrsplitDelimiterCheck.h" #include "NoInternalDependenciesCheck.h" #include "NoNamespaceCheck.h" @@ -37,6 +38,8 @@ "abseil-duration-factory-float"); CheckFactories.registerCheck( "abseil-duration-factory-scale"); + CheckFactories.registerCheck( + "abseil-duration-subtraction"); CheckFactories.registerCheck( "abseil-faster-strsplit-delimiter"); CheckFactories.registerCheck( Index: clang-tidy/abseil/CMakeLists.txt =================================================================== --- clang-tidy/abseil/CMakeLists.txt +++ clang-tidy/abseil/CMakeLists.txt @@ -7,6 +7,7 @@ DurationFactoryFloatCheck.cpp DurationFactoryScaleCheck.cpp DurationRewriter.cpp + DurationSubtractionCheck.cpp FasterStrsplitDelimiterCheck.cpp NoInternalDependenciesCheck.cpp NoNamespaceCheck.cpp Index: clang-tidy/abseil/DurationComparisonCheck.cpp =================================================================== --- clang-tidy/abseil/DurationComparisonCheck.cpp +++ clang-tidy/abseil/DurationComparisonCheck.cpp @@ -19,101 +19,6 @@ namespace tidy { namespace abseil { -/// Given the name of an inverse Duration function (e.g., `ToDoubleSeconds`), -/// return its `DurationScale`, or `None` if a match is not found. -static llvm::Optional getScaleForInverse(llvm::StringRef Name) { - static const llvm::DenseMap ScaleMap( - {{"ToDoubleHours", DurationScale::Hours}, - {"ToInt64Hours", DurationScale::Hours}, - {"ToDoubleMinutes", DurationScale::Minutes}, - {"ToInt64Minutes", DurationScale::Minutes}, - {"ToDoubleSeconds", DurationScale::Seconds}, - {"ToInt64Seconds", DurationScale::Seconds}, - {"ToDoubleMilliseconds", DurationScale::Milliseconds}, - {"ToInt64Milliseconds", DurationScale::Milliseconds}, - {"ToDoubleMicroseconds", DurationScale::Microseconds}, - {"ToInt64Microseconds", DurationScale::Microseconds}, - {"ToDoubleNanoseconds", DurationScale::Nanoseconds}, - {"ToInt64Nanoseconds", DurationScale::Nanoseconds}}); - - auto ScaleIter = ScaleMap.find(std::string(Name)); - if (ScaleIter == ScaleMap.end()) - return llvm::None; - - return ScaleIter->second; -} - -/// Given a `Scale` return the inverse functions for it. -static const std::pair & -getInverseForScale(DurationScale Scale) { - static const std::unordered_map> - InverseMap( - {{DurationScale::Hours, - std::make_pair("::absl::ToDoubleHours", "::absl::ToInt64Hours")}, - {DurationScale::Minutes, std::make_pair("::absl::ToDoubleMinutes", - "::absl::ToInt64Minutes")}, - {DurationScale::Seconds, std::make_pair("::absl::ToDoubleSeconds", - "::absl::ToInt64Seconds")}, - {DurationScale::Milliseconds, - std::make_pair("::absl::ToDoubleMilliseconds", - "::absl::ToInt64Milliseconds")}, - {DurationScale::Microseconds, - std::make_pair("::absl::ToDoubleMicroseconds", - "::absl::ToInt64Microseconds")}, - {DurationScale::Nanoseconds, - std::make_pair("::absl::ToDoubleNanoseconds", - "::absl::ToInt64Nanoseconds")}}); - - // We know our map contains all the Scale values, so we can skip the - // nonexistence check. - auto InverseIter = InverseMap.find(Scale); - assert(InverseIter != InverseMap.end() && "Unexpected scale found"); - return InverseIter->second; -} - -/// If `Node` is a call to the inverse of `Scale`, return that inverse's -/// argument, otherwise None. -static llvm::Optional -maybeRewriteInverseDurationCall(const MatchFinder::MatchResult &Result, - DurationScale Scale, const Expr &Node) { - const std::pair &InverseFunctions = - getInverseForScale(Scale); - if (const Expr *MaybeCallArg = selectFirst( - "e", match(callExpr(callee(functionDecl( - hasAnyName(InverseFunctions.first.c_str(), - InverseFunctions.second.c_str()))), - hasArgument(0, expr().bind("e"))), - Node, *Result.Context))) { - return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str(); - } - - return llvm::None; -} - -/// Assuming `Node` has type `double` or `int` representing a time interval of -/// `Scale`, return the expression to make it a suitable `Duration`. -static llvm::Optional rewriteExprFromNumberToDuration( - const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale, - const Expr *Node) { - const Expr &RootNode = *Node->IgnoreParenImpCasts(); - - if (RootNode.getBeginLoc().isMacroID()) - return llvm::None; - - // First check to see if we can undo a complimentary function call. - if (llvm::Optional MaybeRewrite = - maybeRewriteInverseDurationCall(Result, Scale, RootNode)) - return *MaybeRewrite; - - if (IsLiteralZero(Result, RootNode)) - return std::string("absl::ZeroDuration()"); - - return (llvm::Twine(getFactoryForScale(Scale)) + "(" + - simplifyDurationFactoryArg(Result, RootNode) + ")") - .str(); -} - void DurationComparisonCheck::registerMatchers(MatchFinder *Finder) { auto Matcher = binaryOperator(anyOf(hasOperatorName(">"), hasOperatorName(">="), Index: clang-tidy/abseil/DurationRewriter.h =================================================================== --- clang-tidy/abseil/DurationRewriter.h +++ clang-tidy/abseil/DurationRewriter.h @@ -19,8 +19,8 @@ namespace abseil { /// Duration factory and conversion scales -enum class DurationScale : std::int8_t { - Hours, +enum class DurationScale : std::uint8_t { + Hours = 0, Minutes, Seconds, Milliseconds, @@ -78,6 +78,16 @@ simplifyDurationFactoryArg(const ast_matchers::MatchFinder::MatchResult &Result, const Expr &Node); +/// Given the name of an inverse Duration function (e.g., `ToDoubleSeconds`), +/// return its `DurationScale`, or `None` if a match is not found. +llvm::Optional getScaleForInverse(llvm::StringRef Name); + +/// Assuming `Node` has type `double` or `int` representing a time interval of +/// `Scale`, return the expression to make it a suitable `Duration`. +llvm::Optional rewriteExprFromNumberToDuration( + const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale, + const Expr *Node); + AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher, DurationConversionFunction) { using namespace clang::ast_matchers; Index: clang-tidy/abseil/DurationRewriter.cpp =================================================================== --- clang-tidy/abseil/DurationRewriter.cpp +++ clang-tidy/abseil/DurationRewriter.cpp @@ -9,6 +9,7 @@ #include "DurationRewriter.h" #include "clang/Tooling/FixIt.h" +#include "llvm/ADT/IndexedMap.h" using namespace clang::ast_matchers; @@ -16,6 +17,13 @@ namespace tidy { namespace abseil { +struct DurationScale2IndexFunctor { + using argument_type = DurationScale; + unsigned operator()(DurationScale Scale) const { + return static_cast(Scale); + } +}; + /// Returns an integer if the fractional part of a `FloatingLiteral` is `0`. static llvm::Optional truncateIfIntegral(const FloatingLiteral &FloatLiteral) { @@ -29,6 +37,55 @@ return llvm::None; } +/// Given a `Scale` return the inverse functions for it. +static const std::pair & +getInverseForScale(DurationScale Scale) { + static const llvm::IndexedMap, + DurationScale2IndexFunctor> + InverseMap = []() { + // TODO: Revisit the immediately invoked lamba technique when + // IndexedMap gets an initializer list constructor. + llvm::IndexedMap, + DurationScale2IndexFunctor> + InverseMap; + InverseMap.resize(6); + InverseMap[DurationScale::Hours] = + std::make_pair("::absl::ToDoubleHours", "::absl::ToInt64Hours"); + InverseMap[DurationScale::Minutes] = + std::make_pair("::absl::ToDoubleMinutes", "::absl::ToInt64Minutes"); + InverseMap[DurationScale::Seconds] = + std::make_pair("::absl::ToDoubleSeconds", "::absl::ToInt64Seconds"); + InverseMap[DurationScale::Milliseconds] = std::make_pair( + "::absl::ToDoubleMilliseconds", "::absl::ToInt64Milliseconds"); + InverseMap[DurationScale::Microseconds] = std::make_pair( + "::absl::ToDoubleMicroseconds", "::absl::ToInt64Microseconds"); + InverseMap[DurationScale::Nanoseconds] = std::make_pair( + "::absl::ToDoubleNanoseconds", "::absl::ToInt64Nanoseconds"); + return InverseMap; + }(); + + return InverseMap[Scale]; +} + +/// If `Node` is a call to the inverse of `Scale`, return that inverse's +/// argument, otherwise None. +static llvm::Optional +maybeRewriteInverseDurationCall(const MatchFinder::MatchResult &Result, + DurationScale Scale, const Expr &Node) { + const std::pair &InverseFunctions = + getInverseForScale(Scale); + if (const auto *MaybeCallArg = selectFirst( + "e", + match(callExpr(callee(functionDecl(hasAnyName( + InverseFunctions.first, InverseFunctions.second))), + hasArgument(0, expr().bind("e"))), + Node, *Result.Context))) { + return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str(); + } + + return llvm::None; +} + /// Returns the factory function name for a given `Scale`. llvm::StringRef getFactoryForScale(DurationScale Scale) { switch (Scale) { @@ -104,6 +161,49 @@ return tooling::fixit::getText(Node, *Result.Context).str(); } +llvm::Optional getScaleForInverse(llvm::StringRef Name) { + static const llvm::StringMap ScaleMap( + {{"ToDoubleHours", DurationScale::Hours}, + {"ToInt64Hours", DurationScale::Hours}, + {"ToDoubleMinutes", DurationScale::Minutes}, + {"ToInt64Minutes", DurationScale::Minutes}, + {"ToDoubleSeconds", DurationScale::Seconds}, + {"ToInt64Seconds", DurationScale::Seconds}, + {"ToDoubleMilliseconds", DurationScale::Milliseconds}, + {"ToInt64Milliseconds", DurationScale::Milliseconds}, + {"ToDoubleMicroseconds", DurationScale::Microseconds}, + {"ToInt64Microseconds", DurationScale::Microseconds}, + {"ToDoubleNanoseconds", DurationScale::Nanoseconds}, + {"ToInt64Nanoseconds", DurationScale::Nanoseconds}}); + + auto ScaleIter = ScaleMap.find(std::string(Name)); + if (ScaleIter == ScaleMap.end()) + return llvm::None; + + return ScaleIter->second; +} + +llvm::Optional rewriteExprFromNumberToDuration( + const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale, + const Expr *Node) { + const Expr &RootNode = *Node->IgnoreParenImpCasts(); + + if (RootNode.getBeginLoc().isMacroID()) + return llvm::None; + + // First check to see if we can undo a complimentary function call. + if (llvm::Optional MaybeRewrite = + maybeRewriteInverseDurationCall(Result, Scale, RootNode)) + return *MaybeRewrite; + + if (IsLiteralZero(Result, RootNode)) + return std::string("absl::ZeroDuration()"); + + return (llvm::Twine(getFactoryForScale(Scale)) + "(" + + simplifyDurationFactoryArg(Result, RootNode) + ")") + .str(); +} + } // namespace abseil } // namespace tidy } // namespace clang Index: clang-tidy/abseil/DurationSubtractionCheck.h =================================================================== --- /dev/null +++ clang-tidy/abseil/DurationSubtractionCheck.h @@ -0,0 +1,36 @@ +//===--- DurationSubtractionCheck.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_DURATIONSUBTRACTIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_DURATIONSUBTRACTIONCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace abseil { + +/// Checks for cases where subtraction should be performed in the +/// `absl::Duration` domain. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-subtraction.html +class DurationSubtractionCheck : public ClangTidyCheck { +public: + DurationSubtractionCheck(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_DURATIONSUBTRACTIONCHECK_H Index: clang-tidy/abseil/DurationSubtractionCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/abseil/DurationSubtractionCheck.cpp @@ -0,0 +1,63 @@ +//===--- DurationSubtractionCheck.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 "DurationSubtractionCheck.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 DurationSubtractionCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + binaryOperator( + hasOperatorName("-"), + hasLHS(callExpr(callee(functionDecl(DurationConversionFunction()) + .bind("function_decl")), + hasArgument(0, expr().bind("lhs_arg"))))) + .bind("binop"), + this); +} + +void DurationSubtractionCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Binop = Result.Nodes.getNodeAs("binop"); + const auto *FuncDecl = Result.Nodes.getNodeAs("function_decl"); + + // Don't try to replace things inside of macro definitions. + if (Binop->getExprLoc().isMacroID()) + return; + + llvm::Optional Scale = getScaleForInverse(FuncDecl->getName()); + if (!Scale) + return; + + llvm::Optional RhsReplacement = + rewriteExprFromNumberToDuration(Result, *Scale, Binop->getRHS()); + if (!RhsReplacement) + return; + + const Expr *LhsArg = Result.Nodes.getNodeAs("lhs_arg"); + + diag(Binop->getBeginLoc(), "perform subtraction in the duration domain") + << FixItHint::CreateReplacement( + Binop->getSourceRange(), + (llvm::Twine("absl::") + FuncDecl->getName() + "(" + + tooling::fixit::getText(*LhsArg, *Result.Context) + " - " + + *RhsReplacement + ")") + .str()); +} + +} // namespace abseil +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -93,6 +93,12 @@ Checks for cases where arguments to ``absl::Duration`` factory functions are scaled internally and could be changed to a different factory function. +- New :doc:`abseil-duration-subtraction + ` check. + + Checks for cases where subtraction should be performed in the + ``absl::Duration`` domain. + - New :doc:`abseil-faster-strsplit-delimiter ` check. Index: docs/clang-tidy/checks/abseil-duration-subtraction.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/abseil-duration-subtraction.rst @@ -0,0 +1,30 @@ +.. title:: clang-tidy - abseil-duration-subtraction + +abseil-duration-subtraction +=========================== + +Checks for cases where subtraction should be performed in the +``absl::Duration`` domain. When subtracting two values, and the first one is +known to be a conversion from ``absl::Duration``, we can infer that the second +should also be interpreted as an ``absl::Duration``, and make that inference +explicit. + +Examples: + +.. code-block:: c++ + + // Original - Subtraction in the double domain + double x; + absl::Duration d; + double result = absl::ToDoubleSeconds(d) - x; + + // Suggestion - Subtraction in the absl::Duration domain instead + double result = absl::ToDoubleSeconds(d - absl::Seconds(x)); + + + // Original - Subtraction of two Durations in the double domain + absl::Duration d1, d2; + double result = absl::ToDoubleSeconds(d1) - absl::ToDoubleSeconds(d2); + + // Suggestion - Subtraction in the absl::Duration domain instead + double result = absl::ToDoubleSeconds(d1 - d2); 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-division abseil-duration-factory-float abseil-duration-factory-scale + abseil-duration-subtraction abseil-faster-strsplit-delimiter abseil-no-internal-dependencies abseil-no-namespace Index: test/clang-tidy/Inputs/absl/time/time.h =================================================================== --- /dev/null +++ test/clang-tidy/Inputs/absl/time/time.h @@ -0,0 +1,57 @@ +// Mimic the implementation of absl::Duration +namespace absl { + +class Duration {}; +class Time{}; + +Duration Nanoseconds(long long); +Duration Microseconds(long long); +Duration Milliseconds(long long); +Duration Seconds(long long); +Duration Minutes(long long); +Duration Hours(long long); + +#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \ + Duration NAME(float n); \ + Duration NAME(double n); \ + template \ + Duration NAME(T n); + +GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds); +GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds); +GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds); +GENERATE_DURATION_FACTORY_OVERLOADS(Seconds); +GENERATE_DURATION_FACTORY_OVERLOADS(Minutes); +GENERATE_DURATION_FACTORY_OVERLOADS(Hours); +#undef GENERATE_DURATION_FACTORY_OVERLOADS + +using int64_t = long long int; + +double ToDoubleHours(Duration d); +double ToDoubleMinutes(Duration d); +double ToDoubleSeconds(Duration d); +double ToDoubleMilliseconds(Duration d); +double ToDoubleMicroseconds(Duration d); +double ToDoubleNanoseconds(Duration d); +int64_t ToInt64Hours(Duration d); +int64_t ToInt64Minutes(Duration d); +int64_t ToInt64Seconds(Duration d); +int64_t ToInt64Milliseconds(Duration d); +int64_t ToInt64Microseconds(Duration d); +int64_t ToInt64Nanoseconds(Duration d); + +// Relational Operators +constexpr bool operator<(Duration lhs, Duration rhs); +constexpr bool operator>(Duration lhs, Duration rhs); +constexpr bool operator>=(Duration lhs, Duration rhs); +constexpr bool operator<=(Duration lhs, Duration rhs); +constexpr bool operator==(Duration lhs, Duration rhs); +constexpr bool operator!=(Duration lhs, Duration rhs); + +// Additive Operators +inline Time operator+(Time lhs, Duration rhs); +inline Time operator+(Duration lhs, Time rhs); +inline Time operator-(Time lhs, Duration rhs); +inline Duration operator-(Time lhs, Time rhs); + +} // namespace absl Index: test/clang-tidy/abseil-duration-comparison.cpp =================================================================== --- test/clang-tidy/abseil-duration-comparison.cpp +++ test/clang-tidy/abseil-duration-comparison.cpp @@ -1,62 +1,6 @@ -// RUN: %check_clang_tidy %s abseil-duration-comparison %t +// RUN: %check_clang_tidy %s abseil-duration-comparison %t -- -- -I %S/Inputs -// Mimic the implementation of absl::Duration -namespace absl { - -class Duration {}; -class Time{}; - -Duration Nanoseconds(long long); -Duration Microseconds(long long); -Duration Milliseconds(long long); -Duration Seconds(long long); -Duration Minutes(long long); -Duration Hours(long long); - -#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \ - Duration NAME(float n); \ - Duration NAME(double n); \ - template \ - Duration NAME(T n); - -GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Seconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Minutes); -GENERATE_DURATION_FACTORY_OVERLOADS(Hours); -#undef GENERATE_DURATION_FACTORY_OVERLOADS - -using int64_t = long long int; - -double ToDoubleHours(Duration d); -double ToDoubleMinutes(Duration d); -double ToDoubleSeconds(Duration d); -double ToDoubleMilliseconds(Duration d); -double ToDoubleMicroseconds(Duration d); -double ToDoubleNanoseconds(Duration d); -int64_t ToInt64Hours(Duration d); -int64_t ToInt64Minutes(Duration d); -int64_t ToInt64Seconds(Duration d); -int64_t ToInt64Milliseconds(Duration d); -int64_t ToInt64Microseconds(Duration d); -int64_t ToInt64Nanoseconds(Duration d); - -// Relational Operators -constexpr bool operator<(Duration lhs, Duration rhs); -constexpr bool operator>(Duration lhs, Duration rhs); -constexpr bool operator>=(Duration lhs, Duration rhs); -constexpr bool operator<=(Duration lhs, Duration rhs); -constexpr bool operator==(Duration lhs, Duration rhs); -constexpr bool operator!=(Duration lhs, Duration rhs); - -// Additive Operators -inline Time operator+(Time lhs, Duration rhs); -inline Time operator+(Duration lhs, Time rhs); -inline Time operator-(Time lhs, Duration rhs); -inline Duration operator-(Time lhs, Time rhs); - -} // namespace absl +#include "absl/time/time.h" void f() { double x; Index: test/clang-tidy/abseil-duration-factory-float.cpp =================================================================== --- test/clang-tidy/abseil-duration-factory-float.cpp +++ test/clang-tidy/abseil-duration-factory-float.cpp @@ -1,32 +1,6 @@ -// RUN: %check_clang_tidy %s abseil-duration-factory-float %t - -// Mimic the implementation of absl::Duration -namespace absl { - -class Duration {}; - -Duration Nanoseconds(long long); -Duration Microseconds(long long); -Duration Milliseconds(long long); -Duration Seconds(long long); -Duration Minutes(long long); -Duration Hours(long long); - -#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \ - Duration NAME(float n); \ - Duration NAME(double n); \ - template \ - Duration NAME(T n); - -GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Seconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Minutes); -GENERATE_DURATION_FACTORY_OVERLOADS(Hours); -#undef GENERATE_DURATION_FACTORY_OVERLOADS - -} // namespace absl +// RUN: %check_clang_tidy %s abseil-duration-factory-float %t -- -- -I %S/Inputs + +#include "absl/time/time.h" void ConvertFloatTest() { absl::Duration d; Index: test/clang-tidy/abseil-duration-factory-scale.cpp =================================================================== --- test/clang-tidy/abseil-duration-factory-scale.cpp +++ test/clang-tidy/abseil-duration-factory-scale.cpp @@ -1,32 +1,6 @@ -// RUN: %check_clang_tidy %s abseil-duration-factory-scale %t +// RUN: %check_clang_tidy %s abseil-duration-factory-scale %t -- -- -I %S/Inputs -// Mimic the implementation of absl::Duration -namespace absl { - -class Duration {}; - -Duration Nanoseconds(long long); -Duration Microseconds(long long); -Duration Milliseconds(long long); -Duration Seconds(long long); -Duration Minutes(long long); -Duration Hours(long long); - -#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \ - Duration NAME(float n); \ - Duration NAME(double n); \ - template \ - Duration NAME(T n); - -GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Seconds); -GENERATE_DURATION_FACTORY_OVERLOADS(Minutes); -GENERATE_DURATION_FACTORY_OVERLOADS(Hours); -#undef GENERATE_DURATION_FACTORY_OVERLOADS - -} // namespace absl +#include "absl/time/time.h" void ScaleTest() { absl::Duration d; Index: test/clang-tidy/abseil-duration-subtraction.cpp =================================================================== --- /dev/null +++ test/clang-tidy/abseil-duration-subtraction.cpp @@ -0,0 +1,56 @@ +// RUN: %check_clang_tidy %s abseil-duration-subtraction %t -- -- -I %S/Inputs + +#include "absl/time/time.h" + +void f() { + double x; + absl::Duration d, d1; + + x = absl::ToDoubleSeconds(d) - 1.0; + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(1)) + x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(d1); + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: absl::ToDoubleSeconds(d - d1); + x = absl::ToDoubleSeconds(d) - 6.5 - 8.0; + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(6.5)) - 8.0; + x = absl::ToDoubleHours(d) - 1.0; + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: absl::ToDoubleHours(d - absl::Hours(1)) + x = absl::ToDoubleMinutes(d) - 1; + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: absl::ToDoubleMinutes(d - absl::Minutes(1)) + x = absl::ToDoubleMilliseconds(d) - 9; + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: absl::ToDoubleMilliseconds(d - absl::Milliseconds(9)) + x = absl::ToDoubleMicroseconds(d) - 9; + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: absl::ToDoubleMicroseconds(d - absl::Microseconds(9)) + x = absl::ToDoubleNanoseconds(d) - 42; + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: absl::ToDoubleNanoseconds(d - absl::Nanoseconds(42)) + + // We can rewrite the argument of the duration conversion +#define THIRTY absl::Seconds(30) + x = absl::ToDoubleSeconds(THIRTY) - 1.0; + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: absl::ToDoubleSeconds(THIRTY - absl::Seconds(1)) +#undef THIRTY + + // Some other contexts + if (absl::ToDoubleSeconds(d) - 1.0 > 10) {} + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction] + // CHECK-FIXES: if (absl::ToDoubleSeconds(d - absl::Seconds(1)) > 10) {} + + // These should not match + x = 5 - 6; + x = 4 - absl::ToDoubleSeconds(d) - 6.5 - 8.0; + x = absl::ToDoubleSeconds(d) + 1.0; + x = absl::ToDoubleSeconds(d) * 1.0; + x = absl::ToDoubleSeconds(d) / 1.0; + +#define MINUS_FIVE(z) absl::ToDoubleSeconds(z) - 5 + x = MINUS_FIVE(d); +#undef MINUS_FIVE +}