[clang-tools-extra] r349073 - [clang-tidy] Add the abseil-duration-subtraction check
Hyrum Wright via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 13 11:23:52 PST 2018
Author: hwright
Date: Thu Dec 13 11:23:52 2018
New Revision: 349073
URL: http://llvm.org/viewvc/llvm-project?rev=349073&view=rev
Log:
[clang-tidy] Add the abseil-duration-subtraction check
Summary:
This check uses the context of a subtraction expression as well as knowledge
about the Abseil Time types, to infer the type of the second operand of some
subtraction expressions in Duration conversions. For example:
absl::ToDoubleSeconds(duration) - foo
can become
absl::ToDoubleSeconds(duration - absl::Seconds(foo))
This ensures that time calculations are done in the proper domain, and also
makes it easier to further deduce the types of the second operands to these
expressions.
Reviewed By: JonasToth
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D55245
Added:
clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=349073&r1=349072&r2=349073&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Thu Dec 13 11:23:52 2018
@@ -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 @@ public:
"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>(
Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=349073&r1=349072&r2=349073&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Thu Dec 13 11:23:52 2018
@@ -7,6 +7,7 @@ add_clang_library(clangTidyAbseilModule
DurationFactoryFloatCheck.cpp
DurationFactoryScaleCheck.cpp
DurationRewriter.cpp
+ DurationSubtractionCheck.cpp
FasterStrsplitDelimiterCheck.cpp
NoInternalDependenciesCheck.cpp
NoNamespaceCheck.cpp
Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp?rev=349073&r1=349072&r2=349073&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationComparisonCheck.cpp Thu Dec 13 11:23:52 2018
@@ -19,101 +19,6 @@ namespace clang {
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(">="),
Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp?rev=349073&r1=349072&r2=349073&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp Thu Dec 13 11:23:52 2018
@@ -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 clang {
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 @@ truncateIfIntegral(const FloatingLiteral
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>
+rewriteInverseDurationCall(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 @@ std::string simplifyDurationFactoryArg(c
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 =
+ rewriteInverseDurationCall(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
Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h?rev=349073&r1=349072&r2=349073&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h Thu Dec 13 11:23:52 2018
@@ -19,34 +19,15 @@ namespace tidy {
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,
Microseconds,
Nanoseconds,
};
-} // namespace abseil
-} // namespace tidy
-} // namespace clang
-
-namespace std {
-template <> struct hash<::clang::tidy::abseil::DurationScale> {
- using argument_type = ::clang::tidy::abseil::DurationScale;
- using underlying_type = std::underlying_type<argument_type>::type;
- using result_type = std::hash<underlying_type>::result_type;
-
- result_type operator()(const argument_type &arg) const {
- std::hash<underlying_type> hasher;
- return hasher(static_cast<underlying_type>(arg));
- }
-};
-} // namespace std
-namespace clang {
-namespace tidy {
-namespace abseil {
/// Given a `Scale`, return the appropriate factory function call for
/// constructing a `Duration` for that scale.
llvm::StringRef getFactoryForScale(DurationScale Scale);
@@ -78,6 +59,16 @@ std::string
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;
Added: clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.cpp?rev=349073&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.cpp Thu Dec 13 11:23:52 2018
@@ -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() || Binop->getExprLoc().isInvalid())
+ 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
Added: clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.h?rev=349073&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationSubtractionCheck.h Thu Dec 13 11:23:52 2018
@@ -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
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=349073&r1=349072&r2=349073&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Thu Dec 13 11:23:52 2018
@@ -93,6 +93,12 @@ Improvements to clang-tidy
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.
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst?rev=349073&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-subtraction.rst Thu Dec 13 11:23:52 2018
@@ -0,0 +1,36 @@
+.. 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);
+
+Note: As with other ``clang-tidy`` checks, it is possible that multiple fixes
+may overlap (as in the case of nested expressions), so not all occurences can
+be transformed in one run. In particular, this may occur for nested subtraction
+expressions. Running ``clang-tidy`` multiple times will find and fix these
+overlaps.
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=349073&r1=349072&r2=349073&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Thu Dec 13 11:23:52 2018
@@ -8,6 +8,7 @@ Clang-Tidy Checks
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
Added: clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp?rev=349073&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-subtraction.cpp Thu Dec 13 11:23:52 2018
@@ -0,0 +1,64 @@
+// 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, d2;
+
+ 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) {}
+
+ // A nested occurance
+ x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(absl::Seconds(5));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+ // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(5))
+ x = absl::ToDoubleSeconds(d) - absl::ToDoubleSeconds(absl::Seconds(absl::ToDoubleSeconds(d1)));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the duration domain [abseil-duration-subtraction]
+ // CHECK-FIXES: absl::ToDoubleSeconds(d - absl::Seconds(absl::ToDoubleSeconds(d1)))
+
+ // 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
+}
More information about the cfe-commits
mailing list