alexfh added inline comments.
================
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:33-36
+ anyOf(hasAncestor(
+ functionTemplateDecl(HasMatchingDependentDescendant)),
+ hasAncestor(
+ classTemplateDecl(HasMatchingDependentDescendant))))
----------------
The hasAncestor and hasDescendant matchers frequently have non-trivial
performance implications. Especially the latter, especially when called on a
large number of nodes with a large number of transitive children (this
obviously depends on the code being analyzed). I don't know how likely is this
to cause problems here (the matcher seems to only be used when the check is
about to issue a diagnostic), but it's always good to be aware of the possible
issues.
Frequently there's a more efficient (and easier to understand) alternative. For
example, instead of trying to evaluate certain conditions while traversing the
code (which may require a large number of lookups into relatively distant parts
of the AST), it's sometimes more efficient to split the analysis into two or
more stages. During the AST traversal (i.e. when AST matchers run) the check
could collect some raw information about all potentially problematic places in
the code and then (for example, in `onEndOfTranslationUnit`) analyze the
collected information together in a second stage. This works best if there's a
way to arrange the data gathered on the first pass such that the second pass
can efficiently look up necessary information.
================
Comment at: clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp:136-141
+ callee(functionDecl(anyOf(hasName("::absl::Nanoseconds"),
+ hasName("::absl::Microseconds"),
+ hasName("::absl::Milliseconds"),
+ hasName("::absl::Seconds"),
+ hasName("::absl::Minutes"),
+ hasName("::absl::Hours")),
----------------
Use hasAnyName instead.
https://reviews.llvm.org/D53830
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
