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 wrote:
> What happens on invalid ranges? Are they considered equal, or is it forbidden
> to pass them in?
Good point. I think this is a non-issue with the way the code is set up now,
but better make it explicitly work :). We only care about valid source ranges.
I've updated the name of the matcher and added a check here for the Node's
source range and another in the matcher below to return false early.
================
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:123
+ auto HasMatchingDependentDescendant = hasDescendant(
+ expr(hasSourceRange(Node.getSourceRange()), isInstantiationDependent()));
+ return expr(anyOf(hasAncestor(
----------------
JonasToth wrote:
> Please add a test-case where the `Node.getSourceRange()` is a macro, if not
> already existing. I believe that is currently missing.
Yes, thank you, that was missing. Added a few macro + template interactions.
================
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:158
+ *Result.Context)
+ .empty()) {
+ diag(ArgExpr->getBeginLoc(), Message);
----------------
JonasToth wrote:
> You could ellide these braces, but I feel that this matching code be merged
> into one matcher with `equalsBoundNode()` (see ASTMatcher reference).
Started with removing braces.
Sorry I had a look at `equalsBoundNode()`, but couldn't see exactly what you
meant. Could you please elaborate about the merging?
================
Comment at: test/clang-tidy/abseil-upgrade-duration-conversions.cpp:142
+
+template <typename T> void templateForOpsSpecialization(T) {}
+template <>
----------------
JonasToth wrote:
> what about non-type template parameters? Do they need consideration for the
> check as well?
> If i am not mistaken floats are allowed in newwer standards as well.
IIUC non-type template parameters should be no different for this check. This
particular case is to make sure explicit specializations are treated
differently from instantiations and get fix-it hints.
https://reviews.llvm.org/D53830
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
