[PATCH] D35012: [refactor] Add the AST source selection component

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:68-74
+/// Traverses the given ASTContext and creates a tree of selected AST nodes.
+///
+/// \returns None if no nodes are selected in the AST, or a selected AST node
+/// that corresponds to the TranslationUnitDecl otherwise.
+Optional<SelectedASTNode> findSelectedASTNodes(const ASTContext &Context,
+                                               SourceLocation Location,
+                                               SourceRange SelectionRange);
----------------
klimek wrote:
> Any reason not to do multiple ranges?
> 
> Also, it's not clear from reading the description here why we need the Location.
I guess multiple ranges can be supported in follow-up patches to keep this one simpler.

I'll add it to the comment, but the idea is that the location corresponds to the cursor/right click position in the editor. That means that location doesn't have to be identical to the selection, since you can select something and then right click anywhere in the editor. 


================
Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:100
+      SelectionStack.back().Children.push_back(std::move(Node));
+    return true;
+  }
----------------
klimek wrote:
> Why do we always stop traversal?
False stops traversal, true is the default return value.


Repository:
  rL LLVM

https://reviews.llvm.org/D35012