[PATCH] D64329: [Clangd] Fixed SelectionTree bug for macros
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 19 02:02:53 PDT 2019
sammccall added a comment.
Just doc nits I think.
This eliminates a semi-fast path (that uses approximate token matching to avoid running the lexer in claimRange sometimes) but I think we can live without it.
================
Comment at: clang-tools-extra/clangd/Selection.cpp:242
// This is usually called from pop(), so we can take children into account.
+ // FIXME: Doesn't select the binary operator node in
+ // #define FOO(X) X + 1
----------------
I'm not sure why the FIXME is on this function, it can't be fixed here.
If you're not sure where to put it, I'd suggest putting the FIXME on the unit test that shows the wrong behavior.
================
Comment at: clang-tools-extra/clangd/Selection.cpp:261
return SelectionTree::Unselected;
// Cheap test: is there any overlap at all between the selection and range?
+ if (B.second >= SelEnd || E.second < SelBegin)
----------------
The rest of this function talks about cheap vs precise checks etc - you've removed the cheap path (which is OK to improve correctness, I think), so the comments need to be rewritten
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64329/new/
https://reviews.llvm.org/D64329
More information about the cfe-commits
mailing list