Skip to content

Revert "Revert "[cxx-interop] Import decls in extern blocks within namespaces"" #83589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 76 additions & 61 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2738,72 +2738,87 @@ static void addNamespaceMembers(Decl *decl,
if (declOwner && declOwner != redeclOwner->getTopLevelModule())
continue;
}
for (auto member : redecl->decls()) {
if (auto classTemplate = dyn_cast<clang::ClassTemplateDecl>(member)) {
// Add all specializations to a worklist so we don't accidentally mutate
// the list of decls we're iterating over.
llvm::SmallPtrSet<const clang::ClassTemplateSpecializationDecl *, 16> specWorklist;
for (auto spec : classTemplate->specializations())
specWorklist.insert(spec);
for (auto spec : specWorklist) {
if (auto import =
ctx.getClangModuleLoader()->importDeclDirectly(spec))
if (addedMembers.insert(import).second)
members.push_back(import);
}
}

auto lookupAndAddMembers = [&](DeclName name) {
auto allResults = evaluateOrDefault(
ctx.evaluator, ClangDirectLookupRequest({decl, redecl, name}), {});

for (auto found : allResults) {
auto clangMember = cast<clang::NamedDecl *>(found);
if (auto importedDecl =
ctx.getClangModuleLoader()->importDeclDirectly(clangMember)) {
if (addedMembers.insert(importedDecl).second) {
members.push_back(importedDecl);

// Handle macro-expanded declarations.
importedDecl->visitAuxiliaryDecls([&](Decl *decl) {
auto valueDecl = dyn_cast<ValueDecl>(decl);
if (!valueDecl)
return;

// Bail out if the auxiliary decl was not produced by a macro.
auto module = decl->getDeclContext()->getParentModule();
auto *sf = module->getSourceFileContainingLocation(decl->getLoc());
if (!sf || sf->Kind != SourceFileKind::MacroExpansion)
return;

members.push_back(valueDecl);
});
std::function<void(clang::DeclContext *)> addDeclsFromContext =
[&](clang::DeclContext *declContext) {
for (auto member : declContext->decls()) {
if (auto classTemplate =
dyn_cast<clang::ClassTemplateDecl>(member)) {
// Add all specializations to a worklist so we don't accidentally
// mutate the list of decls we're iterating over.
llvm::SmallPtrSet<const clang::ClassTemplateSpecializationDecl *,
16>
specWorklist;
for (auto spec : classTemplate->specializations())
specWorklist.insert(spec);
for (auto spec : specWorklist) {
if (auto import =
ctx.getClangModuleLoader()->importDeclDirectly(spec))
if (addedMembers.insert(import).second)
members.push_back(import);
}
}
}
}
};

auto namedDecl = dyn_cast<clang::NamedDecl>(member);
if (!namedDecl)
continue;
auto name = ctx.getClangModuleLoader()->importName(namedDecl);
if (!name)
continue;
lookupAndAddMembers(name);

// Unscoped enums could have their enumerators present
// in the parent namespace.
if (auto *ed = dyn_cast<clang::EnumDecl>(member)) {
if (!ed->isScoped()) {
for (const auto *ecd : ed->enumerators()) {
auto name = ctx.getClangModuleLoader()->importName(ecd);
if (!name)
auto lookupAndAddMembers = [&](clang::NamedDecl *namedDecl) {
auto name = ctx.getClangModuleLoader()->importName(namedDecl);
if (!name)
return;

auto allResults = evaluateOrDefault(
ctx.evaluator, ClangDirectLookupRequest({decl, redecl, name}),
{});

for (auto found : allResults) {
auto clangMember = cast<clang::NamedDecl *>(found);
if (auto importedDecl =
ctx.getClangModuleLoader()->importDeclDirectly(
clangMember)) {
if (addedMembers.insert(importedDecl).second) {
members.push_back(importedDecl);

// Handle macro-expanded declarations.
importedDecl->visitAuxiliaryDecls([&](Decl *decl) {
auto valueDecl = dyn_cast<ValueDecl>(decl);
if (!valueDecl)
return;

// Bail out if the auxiliary decl was not produced by a
// macro.
auto module = decl->getDeclContext()->getParentModule();
auto *sf = module->getSourceFileContainingLocation(
decl->getLoc());
if (!sf || sf->Kind != SourceFileKind::MacroExpansion)
return;

members.push_back(valueDecl);
});
}
}
}
};

// Look through `extern` blocks.
if (auto linkageSpecDecl = dyn_cast<clang::LinkageSpecDecl>(member))
addDeclsFromContext(linkageSpecDecl);

auto namedDecl = dyn_cast<clang::NamedDecl>(member);
if (!namedDecl)
continue;
lookupAndAddMembers(name);
lookupAndAddMembers(namedDecl);

// Unscoped enums could have their enumerators present
// in the parent namespace.
if (auto *ed = dyn_cast<clang::EnumDecl>(member)) {
if (!ed->isScoped()) {
for (auto *ecd : ed->enumerators()) {
lookupAndAddMembers(ecd);
}
}
}
}
}
}
}
};

addDeclsFromContext(redecl);
}
}

Expand Down
5 changes: 5 additions & 0 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5064,6 +5064,11 @@ static bool isDirectLookupMemberContext(const clang::Decl *foundClangDecl,
return firstDecl->getCanonicalDecl() == parent->getCanonicalDecl();
}
}
// Look through `extern` blocks.
if (auto linkageSpecDecl = dyn_cast<clang::LinkageSpecDecl>(memberContext)) {
if (auto parentDecl = dyn_cast<clang::Decl>(linkageSpecDecl->getParent()))
return isDirectLookupMemberContext(foundClangDecl, parentDecl, parent);
}
return false;
}

Expand Down
7 changes: 5 additions & 2 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1155,11 +1155,14 @@ namespace {
return nullptr;
// If this is a top-level namespace, don't put it in the module we're
// importing, put it in the "__ObjC" module that is implicitly imported.
if (!decl->getParent()->isNamespace())
auto clangDC = decl->getDeclContext();
while (isa<clang::LinkageSpecDecl>(clangDC))
clangDC = clangDC->getParent();
if (!clangDC->isNamespace())
dc = Impl.ImportedHeaderUnit;
else {
// This is a nested namespace, so just lookup it's parent normally.
auto parentNS = cast<clang::NamespaceDecl>(decl->getParent());
auto parentNS = cast<clang::NamespaceDecl>(clangDC);
auto parent =
Impl.importDecl(parentNS, getVersion(), /*UseCanonicalDecl*/ false);
// The parent namespace might not be imported if it's `swift_private`.
Expand Down
24 changes: 24 additions & 0 deletions lib/ClangImporter/SwiftLookupTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2034,6 +2034,30 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
namedMember = def;
addEntryToLookupTable(table, namedMember, nameImporter);
}
if (auto linkageSpecDecl =
dyn_cast<clang::LinkageSpecDecl>(canonicalMember)) {
std::function<void(clang::DeclContext *)> addDeclsFromContext =
[&](clang::DeclContext *declContext) {
for (auto nestedDecl : declContext->decls()) {
if (auto namedMember = dyn_cast<clang::NamedDecl>(nestedDecl))
addEntryToLookupTable(table, namedMember, nameImporter);
else if (auto nestedLinkageSpecDecl =
dyn_cast<clang::LinkageSpecDecl>(nestedDecl))
addDeclsFromContext(nestedLinkageSpecDecl);
}
};

// HACK: libc++ redeclares lgamma_r in one of its headers, and that
// declaration hijacks lgamma_r from math.h where it is originally
// defined. This causes deserialization issues when loading the Darwin
// overlay on Apple platforms, because Swift cannot find lgamma_r in
// module _math.
bool shouldSkip = canonicalMember->getOwningModule() &&
canonicalMember->getOwningModule()->Name ==
"std_private_random_binomial_distribution";
if (!shouldSkip)
addDeclsFromContext(linkageSpecDecl);
}
}
}
if (auto usingDecl = dyn_cast<clang::UsingDecl>(named)) {
Expand Down
28 changes: 28 additions & 0 deletions test/Interop/Cxx/namespace/Inputs/extern-within-namespace.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
namespace Outer {
namespace Inner {
extern "C" {
int foobar() { return 123; }
struct NestedType {
char c;
};
}
} // namespace Inner

inline namespace InnerInline {
extern "C" {
int baz() { return 321; }
}
} // namespace InnerInline
} // namespace Outer

namespace ExternWithinExtern {
extern "C" {
extern "C++" {
namespace Inner {
extern "C" {
int deep() { return 42; }
}
} // namespace Inner
}
}
} // namespace ExternWithinExtern
6 changes: 6 additions & 0 deletions test/Interop/Cxx/namespace/Inputs/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ module ClassesSecondHeader {
requires cplusplus
}

module ExternWithinNamespace {
header "extern-within-namespace.h"
export *
requires cplusplus
}

module FreeFunctions {
header "free-functions.h"
requires cplusplus
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// RUN: %target-swift-ide-test -print-module -module-to-print=ExternWithinNamespace -I %S/Inputs -source-filename=x -cxx-interoperability-mode=upcoming-swift | %FileCheck %s

// CHECK: enum Outer {
// CHECK-NEXT: enum Inner {
// CHECK-NEXT: static func foobar()
// CHECK-NEXT: struct NestedType {
// CHECK: }
// CHECK-NEXT: }
// CHECK-NEXT: enum InnerInline {
// CHECK-NEXT: static func baz() -> Int32
// CHECK-NEXT: }
// CHECK-NEXT: }
// CHECK-NEXT: enum ExternWithinExtern {
// CHECK-NEXT: enum Inner {
// CHECK-NEXT: static func deep() -> Int32
// CHECK-NEXT: }
// CHECK-NEXT: }
25 changes: 25 additions & 0 deletions test/Interop/Cxx/namespace/extern-within-namespace.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// RUN: %target-run-simple-swift(-I %S/Inputs -Xfrontend -cxx-interoperability-mode=upcoming-swift)

// REQUIRES: executable_test

import StdlibUnittest
import ExternWithinNamespace

var ExternTestSuite = TestSuite("Extern block within namespaces")

ExternTestSuite.test("Function within extern block within namespace") {
let r = Outer.Inner.foobar()
expectEqual(r, 123)
}

ExternTestSuite.test("Function within extern block within inline namespace") {
let r = Outer.InnerInline.baz()
expectEqual(r, 321)
}

ExternTestSuite.test("Function within extern block within extern block") {
let r = ExternWithinExtern.Inner.deep()
expectEqual(r, 42)
}

runAllTests()