hwright updated this revision to Diff 177509.
hwright marked 5 inline comments as done.
hwright added a comment.
Use `static_cast` instead of a `switch` for `IndexedMap` lookup.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55245/new/
https://reviews.llvm.org/D55245
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationComparisonCheck.cpp
clang-tidy/abseil/DurationRewriter.cpp
clang-tidy/abseil/DurationRewriter.h
clang-tidy/abseil/DurationSubtractionCheck.cpp
clang-tidy/abseil/DurationSubtractionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-subtraction.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/Inputs/absl/time/time.h
test/clang-tidy/abseil-duration-comparison.cpp
test/clang-tidy/abseil-duration-factory-float.cpp
test/clang-tidy/abseil-duration-factory-scale.cpp
test/clang-tidy/abseil-duration-subtraction.cpp
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
+}
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 <typename T> \
- 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-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 <typename T> \
- 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-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 <typename T> \
- 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/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 <typename T> \
+ 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: 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: 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/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
+ <clang-tidy/checks/abseil-duration-subtraction>` check.
+
+ Checks for cases where subtraction should be performed in the
+ ``absl::Duration`` domain.
+
- New :doc:`abseil-faster-strsplit-delimiter
<clang-tidy/checks/abseil-faster-strsplit-delimiter>` check.
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<BinaryOperator>("binop");
+ const auto *FuncDecl = Result.Nodes.getNodeAs<FunctionDecl>("function_decl");
+
+ // Don't try to replace things inside of macro definitions.
+ if (Binop->getExprLoc().isMacroID())
+ return;
+
+ llvm::Optional<DurationScale> Scale = getScaleForInverse(FuncDecl->getName());
+ if (!Scale)
+ return;
+
+ llvm::Optional<std::string> RhsReplacement =
+ rewriteExprFromNumberToDuration(Result, *Scale, Binop->getRHS());
+ if (!RhsReplacement)
+ return;
+
+ const Expr *LhsArg = Result.Nodes.getNodeAs<Expr>("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: 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<DurationScale> 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<std::string> rewriteExprFromNumberToDuration(
+ const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale,
+ const Expr *Node);
+
AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<FunctionDecl>,
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<unsigned>(Scale);
+ }
+};
+
/// Returns an integer if the fractional part of a `FloatingLiteral` is `0`.
static llvm::Optional<llvm::APSInt>
truncateIfIntegral(const FloatingLiteral &FloatLiteral) {
@@ -29,6 +37,55 @@
return llvm::None;
}
+/// Given a `Scale` return the inverse functions for it.
+static const std::pair<llvm::StringRef, llvm::StringRef> &
+getInverseForScale(DurationScale Scale) {
+ static const llvm::IndexedMap<std::pair<llvm::StringRef, llvm::StringRef>,
+ DurationScale2IndexFunctor>
+ InverseMap = []() {
+ // TODO: Revisit the immediately invoked lamba technique when
+ // IndexedMap gets an initializer list constructor.
+ llvm::IndexedMap<std::pair<llvm::StringRef, llvm::StringRef>,
+ 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<std::string>
+maybeRewriteInverseDurationCall(const MatchFinder::MatchResult &Result,
+ DurationScale Scale, const Expr &Node) {
+ const std::pair<llvm::StringRef, llvm::StringRef> &InverseFunctions =
+ getInverseForScale(Scale);
+ if (const auto *MaybeCallArg = selectFirst<const Expr>(
+ "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<DurationScale> getScaleForInverse(llvm::StringRef Name) {
+ static const llvm::StringMap<DurationScale> 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<std::string> 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<std::string> 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/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<DurationScale> getScaleForInverse(llvm::StringRef Name) {
- static const llvm::DenseMap<llvm::StringRef, DurationScale> 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<llvm::StringRef, llvm::StringRef> &
-getInverseForScale(DurationScale Scale) {
- static const std::unordered_map<DurationScale,
- std::pair<llvm::StringRef, llvm::StringRef>>
- 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<std::string>
-maybeRewriteInverseDurationCall(const MatchFinder::MatchResult &Result,
- DurationScale Scale, const Expr &Node) {
- const std::pair<std::string, std::string> &InverseFunctions =
- getInverseForScale(Scale);
- if (const Expr *MaybeCallArg = selectFirst<const Expr>(
- "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<std::string> 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<std::string> 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/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/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<DurationFactoryScaleCheck>(
"abseil-duration-factory-scale");
+ CheckFactories.registerCheck<DurationSubtractionCheck>(
+ "abseil-duration-subtraction");
CheckFactories.registerCheck<FasterStrsplitDelimiterCheck>(
"abseil-faster-strsplit-delimiter");
CheckFactories.registerCheck<NoInternalDependenciesCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
