aaron.ballman added inline comments.
================
Comment at: clang/include/clang/ASTMatchers/ASTMatchFinder.h:118
+ /// behavior of clang-tidy.
+ virtual llvm::Optional<ast_type_traits::TraversalKind>
+ getCheckTraversalKind() const;
----------------
steveire wrote:
> sammccall wrote:
> > I don't really get why this would be optional.
> > A check's implementation isn't really going to be robust to "whatever the
> > default is", it's going to be tested against one or the other.
> > So None isn't really a sensible return value - can't the base class simply
> > return the actual default?
> ASTMatchFinder doesn't know the ASTContext, so it can't access the default.
> That's why I made it an optional.
The fact that it's optional is a bit weird. Given that this is a temporary API,
I see two approaches: hold our nose because the ugliness shouldn't be
long-lived, or have the default be specified in two places with comments saying
"don't forget to update over here if you change this value" so we can get rid
of the `Optional<>` here. WDYT?
================
Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:1205
MatchCallback *Action) {
- Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
+ if (auto TK = Action->getCheckTraversalKind()) {
+ Matchers.DeclOrStmt.emplace_back(traverse(*TK, NodeMatch), Action);
----------------
Elide the braces.
================
Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:1221
MatchCallback *Action) {
- Matchers.DeclOrStmt.emplace_back(NodeMatch, Action);
+ if (auto TK = Action->getCheckTraversalKind()) {
+ Matchers.DeclOrStmt.emplace_back(traverse(*TK, NodeMatch), Action);
----------------
Elide the braces.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80623/new/
https://reviews.llvm.org/D80623
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
