ioeric added a comment.
Some nits.
================
Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:23
+/// of the cursor or overlap with the selection range.
+class ASTSelectionFinder : public RecursiveASTVisitor<ASTSelectionFinder> {
+ const SourceLocation Location;
----------------
In LLVM, we use the following class layout:
```
class X {
public:
void f();
private:
void g();
int member;
}
```
================
Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:76
+
+ Optional<SelectedASTNode> getResult() {
+ assert(SelectionStack.size() == 1 && "stack was not popped");
----------------
I think `getSelectedASTNode` would be clearer.
================
Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:90
+ return RecursiveASTVisitor::TraverseDecl(D);
+ // TODO (Alex): Add location adjustment for ObjCImplDecls.
+ SourceSelectionKind SelectionKind =
----------------
Use FIXME(<user-id>) instead of TODO in LLVM. Here and elsewhere.
================
Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:126-129
+ SelectedASTNode Node = std::move(SelectionStack.back());
+ SelectionStack.pop_back();
+ if (SelectionKind != SourceSelectionKind::None || !Node.Children.empty())
+ SelectionStack.back().Children.push_back(std::move(Node));
----------------
This seems to be a recurring pattern. Maybe pull into a function?
Repository:
rL LLVM
https://reviews.llvm.org/D35012
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
