hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Thanks, the patch looks good! please also update the patch description.
================
Comment at: clang-tools-extra/clangd/Selection.cpp:63
+ // Claims whichever expanded tokens in SourceRange are not yet claimed.
+ // Writes contiguous claimed subranges to Result, and returns true if any.
+ llvm::SmallVector<llvm::ArrayRef<T>, 4> erase(llvm::ArrayRef<T> Claim) {
----------------
nit: this comment seems not reflect to the code now.
================
Comment at: clang-tools-extra/clangd/Selection.cpp:126
+
+// Nodes start start with NoTokens, and then use this function to aggregate
+// the selectedness as more tokens are found.
----------------
nit: remove the redundant start.
================
Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:468
+ // Unfortunately, this makes the common ancestor the CallExpr...
+ // FIXME: hack around this by picking one?
+ EXPECT_EQ("CallExpr", T.commonAncestor()->kind());
----------------
I think cases like below are broken:
```
#define greater(x, y) x > y? x : y
#define abs(x) x > 0 ? x : -x
```
Selecting the first element for a macro arg seems good to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70512/new/
https://reviews.llvm.org/D70512
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
