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