[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
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=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 , const Expr ) {
auto ZeroMatcher =
@@ -197,6 +216,22 @@
return ScaleIter->second;
}
+llvm::Optional getScaleForTimeInverse(llvm::StringRef Name) {
+ static const llvm::StringMap 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 , 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 ) override {
+CheckFactories.registerCheck(
+"abseil-duration-addition");
CheckFactories.registerCheck(
"abseil-duration-comparison");
CheckFactories.registerCheck(
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(
+
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57185/new/ https://reviews.llvm.org/D57185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
hwright updated this revision to Diff 183583.
hwright marked an inline comment as done.
hwright added a comment.
Add another test
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57185/new/
https://reviews.llvm.org/D57185
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationAdditionCheck.cpp
clang-tidy/abseil/DurationAdditionCheck.h
clang-tidy/abseil/DurationRewriter.cpp
clang-tidy/abseil/DurationRewriter.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-addition.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/Inputs/absl/time/time.h
test/clang-tidy/abseil-duration-addition.cpp
Index: test/clang-tidy/abseil-duration-addition.cpp
===
--- /dev/null
+++ 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 =
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
JonasToth added inline comments.
Comment at: test/clang-tidy/abseil-duration-addition.cpp:84
+#undef PLUS_FIVE
+}
hwright wrote:
> JonasToth wrote:
> > a view template test-cases would be good to have.
> I'm not sure I know that terminology; do you have an example?
```
template
void foo(absl::Time t) {
int i = absl::ToUnixNanos(t) + T{};
}
foo(t);
foo(t);
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57185/new/
https://reviews.llvm.org/D57185
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
hwright marked 2 inline comments as done. hwright added inline comments. Comment at: test/clang-tidy/abseil-duration-addition.cpp:84 +#undef PLUS_FIVE +} JonasToth wrote: > a view template test-cases would be good to have. I'm not sure I know that terminology; do you have an example? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57185/new/ https://reviews.llvm.org/D57185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
JonasToth added inline comments.
Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:22
+void DurationAdditionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(hasOperatorName("+"),
hwright wrote:
> JonasToth wrote:
> > This whole file is structurally similar to the
> > `DurationSubtractionCheck.cpp` equivalent (which is expected :)), maybe
> > some parts could be refactored out?
> > Will there be a check for duration scaling or the like?
> I think that most of it already has been factored out (e.g.,
> `rewriteExprFromNumberToDuration` and `getScaleForTimeInverse`, etc) . The
> actual meat here is pretty light: the matcher, and then the diagnostic
> generation.
>
> A scaling change may also follow. Our experience has been that scaling isn't
> quite straight forward, as the scaling factor may have semantic meaning which
> changes the result, but which isn't captured by the type system. Probably
> not a design discussion to have here, but suffice it to say that I don't know
> if this is yet settled.
Your right, there is not too much to gain. Only if there are more checks coming
with the same structure it makes sense to think again.
Comment at: test/clang-tidy/abseil-duration-addition.cpp:84
+#undef PLUS_FIVE
+}
a view template test-cases would be good to have.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57185/new/
https://reviews.llvm.org/D57185
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
hwright added inline comments.
Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:22
+void DurationAdditionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(hasOperatorName("+"),
JonasToth wrote:
> This whole file is structurally similar to the `DurationSubtractionCheck.cpp`
> equivalent (which is expected :)), maybe some parts could be refactored out?
> Will there be a check for duration scaling or the like?
I think that most of it already has been factored out (e.g.,
`rewriteExprFromNumberToDuration` and `getScaleForTimeInverse`, etc) . The
actual meat here is pretty light: the matcher, and then the diagnostic
generation.
A scaling change may also follow. Our experience has been that scaling isn't
quite straight forward, as the scaling factor may have semantic meaning which
changes the result, but which isn't captured by the type system. Probably not
a design discussion to have here, but suffice it to say that I don't know if
this is yet settled.
Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:50
+diag(Binop->getBeginLoc(), "perform addition in the duration domain")
+<< FixItHint::CreateReplacement(
+ Binop->getSourceRange(),
JonasToth wrote:
> The only difference between the two `diag` seems to be the `FixItHint`. I
> would rather refactor the diag out of the `if`.
This doesn't really de-dupe very much, since the bulk of the work here is
creating the `FixItHint`. Done.
Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:60
+ } else {
+assert(Call == Binop->getRHS()->IgnoreImpCast() && "Call should be found
on the RHS");
+diag(Binop->getBeginLoc(), "perform addition in the duration domain")
JonasToth wrote:
> What happens with paren-casts, are they ignored as well? (See one comment in
> the test-cases)
Now they are.
Comment at: test/clang-tidy/abseil-duration-addition.cpp:28
+
+ i = 3 + absl::ToUnixHours(t);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration
domain [abseil-duration-addition]
JonasToth wrote:
> Call is on the right side, what happens for `3 + (abs::ToUnixHours(t));`
Added this test, and updated to ignore paren casts.
Comment at: test/clang-tidy/abseil-duration-addition.cpp:48
+ // 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]
JonasToth wrote:
> Is something like this
> ```
> i += absl::ToInt64MicroSeconds(absl::Seconds(1));
> ```
> possible (syntactically and semantically)?
> The compound operators are currently not covered by this (and the
> subtraction-check), but in this case it looks like a possible use-case.
Semantically, this is possible, but wouldn't fall under this transform, since
it would require changing the type of `i`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57185/new/
https://reviews.llvm.org/D57185
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
hwright updated this revision to Diff 183539.
hwright marked 11 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57185/new/
https://reviews.llvm.org/D57185
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationAdditionCheck.cpp
clang-tidy/abseil/DurationAdditionCheck.h
clang-tidy/abseil/DurationRewriter.cpp
clang-tidy/abseil/DurationRewriter.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-addition.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/Inputs/absl/time/time.h
test/clang-tidy/abseil-duration-addition.cpp
Index: test/clang-tidy/abseil-duration-addition.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-addition.cpp
@@ -0,0 +1,84 @@
+// 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 +=
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
hwright updated this revision to Diff 183531.
hwright added a comment.
Sort release notes
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57185/new/
https://reviews.llvm.org/D57185
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationAdditionCheck.cpp
clang-tidy/abseil/DurationAdditionCheck.h
clang-tidy/abseil/DurationRewriter.cpp
clang-tidy/abseil/DurationRewriter.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-addition.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/Inputs/absl/time/time.h
test/clang-tidy/abseil-duration-addition.cpp
Index: test/clang-tidy/abseil-duration-addition.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-addition.cpp
@@ -0,0 +1,78 @@
+// 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))
+
+ // 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;
+
+#define PLUS_FIVE(z) absl::ToUnixSeconds(z) + 5
+ i = PLUS_FIVE(t);
+#undef PLUS_FIVE
+}
Index: test/clang-tidy/Inputs/absl/time/time.h
===
---
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
JonasToth added inline comments.
Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:22
+void DurationAdditionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(hasOperatorName("+"),
This whole file is structurally similar to the `DurationSubtractionCheck.cpp`
equivalent (which is expected :)), maybe some parts could be refactored out?
Will there be a check for duration scaling or the like?
Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:50
+diag(Binop->getBeginLoc(), "perform addition in the duration domain")
+<< FixItHint::CreateReplacement(
+ Binop->getSourceRange(),
The only difference between the two `diag` seems to be the `FixItHint`. I would
rather refactor the diag out of the `if`.
Comment at: clang-tidy/abseil/DurationAdditionCheck.cpp:60
+ } else {
+assert(Call == Binop->getRHS()->IgnoreImpCast() && "Call should be found
on the RHS");
+diag(Binop->getBeginLoc(), "perform addition in the duration domain")
What happens with paren-casts, are they ignored as well? (See one comment in
the test-cases)
Comment at: test/clang-tidy/abseil-duration-addition.cpp:28
+
+ i = 3 + absl::ToUnixHours(t);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform addition in the duration
domain [abseil-duration-addition]
Call is on the right side, what happens for `3 + (abs::ToUnixHours(t));`
Comment at: test/clang-tidy/abseil-duration-addition.cpp:48
+ // 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]
Is something like this
```
i += absl::ToInt64MicroSeconds(absl::Seconds(1));
```
possible (syntactically and semantically)?
The compound operators are currently not covered by this (and the
subtraction-check), but in this case it looks like a possible use-case.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57185/new/
https://reviews.llvm.org/D57185
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:76 +- New :doc:`abseil-duration-addition + ` check. Please use alphabetical order for new checks. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57185/new/ https://reviews.llvm.org/D57185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57185: [clang-tidy] Add the abseil-duration-addition check
hwright created this revision.
hwright added reviewers: hokein, aaron.ballman, JonasToth.
hwright added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
This is an analog to the existing `abseil-duration-subtraction` check.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D57185
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationAdditionCheck.cpp
clang-tidy/abseil/DurationAdditionCheck.h
clang-tidy/abseil/DurationRewriter.cpp
clang-tidy/abseil/DurationRewriter.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-addition.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/Inputs/absl/time/time.h
test/clang-tidy/abseil-duration-addition.cpp
Index: test/clang-tidy/abseil-duration-addition.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-addition.cpp
@@ -0,0 +1,78 @@
+// 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))
+
+ // 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;
+
+#define PLUS_FIVE(z) absl::ToUnixSeconds(z) + 5
+ i = PLUS_FIVE(t);
+#undef
