ymandel added a comment.
In D60408#1463909 <https://reviews.llvm.org/D60408#1463909>, @ilya-biryukov
wrote:
> 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?
I've done that as far as I can tell. Please let me know if I've missed anything.
================
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
----------------
ilya-biryukov wrote:
> `MatchChange` or something similar might be a better name.
> This actually tries to change the matched AST node to a textual replacement.
I chose this to contrast with an AST change. The idea being that we're
specifying a replacement relative to source code locations (informed by the
ast). If we later, say, integrate with your library I could imagine specifying
changes to AST nodes. But, maybe I'm overthinking... If we're going to drop
"text", what about "source?" be clearer than "text"? E.g, `SourceChange` or (my
preference) `SourceEdit`?
================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:87
+ TextGenerator Replacement;
+ TextGenerator Explanation;
+};
----------------
ilya-biryukov wrote:
> 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?
I think that for most cases, one explanation sufficies for the whole
transformation. However, there are some tidies that emit multiple diagnoses
(for example if changing before a declaration and a definition). Would it
help if I clarify in the comments?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60408/new/
https://reviews.llvm.org/D60408
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
