EricWF closed this revision.
EricWF added a comment.
Committed as r348633.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53830/new/
https://reviews.llvm.org/D53830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.or
hokein added a subscriber: hwright.
hokein added a comment.
+ @hwright who have implemented a bunch of `absl-duration-*` checks, you might
be interested in this as well.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53830/new/
https://reviews.llvm.org/D53830
astrelni marked 5 inline comments as done.
astrelni added a comment.
Sorry for the long delay.
I've reworked the template instantiation stuff a little bit yet again. Going to
still come back and comment with results of profiling but I think now this
shouldn't be much slower than if the template
astrelni updated this revision to Diff 174931.
https://reviews.llvm.org/D53830
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
clang-tidy/abseil/UpgradeDurationConversionsCheck.h
docs/ReleaseNotes.rst
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG with one more comment. Please address other reviewers' comments though.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:153
+ // required so we provide
astrelni updated this revision to Diff 174226.
astrelni added a comment.
Fix incorrect uploaded diff.
https://reviews.llvm.org/D53830
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
clang-tidy/abseil/Up
astrelni updated this revision to Diff 174218.
astrelni marked an inline comment as done.
astrelni added a comment.
Fix to use `hasAnyName` everywhere
https://reviews.llvm.org/D53830
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/UpgradeDu
alexfh added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:33-36
+ anyOf(hasAncestor(
+functionTemplateDecl(HasMatchingDependentDescendant)),
+hasAncestor(
+class
astrelni marked an inline comment as done.
astrelni added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:33-36
+ anyOf(hasAncestor(
+functionTemplateDecl(HasMatchingDependentDescendant)),
+
astrelni updated this revision to Diff 172402.
astrelni added a comment.
Herald added a subscriber: mgrang.
Updated filtering of template instantiations to not use potentially costly
hasDescendent matcher.
https://reviews.llvm.org/D53830
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-
alexfh added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:33-36
+ anyOf(hasAncestor(
+functionTemplateDecl(HasMatchingDependentDescendant)),
+hasAncestor(
+class
astrelni added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:151-152
+ const auto *ArgExpr = Result.Nodes.getNodeAs("arg");
+ llvm::StringRef Message = "perform explicit cast on expression getting "
+"implicitly c
astrelni added inline comments.
Comment at: docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst:22
+prevent unintended behavior. Passing an argument of class type will result in
+compile error, even if the type is implicitly convertible to an arithmetic
type.
+
--
astrelni updated this revision to Diff 172005.
astrelni marked 3 inline comments as done.
astrelni added a comment.
Reply to comments:
- Change diagnostic message
- Update documentation
https://reviews.llvm.org/D53830
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLis
astrelni added inline comments.
Comment at: test/clang-tidy/abseil-upgrade-duration-conversions.cpp:142
+
+template void templateForOpsSpecialization(T) {}
+template <>
JonasToth wrote:
> astrelni wrote:
> > JonasToth wrote:
> > > what about non-type template pa
astrelni updated this revision to Diff 172000.
astrelni marked 2 inline comments as done.
astrelni added a comment.
Reply to review comments:
- minor code reorder
- improve test coverage
https://reviews.llvm.org/D53830
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLi
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:151-152
+ const auto *ArgExpr = Result.Nodes.getNodeAs("arg");
+ llvm::StringRef Message = "perform explicit cast on expression getting "
+"implici
JonasToth added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:167
+// likely needed.
+diag(ArgExpr->getBeginLoc(), Message);
+return;
Minor, but you can move `diag(...)` out the if condition like this:
```
auto Di
astrelni marked 3 inline comments as done.
astrelni added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:158
+ *Result.Context)
+ .empty()) {
+ diag(ArgExpr->getBeginLoc(), Message);
JonasToth wro
astrelni updated this revision to Diff 171973.
astrelni added a comment.
Combine template instantiation matching into the other matcher registration.
https://reviews.llvm.org/D53830
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/UpgradeDur
JonasToth added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:158
+ *Result.Context)
+ .empty()) {
+ diag(ArgExpr->getBeginLoc(), Message);
astrelni wrote:
> JonasToth wrote:
> > You could ellide
astrelni marked an inline comment as done.
astrelni added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:118
+AST_MATCHER_P(Expr, hasSourceRange, SourceRange, Range) {
+ return Node.getSourceRange() == Range;
+}
JonasToth wrot
astrelni updated this revision to Diff 171925.
astrelni marked 9 inline comments as done.
astrelni added a comment.
Thanks for the feedback so far.
Reply to review comments.
- Improve comments.
- Fix matcher to check for invalid source range.
- Improve test coverage for templates and macros.
h
JonasToth added a comment.
Hi astrelni,
my 2cents.
Please upload the patch will full context (i believe `diff -U 9`, but check
the man pages if that doesn't work :D)
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:32
+
+ // a *= b; a /= b;
+ Finder->ad
astrelni updated this revision to Diff 171566.
astrelni added a comment.
Reply to comments: add check for language being c++, explicitly name type in
declaration, alphabetize release notes.
https://reviews.llvm.org/D53830
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMak
Eugene.Zelenko added inline comments.
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:22
+ast_matchers::MatchFinder *Finder) {
+ // For the arithmetic calls, we match only the uses of the templated
operators
+ // where the template parameter is not a buil
26 matches
Mail list logo
