[llvm-branch-commits] [clang-tools-extra] [clangd] check for synthesized symbols when tracking include locations (PR #75128)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangd

Author: Matheus Izvekov (mizvekov)

<details>
<summary>Changes</summary>

This fixes https://github.com/llvm/llvm-project/issues/75115

In C mode with MSVC compatibility, when the `assert` macro is defined, as a workaround, `static_assert` is implicitly defined as well, if not already so, in order to work around a broken `assert.h` implementation.
This workaround was implemented in https://github.com/llvm/llvm-project/commit/8da090381d567d0ec555840f6b2a651d2997e4b3

A synthesized symbol does not occur in source code, and therefore should not have valid source location, but this was not checked when inserting this into a `symbol -> include file` map.

The invalid FileID value is used for empty key representation in the include file hash table, so it's not valid to insert it.

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


2 Files Affected:

- (modified) clang-tools-extra/clangd/index/SymbolCollector.cpp (+5-1) 
- (modified) clang-tools-extra/clangd/test/GH75115.test (+7-4) 


``````````diff
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index aac6676a995fe..e9e76abb90a2d 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -821,7 +821,11 @@ void SymbolCollector::setIncludeLocation(const Symbol &S, SourceLocation DefLoc,
 
   // Use the expansion location to get the #include header since this is
   // where the symbol is exposed.
-  IncludeFiles[S.ID] = SM.getDecomposedExpansionLoc(DefLoc).first;
+  FileID FID = SM.getDecomposedExpansionLoc(DefLoc).first;
+  if (FID.isInvalid())
+    return;
+
+  IncludeFiles[S.ID] = FID;
 
   // We update providers for a symbol with each occurence, as SymbolCollector
   // might run while parsing, rather than at the end of a translation unit.
diff --git a/clang-tools-extra/clangd/test/GH75115.test b/clang-tools-extra/clangd/test/GH75115.test
index 030392f1d69b3..dc86f5b9a6cee 100644
--- a/clang-tools-extra/clangd/test/GH75115.test
+++ b/clang-tools-extra/clangd/test/GH75115.test
@@ -1,12 +1,15 @@
 // RUN: rm -rf %t.dir && mkdir -p %t.dir
 // RUN: echo '[{"directory": "%/t.dir", "command": "clang --target=x86_64-pc-windows-msvc -x c GH75115.test", "file": "GH75115.test"}]' > %t.dir/compile_commands.json
-// RUN: not --crash clangd -enable-config=0 --compile-commands-dir=%t.dir -check=%s 2>&1 | FileCheck -strict-whitespace %s
-
-// FIXME: Crashes
+// RUN: clangd -enable-config=0 --compile-commands-dir=%t.dir -check=%s 2>&1 | FileCheck -strict-whitespace %s
 
 // CHECK: Building preamble...
 // CHECK-NEXT: Built preamble
 // CHECK-NEXT: Indexing headers...
-// CHECK-NEXT: !KeyInfoT::isEqual(Val, EmptyKey) && !KeyInfoT::isEqual(Val, TombstoneKey) && "Empty/Tombstone value shouldn't be inserted into map!"
+// CHECK-NEXT: Building AST...
+// CHECK-NEXT: Indexing AST...
+// CHECK-NEXT: Building inlay hints
+// CHECK-NEXT: semantic highlighting
+// CHECK-NEXT: Testing features at each token
+// CHECK-NEXT: All checks completed, 0 errors
 
 #define assert

``````````

</details>


https://github.com/llvm/llvm-project/pull/75128