[PATCH] D75558: [clang-tidy] Update abseil-duration-unnecessary-conversion check to find more cases.
dblaikie added a comment.
@ymandel Sorry for this inconvenience, but Phabricator has a bug/missing
feature where it doesn't send email if a status change (like approving a patch)
has no user-authored text. In the future, please include some text ("Thanks!",
"Looks good", etc) in any approval to ensure that approval is reflected on the
LLVM mailing lists - otherwise it looks like patches are being committed
without review/approval.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75558/new/
https://reviews.llvm.org/D75558
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75558: [clang-tidy] Update abseil-duration-unnecessary-conversion check to find more cases.
This revision was automatically updated to reflect the committed changes. Closed by commit rG3860b2a0bd09: [clang-tidy] Update Abseil Duration Conversion check to find more cases. (authored by hwright). Changed prior to commit: https://reviews.llvm.org/D75558?vs=247988&id=250252#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75558/new/ https://reviews.llvm.org/D75558 Files: clang-tools-extra/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst clang-tools-extra/test/clang-tidy/checkers/abseil-duration-unnecessary-conversion.cpp Index: clang-tools-extra/test/clang-tidy/checkers/abseil-duration-unnecessary-conversion.cpp === --- clang-tools-extra/test/clang-tidy/checkers/abseil-duration-unnecessary-conversion.cpp +++ clang-tools-extra/test/clang-tidy/checkers/abseil-duration-unnecessary-conversion.cpp @@ -100,6 +100,44 @@ d2 = VALUE(d1); #undef VALUE + // Multiplication + d2 = absl::Nanoseconds(absl::ToDoubleNanoseconds(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Microseconds(absl::ToInt64Microseconds(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Milliseconds(absl::ToDoubleMilliseconds(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Seconds(absl::ToInt64Seconds(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Minutes(absl::ToDoubleMinutes(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Hours(absl::ToInt64Hours(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Nanoseconds(2 * absl::ToDoubleNanoseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Microseconds(2 * absl::ToInt64Microseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Milliseconds(2 * absl::ToDoubleMilliseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Seconds(2 * absl::ToInt64Seconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Minutes(2 * absl::ToDoubleMinutes(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Hours(2 * absl::ToInt64Hours(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + // These should not match d2 = absl::Seconds(absl::ToDoubleMilliseconds(d1)); d2 = absl::Seconds(4); @@ -108,4 +146,6 @@ 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))); + d2 = absl::Seconds(absl::ToInt64Milliseconds(d1) * 2); + d2 = absl::Milliseconds(absl::ToDoubleSeconds(d1) * 2); } Index: clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst === --- clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst +++ clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst @@ -40,6 +40,17 @@ // Suggestion - Remove division and conversion absl::Duration d2 = d1; +Unwrapping scalar operations: + +.. code-block:: c++ + + // Original - Multiplication by a scalar + absl::Duration d1; + absl::Duration d2 = absl::Seconds(absl::ToInt64Seconds(d1) * 2); + + // Suggestion - Remove unnecessary conversion + absl::Duration d2 = d1 * 2; + Note: Converting to an integer and back to an ``absl::Duration`` might be a truncating o
[PATCH] D75558: [clang-tidy] Update abseil-duration-unnecessary-conversion check to find more cases.
hwright created this revision. hwright added reviewers: aaron.ballman, ymandel. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. This check now handles cases where there's a scalar multiplication happening between the two conversions. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75558 Files: clang-tools-extra/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst clang-tools-extra/test/clang-tidy/checkers/abseil-duration-unnecessary-conversion.cpp Index: clang-tools-extra/test/clang-tidy/checkers/abseil-duration-unnecessary-conversion.cpp === --- clang-tools-extra/test/clang-tidy/checkers/abseil-duration-unnecessary-conversion.cpp +++ clang-tools-extra/test/clang-tidy/checkers/abseil-duration-unnecessary-conversion.cpp @@ -100,6 +100,44 @@ d2 = VALUE(d1); #undef VALUE + // Multiplication + d2 = absl::Nanoseconds(absl::ToDoubleNanoseconds(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Microseconds(absl::ToInt64Microseconds(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Milliseconds(absl::ToDoubleMilliseconds(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Seconds(absl::ToInt64Seconds(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Minutes(absl::ToDoubleMinutes(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Hours(absl::ToInt64Hours(d1) * 2); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = d1 * 2 + d2 = absl::Nanoseconds(2 * absl::ToDoubleNanoseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Microseconds(2 * absl::ToInt64Microseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Milliseconds(2 * absl::ToDoubleMilliseconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Seconds(2 * absl::ToInt64Seconds(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Minutes(2 * absl::ToDoubleMinutes(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + d2 = absl::Hours(2 * absl::ToInt64Hours(d1)); + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove unnecessary absl::Duration conversions [abseil-duration-unnecessary-conversion] + // CHECK-FIXES: d2 = 2 * d1 + // These should not match d2 = absl::Seconds(absl::ToDoubleMilliseconds(d1)); d2 = absl::Seconds(4); @@ -108,4 +146,6 @@ 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))); + d2 = absl::Seconds(absl::ToInt64Milliseconds(d1) * 2); + d2 = absl::Milliseconds(absl::ToDoubleSeconds(d1) * 2); } Index: clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst === --- clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst +++ clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst @@ -40,6 +40,17 @@ // Suggestion - Remove division and conversion absl::Duration d2 = d1; +Unwrapping scalar operations: + +.. code-block:: c++ + + // Original - Multiplication by a scalar + absl::Duration d1; + absl::Duration d2 = absl::Seconds(absl::ToInt64Seconds(d1) * 2); + + // Suggestion - Remove unnecessary conversion + absl::Duration d2 = d1 * 2; + 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 w
