hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land.
LGTM. ================ Comment at: unittests/Tooling/ASTSelectionTest.cpp:688 + Source, {2, 2}, None, + [](SourceRange SelectionRange, Optional<SelectedASTNode> Node) { + EXPECT_TRUE(Node); ---------------- arphaman wrote: > 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. I see, thanks. > the AST selection will work even with a cursor location, hence it won't be > empty. Is this documented somewhere in the code? I couldn't find any comments in the definition of `findSelectedASTNodesWithRange` or `findSelectedASTNodes` in this file. Would be nice to document it although this is a test API, so that it won't confuse future code readers. 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