llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangd
Author: R. Aoto (aotodev)
<details>
<summary>Changes</summary>
SymbolCollector::shouldCollectSymbol only accepts symbols whose semantic parent
is a translation unit, namespace, linkage spec, enum, record or one of a few
ObjC containers. ExportDecl is a DeclContext and is returned by
getDeclContext(), so every entity declared with `export` or inside an `export {
... }` block was dropped from the index. Members of exported classes were still
indexed, since their parent is the record.
In C++20 modules code this hides most of the exported interface from the index:
Go to Definition on an exported function stops at the declaration in the module
interface instead of reaching the definition in the implementation unit, and
workspace/symbol does not find it. Rename uses the same check, so it rejects
exported symbols as "not eligible for indexing" (reported in clangd/clangd#<!--
-->2569).
An export-declaration can only appear at namespace scope ([module.interface]),
so treat Decl::Export like the other namespace-level contexts. This also
removes the FIXME describing the gap.
---
Full diff: https://github.com/llvm/llvm-project/pull/234755.diff
3 Files Affected:
- (modified) clang-tools-extra/clangd/index/SymbolCollector.cpp (+2-2)
- (modified) clang-tools-extra/clangd/unittests/RenameTests.cpp (+40)
- (modified) clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp (+22)
``````````diff
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index e53bd3265d96c4..ae753373635bcd 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -524,13 +524,13 @@ bool SymbolCollector::shouldCollectSymbol(const NamedDecl
&ND,
// We want most things but not "local" symbols such as symbols inside
// FunctionDecl, BlockDecl, ObjCMethodDecl and OMPDeclareReductionDecl.
- // FIXME: Need a matcher for ExportDecl in order to include symbols declared
- // within an export.
const auto *DeclCtx = ND.getDeclContext();
switch (DeclCtx->getDeclKind()) {
case Decl::TranslationUnit:
case Decl::Namespace:
case Decl::LinkageSpec:
+ // An export-declaration can only appear at namespace scope.
+ case Decl::Export:
case Decl::Enum:
case Decl::ObjCProtocol:
case Decl::ObjCInterface:
diff --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp
b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 42279b51230e7c..635f8e53f0a6a7 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -1538,6 +1538,46 @@ TEST(RenameTest, ProtobufSymbolIsExcluded) {
testing::HasSubstr("not a supported kind"));
}
+TEST(RenameTest, ExportedSymbols) {
+ // Symbols whose parent is an ExportDecl must be indexable, or rename rejects
+ // them as "not eligible for indexing".
+ const char *Tests[] = {
+ R"cpp(
+ export module m;
+ export int [[f^un]]() { return 32; }
+ int x = [[fun]]();
+ )cpp",
+ R"cpp(
+ export module m;
+ export {
+ struct [[S^truct]] {};
+ }
+ [[Struct]] s;
+ )cpp",
+ R"cpp(
+ export module m;
+ namespace ns {
+ export int [[V^ar]];
+ }
+ int y = ns::[[Var]];
+ )cpp",
+ };
+
+ for (const char *Test : Tests) {
+ SCOPED_TRACE(Test);
+ Annotations Code(Test);
+ TestTU TU = TestTU::withCode(Code.code());
+ TU.Filename = "test.cppm";
+ TU.ExtraArgs = {"-std=c++20", "-xc++-module"};
+ auto AST = TU.build();
+ auto Results =
+ rename({Code.point(), "newName", AST, testPath(TU.Filename)});
+
+ ASSERT_TRUE(bool(Results)) << llvm::toString(Results.takeError());
+ EXPECT_EQ(Results->LocalChanges, Code.ranges());
+ }
+}
+
TEST(RenameTest, PrepareRename) {
Annotations FooH("void func();");
Annotations FooCC(R"cpp(
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index a04756518ef23b..62f8cbb7b1ee00 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -2213,6 +2213,28 @@ TEST_F(SymbolCollectorTest, Concepts) {
qName("A"), hasKind(clang::index::SymbolKind::Concept))));
}
+TEST_F(SymbolCollectorTest, ExportedDeclarations) {
+ const char *Main = R"cpp(
+ export module m;
+ export void f();
+ export int v;
+ export {
+ void g();
+ struct S { void m(); };
+ }
+ namespace ns {
+ export void h();
+ void not_exported();
+ }
+ )cpp";
+
+ runSymbolCollector("", Main, {"-std=c++20", "-xc++-module"});
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(qName("f"), qName("v"), qName("g"),
+ qName("S"), qName("S::m"), qName("ns"),
+ qName("ns::h"), qName("ns::not_exported")));
+}
+
TEST_F(SymbolCollectorTest, IncludeHeaderForwardDecls) {
CollectorOpts.CollectIncludePath = true;
const std::string Header = R"cpp(#pragma once
``````````
</details>
https://github.com/llvm/llvm-project/pull/234755
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
