[PATCH] D80606: [libTooling] Fix Transformer to work with ambient traversal kinds

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 28 06:30:25 PDT 2020


ymandel marked 3 inline comments as done.
ymandel added inline comments.


================
Comment at: clang/lib/Tooling/Transformer/RewriteRule.cpp:154
+    const RewriteRule &Rule,
+    ast_type_traits::TraversalKind DefaultTraversalKind) {
   // Map the cases into buckets of matchers -- one for each "root" AST kind,
----------------
gribozavr2 wrote:
> I don't see any callers using the new argument. Do we need this flexibility?
No. Abortive effort to support pulling it from the context. However, turned out the context is not available when this is called. Moreover, I've decided it's the wrong UX altogether. Matchers are not polymorphic (usually) -- they only make sense with respect to a single TK, whether explicit or implicit.  So, my goal is that RewriteRules will specify how "open" matchers are interpreted -- that is, the matchers TK will be set at rule creation time rather than at rule execution time (by pulling from the context).  I don't achieve that goal in this revision because I'm only updating `applyFirst`, but I should follow up with changes makeRule and the corresponding docs to make this clear.`

That said, I chose `TK_IgnoreUnlessSpelledInSource` as the default setting for "open" matchers to be consistent with the consensus that its worth trying this out as a default.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80606/new/

https://reviews.llvm.org/D80606





More information about the cfe-commits mailing list