[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Looks very polished, thanks!
Will have to sink the change in a bit, will come back with more comments on Monday.
In the meantime, a few initial comments and suggestions.



================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:54
+/// boolean expression language for constructing filters.
+class MatchFilter {
+public:
----------------
Intuitively, it feels that any filtering should be possible at the level of the AST matchers. Is that not the case?
Could you provide some motivating examples where AST matchers cannot be used to nail down the matching nodes and we need `MatchFilter`? 

Please note I have limited experience with AST matchers, so there might be some obvious things that I'm missing.


================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:135
+// \endcode
+class RewriteRule {
+public:
----------------
Maybe consider separating the fluent API to build the rewrite rule from the rewrite rule itself?

Not opposed to having the fluent builder API, this does look nice and seems to nicely align with the matcher APIs.
However, it is somewhat hard to figure out what can `RewriteRule` do **after** it was built when looking at the code right now.
```
class RewriteRule {
public:
  RewriteRule(DynTypedMatcher, TextGenerator Replacement, TextGenerator Explanation);

  DynTypedMatcher matcher() const;
  Expected<string> replacement() const;
  Expected<string> explanation() const;
};

struct RewriteRuleBuilder { // Having a better name than 'Builder' would be nice.
  RewriteRule finish() &&; // produce the final RewriteRule.

  template <typename T>
  RewriteRuleBuilder &change(const TypedNodeId<T> &Target,
                      NodePart Part = NodePart::Node) &;
  RewriteRuleBuilder &replaceWith(TextGenerator Replacement) &;
  RewriteRuleBuilder &because(TextGenerator Explanation) &;
private:
  RewriteRule RuleInProgress;
};
RewriteRuleBuilder makeRewriteRule();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376