This revision was automatically updated to reflect the committed changes.
Closed by commit rL352362: [clang-tidy] Add the abseil-duration-addition check
(authored by hwright, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D57185?vs=183583&id=183840#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57185/new/
https://reviews.llvm.org/D57185
Files:
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/DurationAdditionCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationAdditionCheck.h
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/abseil-duration-addition.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h
clang-tools-extra/trunk/test/clang-tidy/abseil-duration-addition.cpp
Index: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
@@ -103,6 +103,25 @@
llvm_unreachable("unknown scaling factor");
}
+/// Returns the Time factory function name for a given `Scale`.
+llvm::StringRef getTimeFactoryForScale(DurationScale scale) {
+ switch (scale) {
+ case DurationScale::Hours:
+ return "absl::ToUnixHours";
+ case DurationScale::Minutes:
+ return "absl::ToUnixMinutes";
+ case DurationScale::Seconds:
+ return "absl::ToUnixSeconds";
+ case DurationScale::Milliseconds:
+ return "absl::ToUnixMillis";
+ case DurationScale::Microseconds:
+ return "absl::ToUnixMicros";
+ case DurationScale::Nanoseconds:
+ return "absl::ToUnixNanos";
+ }
+ llvm_unreachable("unknown scaling factor");
+}
+
/// Returns `true` if `Node` is a value which evaluates to a literal `0`.
bool IsLiteralZero(const MatchFinder::MatchResult &Result, const Expr &Node) {
auto ZeroMatcher =
@@ -197,6 +216,22 @@
return ScaleIter->second;
}
+llvm::Optional<DurationScale> getScaleForTimeInverse(llvm::StringRef Name) {
+ static const llvm::StringMap<DurationScale> ScaleMap(
+ {{"ToUnixHours", DurationScale::Hours},
+ {"ToUnixMinutes", DurationScale::Minutes},
+ {"ToUnixSeconds", DurationScale::Seconds},
+ {"ToUnixMillis", DurationScale::Milliseconds},
+ {"ToUnixMicros", DurationScale::Microseconds},
+ {"ToUnixNanos", DurationScale::Nanoseconds}});
+
+ auto ScaleIter = ScaleMap.find(std::string(Name));
+ if (ScaleIter == ScaleMap.end())
+ return llvm::None;
+
+ return ScaleIter->second;
+}
+
std::string rewriteExprFromNumberToDuration(
const ast_matchers::MatchFinder::MatchResult &Result, DurationScale Scale,
const Expr *Node) {
Index: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
@@ -9,6 +9,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "DurationAdditionCheck.h"
#include "DurationComparisonCheck.h"
#include "DurationConversionCastCheck.h"
#include "DurationDivisionCheck.h"
@@ -30,6 +31,8 @@
class AbseilModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<DurationAdditionCheck>(
+ "abseil-duration-addition");
CheckFactories.registerCheck<DurationComparisonCheck>(
"abseil-duration-comparison");
CheckFactories.registerCheck<DurationConversionCastCheck>(
Index: clang-tools-extra/trunk/clang-tidy/abseil/DurationAdditionCheck.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationAdditionCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationAdditionCheck.cpp
@@ -0,0 +1,73 @@
+//===--- DurationAdditionCheck.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 "DurationAdditionCheck.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 DurationAdditionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(hasOperatorName("+"),
+ hasEitherOperand(expr(ignoringParenImpCasts(
+ callExpr(callee(functionDecl(TimeFactoryFunction())
+ .bind("function_decl")))
+ .bind("call")))))
+ .bind("binop"),
+ this);
+}
+
+void DurationAdditionCheck::check(const MatchFinder::MatchResult &Result) {
+ const BinaryOperator *Binop =
+ Result.Nodes.getNodeAs<clang::BinaryOperator>("binop");
+ const CallExpr *Call = Result.Nodes.getNodeAs<clang::CallExpr>("call");
+
+ // Don't try to replace things inside of macro definitions.
+ if (Binop->getExprLoc().isMacroID() || Binop->getExprLoc().isInvalid())
+ return;
+
+ llvm::Optional<DurationScale> Scale = getScaleForTimeInverse(
+ Result.Nodes.getNodeAs<clang::FunctionDecl>("function_decl")->getName());
+ if (!Scale)
+ return;
+
+ llvm::StringRef TimeFactory = getTimeFactoryForScale(*Scale);
+
+ FixItHint Hint;
+ if (Call == Binop->getLHS()->IgnoreParenImpCasts()) {
+ Hint = FixItHint::CreateReplacement(
+ Binop->getSourceRange(),
+ (llvm::Twine(TimeFactory) + "(" +
+ tooling::fixit::getText(*Call->getArg(0), *Result.Context) + " + " +
+ rewriteExprFromNumberToDuration(Result, *Scale, Binop->getRHS()) + ")")
+ .str());
+ } else {
+ assert(Call == Binop->getRHS()->IgnoreParenImpCast() &&
+ "Call should be found on the RHS");
+ Hint = FixItHint::CreateReplacement(
+ Binop->getSourceRange(),
+ (llvm::Twine(TimeFactory) + "(" +
+ rewriteExprFromNumberToDuration(Result, *Scale, Binop->getLHS()) +
+ " + " + tooling::fixit::getText(*Call->getArg(0), *Result.Context) +
+ ")")
+ .str());
+ }
+
+ diag(Binop->getBeginLoc(), "perform addition in the duration domain") << Hint;
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
===================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
@@ -31,6 +31,9 @@
/// constructing a `Duration` for that scale.
llvm::StringRef getDurationFactoryForScale(DurationScale Scale);
+/// Returns the Time factory function name for a given `Scale`.
+llvm::StringRef getTimeFactoryForScale(DurationScale scale);
+
// Determine if `Node` represents a literal floating point or integral zero.
bool IsLiteralZero(const ast_matchers::MatchFinder::MatchResult &Result,
const Expr &Node);
@@ -62,6 +65,10 @@
/// return its `DurationScale`, or `None` if a match is not found.
llvm::Optional<DurationScale> getScaleForDurationInverse(llvm::StringRef Name);
+/// Given the name of an inverse Time function (e.g., `ToUnixSeconds`),
+/// return its `DurationScale`, or `None` if a match is not found.
+llvm::Optional<DurationScale> getScaleForTimeInverse(llvm::StringRef Name);
+
/// Given a `Scale` return the fully qualified inverse functions for it.
/// The first returned value is the inverse for `double`, and the second
/// returned value is the inverse for `int64`.
@@ -99,6 +106,14 @@
"::absl::Minutes", "::absl::Hours"));
}
+AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<FunctionDecl>,
+ TimeFactoryFunction) {
+ using namespace clang::ast_matchers;
+ return functionDecl(hasAnyName(
+ "::absl::ToUnixHours", "::absl::ToUnixMinutes", "::absl::ToUnixSeconds",
+ "::absl::ToUnixMillis", "::absl::ToUnixMicros", "::absl::ToUnixNanos"));
+}
+
} // namespace abseil
} // namespace tidy
} // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/abseil/DurationAdditionCheck.h
===================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationAdditionCheck.h
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationAdditionCheck.h
@@ -0,0 +1,35 @@
+//===--- DurationAdditionCheck.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_TIMEADDITIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEADDITIONCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Checks for cases where addition should be performed in the
+/// ``absl::Time`` domain.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-addition.html
+class DurationAdditionCheck : public ClangTidyCheck {
+public:
+ DurationAdditionCheck(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_TIMEADDITIONCHECK_H
Index: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
===================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
@@ -2,6 +2,7 @@
add_clang_library(clangTidyAbseilModule
AbseilTidyModule.cpp
+ DurationAdditionCheck.cpp
DurationComparisonCheck.cpp
DurationConversionCastCheck.cpp
DurationDivisionCheck.cpp
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
Improvements to clang-tidy
--------------------------
+- New :doc:`abseil-duration-addition
+ <clang-tidy/checks/abseil-duration-addition>` check.
+
+ Checks for cases where addition should be performed in the ``absl::Time``
+ domain.
+
- New :doc:`abseil-duration-conversion-cast
<clang-tidy/checks/abseil-duration-conversion-cast>` check.
@@ -80,7 +86,6 @@
Checks whether there are underscores in googletest test and test case names in
test macros, which is prohibited by the Googletest FAQ.
-
Improvements to include-fixer
-----------------------------
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-addition.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-addition.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-addition.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - abseil-duration-addition
+
+abseil-duration-addition
+========================
+
+Check for cases where addition should be performed in the ``absl::Time`` domain.
+When adding two values, and one is known to be an ``absl::Time``, we can infer
+that the other should be interpreted as an ``absl::Duration`` of a similar
+scale, and make that inference explicit.
+
+Examples:
+
+.. code-block:: c++
+
+ // Original - Addition in the integer domain
+ int x;
+ absl::Time t;
+ int result = absl::ToUnixSeconds(t) + x;
+
+ // Suggestion - Addition in the absl::Time domain
+ int result = absl::TounixSeconds(t + absl::Seconds(x));
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
=================
.. toctree::
+ abseil-duration-addition
abseil-duration-comparison
abseil-duration-conversion-cast
abseil-duration-division
Index: clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h
@@ -55,6 +55,19 @@
int64_t ToInt64Microseconds(Duration d);
int64_t ToInt64Nanoseconds(Duration d);
+int64_t ToUnixHours(Time t);
+int64_t ToUnixMinutes(Time t);
+int64_t ToUnixSeconds(Time t);
+int64_t ToUnixMillis(Time t);
+int64_t ToUnixMicros(Time t);
+int64_t ToUnixNanos(Time t);
+Time FromUnixHours(int64_t);
+Time FromUnixMinutes(int64_t);
+Time FromUnixSeconds(int64_t);
+Time FromUnixMillis(int64_t);
+Time FromUnixMicros(int64_t);
+Time FromUnixNanos(int64_t);
+
// Relational Operators
constexpr bool operator<(Duration lhs, Duration rhs);
constexpr bool operator>(Duration lhs, Duration rhs);
Index: clang-tools-extra/trunk/test/clang-tidy/abseil-duration-addition.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-addition.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-addition.cpp
@@ -0,0 +1,98 @@
+// RUN: %check_clang_tidy %s abseil-duration-addition %t -- -- -I%S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+ absl::Time t;
+ int i;
+
+ i = absl::ToUnixHours(t) + 5;
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixHours(t + absl::Hours(5))
+ i = absl::ToUnixMinutes(t) + 5;
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixMinutes(t + absl::Minutes(5))
+ i = absl::ToUnixSeconds(t) + 5;
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixSeconds(t + absl::Seconds(5))
+ i = absl::ToUnixMillis(t) + 5;
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixMillis(t + absl::Milliseconds(5))
+ i = absl::ToUnixMicros(t) + 5;
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixMicros(t + absl::Microseconds(5))
+ i = absl::ToUnixNanos(t) + 5;
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixNanos(t + absl::Nanoseconds(5))
+
+ i = 3 + absl::ToUnixHours(t);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixHours(absl::Hours(3) + t)
+ i = 3 + absl::ToUnixMinutes(t);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixMinutes(absl::Minutes(3) + t)
+ i = 3 + absl::ToUnixSeconds(t);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixSeconds(absl::Seconds(3) + t)
+ i = 3 + absl::ToUnixMillis(t);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixMillis(absl::Milliseconds(3) + t)
+ i = 3 + absl::ToUnixMicros(t);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixMicros(absl::Microseconds(3) + t)
+ i = 3 + absl::ToUnixNanos(t);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixNanos(absl::Nanoseconds(3) + t)
+
+ // Undoing inverse conversions
+ i = absl::ToUnixMicros(t) + absl::ToInt64Microseconds(absl::Seconds(1));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixMicros(t + absl::Seconds(1))
+
+ // Parens
+ i = 3 + (absl::ToUnixHours(t));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixHours(absl::Hours(3) + t)
+
+ // Float folding
+ i = absl::ToUnixSeconds(t) + 5.0;
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixSeconds(t + absl::Seconds(5))
+
+ // We can rewrite the argument of the duration conversion
+#define THIRTY absl::FromUnixSeconds(30)
+ i = absl::ToUnixSeconds(THIRTY) + 1;
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixSeconds(THIRTY + absl::Seconds(1))
+#undef THIRTY
+
+ // Some other contexts
+ if (absl::ToUnixSeconds(t) + 1.0 > 10) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixSeconds(t + absl::Seconds(1))
+
+ // These should not match
+ i = 5 + 6;
+ i = absl::ToUnixSeconds(t) - 1.0;
+ i = absl::ToUnixSeconds(t) * 1.0;
+ i = absl::ToUnixSeconds(t) / 1.0;
+ i += absl::ToInt64Microseconds(absl::Seconds(1));
+
+#define PLUS_FIVE(z) absl::ToUnixSeconds(z) + 5
+ i = PLUS_FIVE(t);
+#undef PLUS_FIVE
+}
+
+// Within a templated function
+template<typename T>
+void foo(absl::Time t) {
+ int i = absl::ToUnixNanos(t) + T{};
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: perform addition in the duration domain [abseil-duration-addition]
+ // CHECK-FIXES: absl::ToUnixNanos(t + absl::Nanoseconds(T{}))
+}
+
+void g() {
+ absl::Time t;
+ foo<int>(t);
+ foo<double>(t);
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
