[clang-tools-extra] r356141 - [clang-tidy] Add additional patterns to the abseil-duration-unnecessary-conversion check.
Hyrum Wright via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 14 06:38:16 PDT 2019
Author: hwright
Date: Thu Mar 14 06:38:16 2019
New Revision: 356141
URL: http://llvm.org/viewvc/llvm-project?rev=356141&view=rev
Log:
[clang-tidy] Add additional patterns to the abseil-duration-unnecessary-conversion check.
Differential Revision: https://reviews.llvm.org/D59183
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h
clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp
Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp?rev=356141&r1=356140&r2=356141&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp Thu Mar 14 06:38:16 2019
@@ -28,12 +28,35 @@ void DurationUnnecessaryConversionCheck:
std::string IntegerConversion =
(llvm::Twine("::absl::ToInt64") + Scale).str();
+ // Matcher which matches the current scale's factory with a `1` argument,
+ // e.g. `absl::Seconds(1)`.
+ auto factory_matcher = cxxConstructExpr(hasArgument(
+ 0,
+ callExpr(callee(functionDecl(hasName(DurationFactory))),
+ hasArgument(0, ignoringImpCasts(integerLiteral(equals(1)))))));
+
+ // Matcher which matches either inverse function and binds its argument,
+ // e.g. `absl::ToDoubleSeconds(dur)`.
+ auto inverse_function_matcher = callExpr(
+ callee(functionDecl(hasAnyName(FloatConversion, IntegerConversion))),
+ hasArgument(0, expr().bind("arg")));
+
+ // Matcher which matches a duration divided by the factory_matcher above,
+ // e.g. `dur / absl::Seconds(1)`.
+ auto division_operator_matcher = cxxOperatorCallExpr(
+ hasOverloadedOperatorName("/"), hasArgument(0, expr().bind("arg")),
+ hasArgument(1, factory_matcher));
+
+ // Matcher which matches a duration argument to `FDivDuration`,
+ // e.g. `absl::FDivDuration(dur, absl::Seconds(1))`
+ auto fdiv_matcher = callExpr(
+ callee(functionDecl(hasName("::absl::FDivDuration"))),
+ hasArgument(0, expr().bind("arg")), hasArgument(1, factory_matcher));
+
Finder->addMatcher(
- callExpr(
- callee(functionDecl(hasName(DurationFactory))),
- hasArgument(0, callExpr(callee(functionDecl(hasAnyName(
- FloatConversion, IntegerConversion))),
- hasArgument(0, expr().bind("arg")))))
+ callExpr(callee(functionDecl(hasName(DurationFactory))),
+ hasArgument(0, anyOf(inverse_function_matcher,
+ division_operator_matcher, fdiv_matcher)))
.bind("call"),
this);
}
@@ -47,7 +70,8 @@ void DurationUnnecessaryConversionCheck:
if (isInMacro(Result, OuterCall))
return;
- diag(OuterCall->getBeginLoc(), "remove unnecessary absl::Duration conversions")
+ diag(OuterCall->getBeginLoc(),
+ "remove unnecessary absl::Duration conversions")
<< FixItHint::CreateReplacement(
OuterCall->getSourceRange(),
tooling::fixit::getText(*Arg, *Result.Context));
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst?rev=356141&r1=356140&r2=356141&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst Thu Mar 14 06:38:16 2019
@@ -6,7 +6,7 @@ abseil-duration-unnecessary-conversion
Finds and fixes cases where ``absl::Duration`` values are being converted to
numeric types and back again.
-Examples:
+Floating-point examples:
.. code-block:: c++
@@ -17,6 +17,15 @@ Examples:
// Suggestion - Remove unnecessary conversions
absl::Duration d2 = d1;
+ // Original - Division to convert to double and back again
+ absl::Duration d2 = absl::Seconds(absl::FDivDuration(d1, absl::Seconds(1)));
+
+ // Suggestion - Remove division and conversion
+ absl::Duration d2 = d1;
+
+Integer examples:
+
+.. code-block:: c++
// Original - Conversion to integer and back again
absl::Duration d1;
@@ -25,6 +34,12 @@ Examples:
// Suggestion - Remove unnecessary conversions
absl::Duration d2 = d1;
+ // Original - Integer division followed by conversion
+ absl::Duration d2 = absl::Seconds(d1 / absl::Seconds(1));
+
+ // Suggestion - Remove division and 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
Modified: clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h?rev=356141&r1=356140&r2=356141&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h (original)
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h Thu Mar 14 06:38:16 2019
@@ -21,6 +21,7 @@ public:
template <typename T> Duration operator*(Duration lhs, T rhs);
template <typename T> Duration operator*(T lhs, Duration rhs);
template <typename T> Duration operator/(Duration lhs, T rhs);
+int64_t operator/(Duration lhs, Duration rhs);
class Time{};
@@ -86,4 +87,6 @@ inline Time operator+(Duration lhs, Time
inline Time operator-(Time lhs, Duration rhs);
inline Duration operator-(Time lhs, Time rhs);
+double FDivDuration(Duration num, Duration den);
+
} // namespace absl
Modified: clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp?rev=356141&r1=356140&r2=356141&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp Thu Mar 14 06:38:16 2019
@@ -8,42 +8,80 @@ void f() {
// Floating point
d2 = absl::Hours(absl::ToDoubleHours(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Minutes(absl::ToDoubleMinutes(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Seconds(absl::ToDoubleSeconds(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Milliseconds(absl::ToDoubleMilliseconds(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Microseconds(absl::ToDoubleMicroseconds(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Nanoseconds(absl::ToDoubleNanoseconds(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
// Integer point
d2 = absl::Hours(absl::ToInt64Hours(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Minutes(absl::ToInt64Minutes(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Seconds(absl::ToInt64Seconds(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Milliseconds(absl::ToInt64Milliseconds(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Microseconds(absl::ToInt64Microseconds(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
d2 = absl::Nanoseconds(absl::ToInt64Nanoseconds(d1));
// CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
- // CHECK-FIXES: d1
+ // CHECK-FIXES: d2 = d1
+
+ d2 = absl::Hours(d1 / absl::Hours(1));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Minutes(d1 / absl::Minutes(1));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Seconds(d1 / absl::Seconds(1));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Milliseconds(d1 / absl::Milliseconds(1));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Microseconds(d1 / absl::Microseconds(1));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Nanoseconds(d1 / absl::Nanoseconds(1));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+
+ d2 = absl::Hours(absl::FDivDuration(d1, absl::Hours(1)));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Minutes(absl::FDivDuration(d1, absl::Minutes(1)));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Seconds(absl::FDivDuration(d1, absl::Seconds(1)));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Milliseconds(absl::FDivDuration(d1, absl::Milliseconds(1)));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Microseconds(absl::FDivDuration(d1, absl::Microseconds(1)));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
+ d2 = absl::Nanoseconds(absl::FDivDuration(d1, absl::Nanoseconds(1)));
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion]
+ // CHECK-FIXES: d2 = d1
// As macro argument
#define PLUS_FIVE_S(x) x + absl::Seconds(5)
@@ -66,4 +104,8 @@ void f() {
d2 = absl::Seconds(absl::ToDoubleMilliseconds(d1));
d2 = absl::Seconds(4);
int i = absl::ToInt64Milliseconds(d1);
+ d2 = absl::Hours(d1 / absl::Minutes(1));
+ d2 = absl::Seconds(d1 / absl::Seconds(30));
+ d2 = absl::Hours(absl::FDivDuration(d1, absl::Minutes(1)));
+ d2 = absl::Milliseconds(absl::FDivDuration(d1, absl::Milliseconds(20)));
}
More information about the cfe-commits
mailing list