arphaman added inline comments.
================
Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:74
+/// An AST selection value that corresponds to a selection of a set of
+/// statements that belong to one body of code (like one function).
+///
----------------
hokein wrote:
> I see all your tests are for function body, does it support other body, i.e.
> "global namespace", "compound statement"?
>
> if yes, how about adding more test cases to cover it.
>
> ```
> // variable in global namespace
> int a; // select int.
> ```
>
> ```
> void f() {
> {
> int a; // select a.
> }
> }
> ```
Yes, I added a couple that you suggested.
================
Comment at: unittests/Tooling/ASTSelectionTest.cpp:688
+ Source, {2, 2}, None,
+ [](SourceRange SelectionRange, Optional<SelectedASTNode> Node) {
+ EXPECT_TRUE(Node);
----------------
hokein wrote:
> I'm a bit confused here, if the selection range is none/empty, shouldn't
> `SelectedASTNode` be empty?
>
> If this behavior is intended, I'd suggest documenting it in
> `findSelectedASTNodesWithRange`.
No, the AST selection will work even with a cursor location, hence it won't be
empty.
However, the CodeRangeASTSelection requires a non-empty selection range, so it
won't work with just cursors.
Repository:
rL LLVM
https://reviews.llvm.org/D38835
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
