[PATCH] D113917: Add infrastructure to support matcher names.

2021-12-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D113917#3193374 , @ymandel wrote: > Aaron - agreed on the points about `StringRef` vs `std::string`. But, before > I make that change, what did you think of moving to a more general method > `getMatcherSpec` that return

[PATCH] D113917: Add infrastructure to support matcher names.

2021-12-14 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Aaron - agreed on the points about `StringRef` vs `std::string`. But, before I make that change, what did you think of moving to a more general method `getMatcherSpec` that returns an object? That object will provide the name (for now) and other data in the future (like

[PATCH] D113917: Add infrastructure to support matcher names.

2021-12-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:88 +/// can be useful for cases like debugging matchers. +template std::string makeMatcherNameFromType() { + return "Matcher"; ymandel wrote: > Please note in

[PATCH] D113917: Add infrastructure to support matcher names.

2021-12-03 Thread James King via Phabricator via cfe-commits
jcking1034 added a comment. @hokein @aaron.ballman Following up on this, let me know if there are any other action items to address! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113917/new/ https://reviews.llvm.org/D113917 __

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-24 Thread James King via Phabricator via cfe-commits
jcking1034 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:152 + } +MAKE_MATCHER_NAME_FROM_TYPE(TemplateArgument) +MAKE_MATCHER_NAME_FROM_TYPE(TemplateArgumentLoc) hokein wrote: > These are types that are not covered in t

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-24 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 389620. jcking1034 marked 3 inline comments as done. jcking1034 added a comment. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-23 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. The approach looks fine in general, just some nits when reading through the patch. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:152 + } +MAKE_MATCHER_NAME_FROM_TYPE(TemplateArgument) +MAKE_MATCHER_NAME_FROM_TYPE(TemplateArgumentLoc

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-18 Thread James King via Phabricator via cfe-commits
jcking1034 marked an inline comment as done. jcking1034 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1022 + std::string getName() const override { +return "HasOverloadedOperatorNameMatcher"; + } ymandel wrote: > h

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-18 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 388327. jcking1034 marked 2 inline comments as done. jcking1034 added a comment. Update comments, drop "Matcher" suffix from `getName`s Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113917/new/ https://revie

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-18 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Looks good, just small comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:88 +/// can be useful for cases like debugging matchers. +template std::string makeMatcherNameFromType() { + return "Matcher"; Please n

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-17 Thread James King via Phabricator via cfe-commits
jcking1034 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1409 return BindableMatcher( - makeAllOfComposite(InnerMatchers).template dynCastTo()); + makeAllOfComposite(InnerMatchers).template dynCastTo(), + makeMatcherNam

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-17 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 388041. jcking1034 added a comment. Add tests, rename `getMatcherName` to `getName`, update `makeAllOfComposite` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113917/new/ https://reviews.llvm.org/D113917 Fi

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. For a little more context: we're working to improve the debuggability of matchers. The first step in that direction seems to be a change of this sort that provides some (dynamic) introspection so that a tool, library, etc. can see some basic information about the matche

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-15 Thread James King via Phabricator via cfe-commits
jcking1034 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1021 + std::string getName() const override { +return "HasOverloadedOperatorNameMatcher"; Here and below, we have the option to use the name of the matcher

[PATCH] D113917: Add infrastructure to support matcher names.

2021-11-15 Thread James King via Phabricator via cfe-commits
jcking1034 created this revision. jcking1034 added reviewers: ymandel, tdl-g, aaron.ballman. jcking1034 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This contains changes to the AST Matcher infrastructure in order to provide contextual i