Skip to content

Revert "Ignore transitive ObjC imports when cross-importing" #30904

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

Closed
Closed
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
4 changes: 1 addition & 3 deletions include/swift/AST/ClangModuleLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ class ClangModuleLoader : public ModuleLoader {

/// Retrieves the Swift wrapper for the given Clang module, creating
/// it if necessary.
virtual ModuleDecl *
getWrapperForModule(const clang::Module *mod,
bool returnOverlayIfPossible = false) const = 0;
virtual ModuleDecl *getWrapperForModule(const clang::Module *mod) const = 0;

/// Adds a new search path to the Clang CompilerInstance, as if specified with
/// -I or -F.
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ class ModuleDecl : public DeclContext, public TypeDecl {
/// Retrieve the top-level module. If this module is already top-level, this
/// returns itself. If this is a submodule such as \c Foo.Bar.Baz, this
/// returns the module \c Foo.
ModuleDecl *getTopLevelModule(bool overlay = false);
ModuleDecl *getTopLevelModule();

bool isResilient() const {
return getResilienceStrategy() != ResilienceStrategy::Default;
Expand Down
4 changes: 1 addition & 3 deletions include/swift/ClangImporter/ClangImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,7 @@ class ClangImporter final : public ClangModuleLoader {

/// Retrieves the Swift wrapper for the given Clang module, creating
/// it if necessary.
ModuleDecl *
getWrapperForModule(const clang::Module *mod,
bool returnOverlayIfPossible = false) const override;
ModuleDecl *getWrapperForModule(const clang::Module *mod) const override;

std::string getBridgingHeaderContents(StringRef headerPath, off_t &fileSize,
time_t &fileModTime);
Expand Down
5 changes: 2 additions & 3 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -514,16 +514,15 @@ SourceLookupCache &ModuleDecl::getSourceLookupCache() const {
return *Cache;
}

ModuleDecl *ModuleDecl::getTopLevelModule(bool overlay) {
ModuleDecl *ModuleDecl::getTopLevelModule() {
// If this is a Clang module, ask the Clang importer for the top-level module.
// We need to check isNonSwiftModule() to ensure we don't look through
// overlays.
if (isNonSwiftModule()) {
if (auto *underlying = findUnderlyingClangModule()) {
auto &ctx = getASTContext();
auto *clangLoader = ctx.getClangModuleLoader();
return clangLoader->getWrapperForModule(underlying->getTopLevelModule(),
overlay);
return clangLoader->getWrapperForModule(underlying->getTopLevelModule());
}
}
// Swift modules don't currently support submodules.
Expand Down
9 changes: 2 additions & 7 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1860,13 +1860,8 @@ ModuleDecl *ClangImporter::getImportedHeaderModule() const {
return Impl.ImportedHeaderUnit->getParentModule();
}

ModuleDecl *
ClangImporter::getWrapperForModule(const clang::Module *mod,
bool returnOverlayIfPossible) const {
auto clangUnit = Impl.getWrapperForModule(mod);
if (returnOverlayIfPossible && clangUnit->getOverlayModule())
return clangUnit->getOverlayModule();
return clangUnit->getParentModule();
ModuleDecl *ClangImporter::getWrapperForModule(const clang::Module *mod) const {
return Impl.getWrapperForModule(mod)->getParentModule();
}

PlatformAvailability::PlatformAvailability(LangOptions &langOpts)
Expand Down
45 changes: 16 additions & 29 deletions lib/Sema/ImportResolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,15 @@ class ImportResolver final : public DeclVisitor<ImportResolver> {
/// The list of fully bound imports.
SmallVector<ImportedModuleDesc, 16> boundImports;

/// All imported modules which should be considered when cross-importing.
/// This is basically the transitive import graph, but with only top-level
/// modules and without reexports from Objective-C modules.
/// All imported modules, including by re-exports, and including submodules.
llvm::DenseSet<ImportedModuleDesc> visibleModules;

/// \c visibleModules but without the submodules.
///
/// We use a \c SmallSetVector here because this doubles as the worklist for
/// cross-importing, so we want to keep it in order; this is feasible
/// because this set is usually fairly small.
/// because this set is usually fairly small, while \c visibleModules is
/// often enormous.
SmallSetVector<ImportedModuleDesc, 64> crossImportableModules;

/// The subset of \c crossImportableModules which may declare cross-imports.
Expand Down Expand Up @@ -221,7 +223,7 @@ class ImportResolver final : public DeclVisitor<ImportResolver> {

/// Adds \p desc and everything it re-exports to \c visibleModules using
/// the settings from \c desc.
void addCrossImportableModules(ImportedModuleDesc desc);
void addVisibleModules(ImportedModuleDesc desc);

/// * If \p I is a cross-import overlay, registers \p M as overlaying
/// \p I.underlyingModule in \c SF.
Expand Down Expand Up @@ -339,7 +341,7 @@ void ImportResolver::bindImport(UnboundImport &&I) {

void ImportResolver::addImport(const UnboundImport &I, ModuleDecl *M) {
auto importDesc = I.makeDesc(M);
addCrossImportableModules(importDesc);
addVisibleModules(importDesc);
boundImports.push_back(importDesc);
}

Expand Down Expand Up @@ -934,7 +936,7 @@ static bool isSubmodule(ModuleDecl* M) {
return clangMod && clangMod->Parent;
}

void ImportResolver::addCrossImportableModules(ImportedModuleDesc importDesc) {
void ImportResolver::addVisibleModules(ImportedModuleDesc importDesc) {
// FIXME: namelookup::getAllImports() doesn't quite do what we need (mainly
// w.r.t. scoped imports), but it seems like we could extend it to do so, and
// then eliminate most of this.
Expand All @@ -951,34 +953,19 @@ void ImportResolver::addCrossImportableModules(ImportedModuleDesc importDesc) {
nextImport.first))
continue;

// If we are importing a submodule, treat it as though we imported its
// top-level module (or rather, the top-level module's clang overlay if it
// has one).
if (isSubmodule(nextImport.second)) {
nextImport.second =
nextImport.second->getTopLevelModule(/*overlay=*/true);

// If the rewritten import is now for our own parent module, this was an
// import of our own clang submodule in a mixed-language module. We don't
// want to process our own cross-imports.
if (nextImport.second == SF.getParentModule())
continue;
}

// Drop this module into the ImportDesc so we treat it as imported with the
// same options and scope as `I`.
importDesc.module.second = nextImport.second;

// Add it to the list of cross-importable modules. If it's already there,
// we've already done the rest of the work of this loop iteration and can
// skip it.
if (!crossImportableModules.insert(importDesc))
// If we've already imported it, we've also already imported its
// imports.
if (!visibleModules.insert(importDesc).second)
continue;

// We don't consider the re-exports of ObjC modules because ObjC re-exports
// everything, so there isn't enough signal there to work from.
if (nextImport.second->isNonSwiftModule())
continue;
// We don't cross-import submodules, so we shouldn't add them to the
// visibility set. (However, we do consider their reexports.)
if(!isSubmodule(importDesc.module.second))
crossImportableModules.insert(importDesc);

// Add the module's re-exports to worklist.
nextImport.second->getImportedModules(importsWorklist,
Expand Down
1 change: 0 additions & 1 deletion test/CrossImport/Inputs/lib-templates/include/core_mi6.h

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,3 @@ module ClangLibrary {
header "clang_library_submodule.h"
}
}
module core_mi6 {
header "core_mi6.h"
export *
}
module UniversalExports {
header "universal_exports.h"
export *
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@
// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name UniversalExports

import Swift

// This is also a clang overlay:
@_exported import UniversalExports

@_exported import AlwaysImported
import NeverImported
import MI6 // @_exported imports NeverImported
6 changes: 0 additions & 6 deletions test/CrossImport/transitive.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ import UniversalExports
import ThinLibrary
#endif

// We should have loaded the underlying clang module.
fromUniversalExportsClang() // no-error

// We should have loaded core_mi6 too.
fromCoreMI6() // no-error

// We should have loaded _AlwaysImportedOverlay.
fromAlwaysImportedOverlay() // no-error

Expand Down