[clangd] Exclude preprocessed-to-nothing tokens from selection

[sammccall]

Summary

This prevents selection of empty preprocessor entities (like #define directives,or text in disabled sections) creating a selection in the parent element.

Based on D83508 by Aleksandr Platonov.

[kadircet]

LGTM, thanks!

clang-tools-extra/clangd/Selection.cpp

228

nit: reduce nesting via

if(!X.Expanded.empty())
  continue

clang-tools-extra/clangd/unittests/SelectionTests.cpp

564

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?)

578

nit: i am not sure if this is worth it's own test

[sammccall]

address comments

[sammccall]

clang-tools-extra/clangd/unittests/SelectionTests.cpp

578

Moved into CommonAncestor tests. (We do need to test it somewhere, that's the point of this patch)

[kadircet]

clang-tools-extra/clangd/unittests/SelectionTests.cpp

578

sorry I was saying that having a separate TEST fixture wasn't worth it, not the test itself :D

clang-tools-extra/clangd/Selection.cpp

SelectionTester(const syntax::TokenBuffer &Buf, FileID SelFile,
     const syntax::Token *SelFirst =
         llvm::partition_point(AllSpelledTokens, [&](const syntax::Token &Tok) {
           return SM.getFileOffset(Tok.endLocation()) <= SelBegin;
         });
     const syntax::Token *SelLimit = std::partition_point(
         SelFirst, AllSpelledTokens.end(), [&](const syntax::Token &Tok) {
           return SM.getFileOffset(Tok.location()) < SelEnd;
         });
+    auto Sel = llvm::makeArrayRef(SelFirst, SelLimit);
+    // Find which of these are preprocessed to nothing and should be ignored.
+    std::vector<bool> PPIgnored(Sel.size(), false);
+    for (const syntax::TokenBuffer::Expansion &X :
+         Buf.expansionsAffecting(Sel)) {
+      if (X.Expanded.empty()) {
+        for (const syntax::Token &Tok : X.Spelled) {
+          if (&Tok >= SelFirst && &Tok < SelLimit)
+            PPIgnored[&Tok - SelFirst] = true;
+        }
+      }
+    }
     // Precompute selectedness and offset for selected spelled tokens.
-    for (const syntax::Token *T = SelFirst; T < SelLimit; ++T) {
-      if (shouldIgnore(*T))
+    for (unsigned I = 0; I < Sel.size(); ++I) {
+      if (shouldIgnore(Sel[I]) || PPIgnored[I])
         continue;
       SpelledTokens.emplace_back();
       Tok &S = SpelledTokens.back();
-      S.Offset = SM.getFileOffset(T->location());
-      if (S.Offset >= SelBegin && S.Offset + T->length() <= SelEnd)
+      S.Offset = SM.getFileOffset(Sel[I].location());
+      if (S.Offset >= SelBegin && S.Offset + Sel[I].length() <= SelEnd)
         S.Selected = SelectionTree::Complete;
       else
         S.Selected = SelectionTree::Partial;
     }
   }
 
   // Test whether a consecutive range of tokens is selected.
   // The tokens are taken from the expanded token stream.

clang-tools-extra/clangd/unittests/SelectionTests.cpp

Case Cases[] = {
             #define CALL_FUNCTION(X) X()
             void bar() { [[C^ALL_FUNC^TION(foo)]]; }
           )cpp",
           "CallExpr",
       },
       {
           R"cpp(
             void foo();
-            #define CALL_FUNCTION(X) X^()^
+            #^define CALL_FUNCTION(X) X(^)
             void bar() { CALL_FUNCTION(foo); }
           )cpp",
           nullptr,
       },
       {
           R"cpp(
+            void foo();
+            #define CALL_FUNCTION(X) X()
+            void bar() { CALL_FUNCTION(foo^)^; }
+          )cpp",
+          nullptr,
+      },
+      {
+          R"cpp(
+            namespace ns {
+            #if 0
+            void fo^o() {}
+            #endif
+            }
+          )cpp",
+          nullptr,
+      },
+      {
+          R"cpp(
             struct S { S(const char*); };
             S [[s ^= "foo"]];
           )cpp",
           "CXXConstructExpr",
       },
       {
           R"cpp(
             struct S { S(const char*); };
Case Cases[] = {
       {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
 
       {R"cpp(
         struct S1 { void f(); };
         struct S2 { S1 * operator->(); };
         void test(S2 s2) {
           s2[[-^>]]f();
         }
-      )cpp", "DeclRefExpr"} // DeclRefExpr to the "operator->" method.
+      )cpp",
+       "DeclRefExpr"} // DeclRefExpr to the "operator->" method.
   };
   for (const Case &C : Cases) {
     trace::TestTracer Tracer;
     Annotations Test(C.Code);
 
     TestTU TU;
     TU.Code = std::string(Test.code());
 
#include "Exp^and.inc"
     }
   )cpp";
   Annotations Test(Case);
   auto TU = TestTU::withCode(Test.code());
   TU.AdditionalFiles["Expand.inc"] = "while(1)\n";
   auto AST = TU.build();
   auto T = makeSelectionTree(Case, AST);
 
-  EXPECT_EQ("WhileStmt", T.commonAncestor()->kind());
+  EXPECT_EQ(nullptr, T.commonAncestor());
 }
 
 TEST(SelectionTest, MacroArgExpansion) {
   // If a macro arg is expanded several times, we only consider the first one
   // selected.
   const char *Case = R"cpp(
     int mul(int, int);
     #define SQUARE(X) mul(X, X);
     int nine = SQUARE(^3);
   )cpp";
   Annotations Test(Case);
   auto AST = TestTU::withCode(Test.code()).build();
   auto T = makeSelectionTree(Case, AST);
   EXPECT_EQ("IntegerLiteral", T.commonAncestor()->kind());
   EXPECT_TRUE(T.commonAncestor()->Selected);
 
   // Verify that the common assert() macro doesn't suffer from this.
   // (This is because we don't associate the stringified token with the arg).
   Case = R"cpp(
     void die(const char*);
     #define assert(x) (x ? (void)0 : die(#x))
     void foo() { assert(^42); }
   )cpp";
   Test = Annotations(Case);
   AST = TestTU::withCode(Test.code()).build();
   T = makeSelectionTree(Case, AST);