ilya-biryukov added a comment.
Looks neat! The only concern I have is about the growing number of overloads in
`Transformer.h`, see the relevant comment.
================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:186
+/// where a \c TextGenerator, \c RangeSelector are otherwise expected.
+inline ASTEdit change(RangeSelector Target, std::string Replacement) {
+ return change(std::move(Target), text(std::move(Replacement)));
----------------
The overloads create quite a lot of noise (and we are already starting to see a
combinatorial explosion with two classes).
Could we try to overcome this? I see two options:
1. Turn `TextGenerator` and `RangeSelector` into classes, which are
constructible from `string` or `function<string(MatchResult)>`.
2. Remove the string overloads, force users to explicitly construct the
parameters.
Option (2) actually is simple and would add a bit of type safety, but will make
some client code a bit less readable. Do you think the readability hit is
significant?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62149/new/
https://reviews.llvm.org/D62149
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
