kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
================
Comment at: clang-tools-extra/clangd/Selection.cpp:228
+ Buf.expansionsAffecting(Sel)) {
+ if (X.Expanded.empty()) {
+ for (const syntax::Token &Tok : X.Spelled) {
----------------
nit: reduce nesting via
```
if(!X.Expanded.empty())
continue
```
================
Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:554
+TEST(SelectionTest, DisabledPreprocessor) {
+ const char *Case = R"cpp(
+ namespace ns {
----------------
this case isn't disabled PP, moreover seems to be already tested in
CommonAncestor, see:
```
{
R"cpp(
void foo();
#define CALL_FUNCTION(X) X^()^
void bar() { CALL_FUNCTION(foo); }
)cpp",
nullptr,
},
```
maybe also put a couple of tests for the macroname and the directive itself?
(or just extend the test above to whole directive line?)
================
Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:568
+ #if 0
+ void fu^nc();
+ #endif
----------------
nit: i am not sure if this is worth it's own test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84012/new/
https://reviews.llvm.org/D84012
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
