[clangd] Avoid hanging in Selection when PP corrects the token sequence.

[sammccall]

Summary

Clang's error correction can invent extra tokens, including at the "eof"
position of various FileIDs.
(I thought this only happened at the AST level - I was mistaken).

Currently this can lead to Selection looping forever unable to make progress
because the location is not in range.
Unfortunately after fixing this the selection we get may be garbage: it can
also invent tokens whose locations coincide with real ones. Nevertheless
this is better than asserting/hanging.

Fixes https://github.com/llvm/llvm-project/issues/58482

[hokein]

clang-tools-extra/clangd/Selection.cpp

316

It reminds me of this fix, the symptom is the same (triggers the Batch.empty() assertion).
With the current fix, it is no longer needed I think (maybe we should keep it, it seems an interesting special case.)

331

This is bad :( but I don't see other better alternative, I agree the current behavior is better than hanging forever.

clang-tools-extra/clangd/Selection.cpp

if (MaybeSelectedExpanded.empty() ||
       return SelectionTree::Unselected;
     }
 
     // The eof token is used as a sentinel.
     // In general, source range from an AST node should not claim the eof token,
     // but it could occur for unmatched-bracket cases.
     // FIXME: fix it in TokenBuffer, expandedTokens(SourceRange) should not
     // return the eof token.
     if (ExpandedTokens.back().kind() == tok::eof)
       ExpandedTokens = ExpandedTokens.drop_back();
 
     SelectionTree::Selection Result = NoTokens;
     while (!ExpandedTokens.empty()) {
       // Take consecutive tokens from the same context together for efficiency.
       SourceLocation Start = ExpandedTokens.front().location();
       FileID FID = SM.getFileID(Start);
       // Comparing SourceLocations against bounds is cheaper than getFileID().
       SourceLocation Limit = SM.getComposedLoc(FID, SM.getFileIDSize(FID));
       auto Batch = ExpandedTokens.take_while([&](const syntax::Token &T) {
-        return T.location() >= Start && T.location() < Limit;
+        // [Start, Limit) rather than [Start, Limit] seems correct.
+        // However PP error-correction can produce one-past-the-end locations.
+        // (The selections we produce may be dubious, but we won't hang!)
+        // See https://github.com/llvm/llvm-project/issues/58482
+        return T.location() >= Start && T.location() <= Limit;
       });
       assert(!Batch.empty());
       ExpandedTokens = ExpandedTokens.drop_front(Batch.size());
 
       update(Result, testChunk(FID, Batch));
     }
     return Result;
   }

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

#include "Expand.inc"
   auto AST = TU.build();
   EXPECT_THAT(*AST.getDiagnostics(), ::testing::IsEmpty());
   auto T = makeSelectionTree(Case, AST);
 
   EXPECT_EQ("BreakStmt", T.commonAncestor()->kind());
   EXPECT_EQ("WhileStmt", T.commonAncestor()->Parent->kind());
 }
 
+TEST(SelectionTest, MacroInitListCorrection) {
+  // This macro use is broken: the , is unparenthesized and so forms two args.
+  // clang can try to repair it, inserting extra parentheses.
+  // However these parens have dubious locations, we used to infloop here.
+  // https://github.com/llvm/llvm-project/issues/58482
+  const char *Case = R"cpp(
+    struct SS{int x,y;};
+    #define ID(X) X
+    auto s = ID(S^S{1,2}); // error-ok
+  )cpp";
+  auto AST = TestTU::withCode(Annotations(Case).code()).build();
+  auto T = makeSelectionTree(Case, AST);
+  // Ideally we would select RecordLoc, but the token sequence emitted by the
+  // preprocessor is confusing (there's an l_paren with the same loc as SS).
+  // Let's just be happy with selecting something and not crashing.
+  EXPECT_NE(nullptr, T.commonAncestor());
+}
+
 TEST(SelectionTest, IncludedFile) {
   const char *Case = R"cpp(
     void test() {
 #include "Exp^and.inc"
         break;
     }
   )cpp";
   Annotations Test(Case);