[PATCH] D60408: [LibTooling] Extend Transformer to support multiple simultaneous changes.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 12 02:25:12 PDT 2019


ilya-biryukov added a comment.

Sorry for the delay.
There seem to be a few changes that are unrelated to the actual patch. Could we separate various non-functional changes (moving code around, etc.) into a separate change to keep the diff for this one minimal?



================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:80
+// \endcode
+struct TextChange {
+  // The (bound) id of the node whose source will be replaced.  This id should
----------------
`MatchChange` or something similar might be a better name.
This actually tries to change the matched AST node to a textual replacement.


================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:87
+  TextGenerator Replacement;
+  TextGenerator Explanation;
+};
----------------
I would've expected explanation to be the trait of the rewrite rule, since all changes have to be applied.
What's the reasoning behind having it at the level of separate changes? How would this explanation be used? For debugging purposes or displaying that to the user?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60408





More information about the cfe-commits mailing list