[clangd] Fix is spelled in source bug

[schenka0]

This fixes the issue reported in #76667 and adds an initial unit test for isSpelledInSource().

Note that in that issue there was still some underlying corrupted AST, but this at least makes isSpelledInSource() robust to it.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-mc
@llvm/pr-subscribers-mlir-llvm
@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clangd

Author: None (schenka0)

Changes

This fixes the issue reported in #76667 and adds an initial unit test for isSpelledInSource().

Note that in that issue there was still some underlying corrupted AST, but this at least makes isSpelledInSource() robust to it.

---

Full diff: https://github.com/llvm/llvm-project/pull/76668.diff

2 Files Affected:

• (modified) clang-tools-extra/clangd/SourceCode.cpp (+6-1)
• (modified) clang-tools-extra/clangd/unittests/SourceCodeTests.cpp (+15)

diff --git a/clang-tools-extra/clangd/SourceCode.cpp b/clang-tools-extra/clangd/SourceCode.cpp
index 835038423fdf37..8c573cc6fc064a 100644
--- a/clang-tools-extra/clangd/SourceCode.cpp
+++ b/clang-tools-extra/clangd/SourceCode.cpp
@@ -232,7 +232,12 @@ bool isSpelledInSource(SourceLocation Loc, const SourceManager &SM) {
   if (Loc.isFileID())
     return true;
   auto Spelling = SM.getDecomposedSpellingLoc(Loc);
-  StringRef SpellingFile = SM.getSLocEntry(Spelling.first).getFile().getName();
+  bool InvalidSLocEntry = false;
+  const auto SLocEntry = SM.getSLocEntry(Spelling.first, &InvalidSLocEntry);
+  if (InvalidSLocEntry) {
+    return false;
+  }
+  const StringRef SpellingFile = SLocEntry.getFile().getName();
   if (SpellingFile == "<scratch space>")
     return false;
   if (SpellingFile == "<built-in>")
diff --git a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
index 08abde87df6d4d..5dced4d317c605 100644
--- a/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -813,6 +813,21 @@ TEST(SourceCodeTests, isKeywords) {
   EXPECT_FALSE(isKeyword("override", LangOpts));
 }
 
+TEST(SourceCodeTests, isSpelledInSource) {
+  Annotations Test(R"cpp(
+        int abc = 1;
+    )cpp");
+
+  ParsedAST AST = TestTU::withCode(Test.code()).build();
+  llvm::errs() << Test.code();
+  const SourceManager &SM = AST.getSourceManager();
+
+  EXPECT_TRUE(
+      isSpelledInSource(SM.getLocForStartOfFile(SM.getMainFileID()), SM));
+  EXPECT_TRUE(isSpelledInSource(SourceLocation(), SM));
+  EXPECT_FALSE(isSpelledInSource(SourceLocation::getFromRawEncoding(-1), SM));
+}
+
 struct IncrementalTestStep {
   llvm::StringRef Src;
   llvm::StringRef Contents;

[schenka0]

PR: @sam-mccall

[zyn0217]

Thanks for your contribution! A couple of nit-picks, and I’d leave the approval to other folks.

[schenka0]

I struggled to rebase my changes around some merges and accidently included additional changes at one point which triggered the llvmbot to add extra tags. Sorry!

[HighCommander4]

Thanks, looks good with the added comments.

[schenka0]

@HighCommander4 Thanks for the review. I've updated the comments, but don't have merge access, could you merge this? Thanks

[HighCommander4]

Thanks!

[HighCommander4]

@HighCommander4 Thanks for the review. I've updated the comments, but don't have merge access, could you merge this? Thanks

Done

[zyn0217]

(A drive-by comment: I stumbled on the commit message of this PR and found it surprising that I'm the co-author of it! I'm not entirely clear on GitHub's schemas, but IMO shouldn't the co-author be the one who clicks the "merge" button?)

[HighCommander4]

(A drive-by comment: I stumbled on the commit message of this PR and found it surprising that I'm the co-author of it! I'm not entirely clear on GitHub's schemas, but IMO shouldn't the co-author be the one who clicks the "merge" button?)

The reason is that one of the four commits that were squashed had you as a co-author:

(I'm not sure how that happened in the first place.)

[schenka0]

(I'm not sure how that happened in the first place.)

I believe you suggested a (minor) change and I just clicked commit suggestion:

Didn't realize it would entangle you in the commit, especially once squashed.