adamcz created this revision.
adamcz added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous,
MaskRay, ilya-biryukov.
Herald added a project: clang.
When preamble contains #undef, indexing code finds the matching #define
and uses that during indexing. However, it would only look for local
definitions. If the macro was defined in a module, MacroInfo
would be nullptr and clangd would crash.
This change fixes the crash by looking for definition in module when
that happens. The indexing result is then exactly the same whether
modules are used or not.
The indexing of macros happens for preamble only, so then #undef must be
in the preamble, which is why we need two .h files in a test.
Note that clangd is currently not ready for module support, but this
brings us one step closer.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80525
Files:
clang-tools-extra/clangd/test/Inputs/symbols-modules/bar.h
clang-tools-extra/clangd/test/Inputs/symbols-modules/compile_commands.json
clang-tools-extra/clangd/test/Inputs/symbols-modules/foo.h
clang-tools-extra/clangd/test/Inputs/symbols-modules/module.map
clang-tools-extra/clangd/test/symbols-modules.test
clang/include/clang/Lex/Preprocessor.h
clang/lib/Index/IndexingAction.cpp
Index: clang/lib/Index/IndexingAction.cpp
===================================================================
--- clang/lib/Index/IndexingAction.cpp
+++ clang/lib/Index/IndexingAction.cpp
@@ -147,14 +147,23 @@
Unit.visitLocalTopLevelDecls(&IndexCtx, topLevelDeclVisitor);
}
-static void indexPreprocessorMacros(const Preprocessor &PP,
+static void indexPreprocessorMacros(Preprocessor &PP,
IndexDataConsumer &DataConsumer) {
- for (const auto &M : PP.macros())
- if (MacroDirective *MD = M.second.getLatest())
+ for (const auto &M : PP.macros()) {
+ if (MacroDirective *MD = M.second.getLatest()) {
+ auto *MI = MD->getMacroInfo();
+ if (!MI) {
+ // It is possible for MI to be nullptr if we our translation unit had
+ // only #undef of a macro defined in a module. In that case, dig deeper
+ // and use the definition from the module to yield the same indexing
+ // result whether we are using modules or not.
+ MI = PP.getMacroInfoForLatestDefinition(M.first);
+ }
DataConsumer.handleMacroOccurrence(
- M.first, MD->getMacroInfo(),
- static_cast<unsigned>(index::SymbolRole::Definition),
+ M.first, MI, static_cast<unsigned>(index::SymbolRole::Definition),
MD->getLocation());
+ }
+ }
}
void index::indexASTUnit(ASTUnit &Unit, IndexDataConsumer &DataConsumer,
Index: clang/include/clang/Lex/Preprocessor.h
===================================================================
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -1115,6 +1115,30 @@
return nullptr;
}
+ // Returns MacroInfo for the latest definition of II. It could possibly be
+ // imported from a module (thus not accesible via getMacroInfo()), whether
+ // it's active or not.
+ MacroInfo *getMacroInfoForLatestDefinition(const IdentifierInfo *II) {
+ if (!II->hasMacroDefinition())
+ return nullptr;
+ if (auto *MI = getMacroInfo(II))
+ return MI;
+ MacroState &S = CurSubmoduleState->Macros[II];
+ auto ActiveModuleMacros = S.getActiveModuleMacros(*this, II);
+ for (auto MM = ActiveModuleMacros.rbegin(); MM != ActiveModuleMacros.rend();
+ MM++) {
+ if (auto *MI = (*MM)->getMacroInfo())
+ return MI;
+ }
+ auto OverridenMacros = S.getOverriddenMacros();
+ for (auto MM = OverridenMacros.rbegin(); MM != OverridenMacros.rend();
+ MM++) {
+ if (auto *MI = (*MM)->getMacroInfo())
+ return MI;
+ }
+ return nullptr;
+ }
+
/// Given an identifier, return the latest non-imported macro
/// directive for that identifier.
///
Index: clang-tools-extra/clangd/test/symbols-modules.test
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/symbols-modules.test
@@ -0,0 +1,44 @@
+# Verify that we do not crash and correctly find the definition of a macro that
+# was imported from a module and then #undef-ed in preamble.
+#
+# Copy over the test files and fix the paths.
+# RUN: rm -rf %t
+# RUN: cp -r %S/Inputs/symbols-modules %t
+# RUN: sed -i -e "s|DIRECTORY|%t|" %t/compile_commands.json
+#
+# Fix the paths in this file as well. We will pass this copy to clangd.
+# RUN: sed -e "s|DIRECTORY|%/t|g" %s > %t.test.1
+#
+# RUN: clangd -lit-test < %t.test.1 | FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"symbol":{"symbolKind":{"valueSet": [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://DIRECTORY/foo.cpp","languageId":"cpp","version":1,"text":"#include \"bar.h\"\nvoid foo();\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"workspace/symbol","params":{"query":"X"}}
+# CHECK: "id": 1,
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "containerName": "",
+# CHECK-NEXT: "kind": 15,
+# CHECK-NEXT: "location": {
+# CHECK-NEXT: "range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT: "character": {{.*}},
+# CHECK-NEXT: "line": {{.*}}
+# CHECK-NEXT: },
+# CHECK-NEXT: "start": {
+# CHECK-NEXT: "character": {{.*}},
+# CHECK-NEXT: "line": {{.*}}
+# CHECK-NEXT: }
+# CHECK-NEXT: },
+# CHECK-NEXT: "uri": "file://{{.*}}/foo.h"
+# CHECK-NEXT: },
+# CHECK-NEXT: "name": "X"
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+# CHECK-NEXT:}
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/test/Inputs/symbols-modules/module.map
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/symbols-modules/module.map
@@ -0,0 +1,4 @@
+module foo {
+ header "foo.h"
+ export *
+}
Index: clang-tools-extra/clangd/test/Inputs/symbols-modules/foo.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/symbols-modules/foo.h
@@ -0,0 +1 @@
+#define X 1
Index: clang-tools-extra/clangd/test/Inputs/symbols-modules/compile_commands.json
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/symbols-modules/compile_commands.json
@@ -0,0 +1,5 @@
+[{
+ "directory": "DIRECTORY",
+ "command": "clang -fmodules -Xclang -fmodule-map-file-home-is-cwd -fmodule-map-file=module.map foo.cpp",
+ "file": "DIRECTORY/foo.cpp"
+}]
Index: clang-tools-extra/clangd/test/Inputs/symbols-modules/bar.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clangd/test/Inputs/symbols-modules/bar.h
@@ -0,0 +1,3 @@
+#include "foo.h"
+
+#undef X
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
