ioeric added a comment.
Thanks for the changes! The code is much clearer.
I am wondering if the current design could be extended to support tools (or
rules) that use AST matchers? Or is the selection expected to be powerful
enough to replace AST matchers?
We have a few tools in `clang-tools-extra` (e.g. `clang-move` and
`change-namespace`) and many internal tools that are based on AST matchers, but
we would really like to move them into `clang-refactor` in the future :)
================
Comment at:
include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h:26
+template <typename T>
+detail::SourceSelectionRequirement<
+ typename selection::detail::EvaluateSelectionChecker<
----------------
Could you help me understand this class?
This seems to be a selection-specific requirement and should live in
`selection`. It is also used in `BaseSpecializedRule` which seems to be a
higher level of abstraction.
================
Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:27
+
+ RefactoringResult(AtomicChange Change) : Kind(AtomicChanges) {
+ Changes.push_back(std::move(Change));
----------------
arphaman wrote:
> alexshap wrote:
> > explicit ?
> Nah, it's more convenient to be able to return a single `AtomicChanges`
> without an explicit initializer I think.
+1 to `explicit` which could prevent unintentional conversion.
`RefactoringResult(Change);` isn't too bad IMO.
================
Comment at: include/clang/Tooling/Refactoring/RefactoringResult.h:21
+struct RefactoringResult {
+ enum ResultKind {
+ /// A set of source replacements represented using a vector of
----------------
I'm a bit unsure about the abstraction of the refactoring result. I would
expected refactoring results to be source changes always. Do you have any
refactoring tool that outputs occurrences in mind?
================
Comment at: include/clang/Tooling/Refactoring/SourceSelectionConstraints.h:55
+
+namespace detail {
+
----------------
Does `detail` mean internal implementation? Maybe use `internal` which is more
commonly used for this?
Repository:
rL LLVM
https://reviews.llvm.org/D36075
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
