Skip to content

[6.1] Revert import access level / attribute filtering #79526

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

Merged
merged 2 commits into from
Feb 21, 2025
Merged
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
5 changes: 5 additions & 0 deletions include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,11 @@ class ModuleDecl
/// This assumes that \p module was imported.
bool isImportedImplementationOnly(const ModuleDecl *module) const;

/// Returns true if decl context or its content can be serialized by
/// cross-module-optimization.
/// The \p ctxt can e.g. be a NominalType or the context of a function.
bool canBeUsedForCrossModuleOptimization(DeclContext *ctxt) const;

/// Finds all top-level decls of this module.
///
/// This does a simple local lookup, not recursively looking through imports.
Expand Down
27 changes: 27 additions & 0 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3010,6 +3010,33 @@ bool ModuleDecl::isImportedImplementationOnly(const ModuleDecl *module) const {
return true;
}

bool ModuleDecl::
canBeUsedForCrossModuleOptimization(DeclContext *ctxt) const {
ModuleDecl *moduleOfCtxt = ctxt->getParentModule();

// If the context defined in the same module - or is the same module, it's
// fine.
if (moduleOfCtxt == this)
return true;

// See if context is imported in a "regular" way, i.e. not with
// @_implementationOnly, `package import` or @_spiOnly.
ModuleDecl::ImportFilter filter = {
ModuleDecl::ImportFilterKind::ImplementationOnly,
ModuleDecl::ImportFilterKind::PackageOnly,
ModuleDecl::ImportFilterKind::SPIOnly
};
SmallVector<ImportedModule, 4> results;
getImportedModules(results, filter);

auto &imports = getASTContext().getImportCache();
for (auto &desc : results) {
if (imports.isImportedBy(moduleOfCtxt, desc.importedModule))
return false;
}
return true;
}

void SourceFile::lookupImportedSPIGroups(
const ModuleDecl *importedModule,
llvm::SmallSetVector<Identifier, 4> &spiGroups) const {
Expand Down
67 changes: 2 additions & 65 deletions lib/SILOptimizer/IPO/CrossModuleOptimization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

#define DEBUG_TYPE "cross-module-serialization-setup"
#include "swift/AST/Module.h"
#include "swift/AST/ImportCache.h"
#include "swift/Basic/Assertions.h"
#include "swift/IRGen/TBDGen.h"
#include "swift/SIL/ApplySite.h"
Expand Down Expand Up @@ -104,11 +103,6 @@ class CrossModuleOptimization {
bool canSerializeType(CanType type);
bool canSerializeDecl(NominalTypeDecl *decl);

/// Check whether decls imported with certain access levels or attributes
/// can be serialized.
/// The \p ctxt can e.g. be a NominalType or the context of a function.
bool checkImports(DeclContext *ctxt) const;

bool canUseFromInline(DeclContext *declCtxt);

bool canUseFromInline(SILFunction *func);
Expand Down Expand Up @@ -742,12 +736,7 @@ static bool couldBeLinkedStatically(DeclContext *funcCtxt, SILModule &module) {
// The stdlib module is always linked dynamically.
if (funcModule == module.getASTContext().getStdlibModule())
return false;

// An sdk or system module should be linked dynamically.
if (isPackageCMOEnabled(module.getSwiftModule()) &&
funcModule->isNonUserModule())
return false;


// Conservatively assume the function is in a statically linked module.
return true;
}
Expand All @@ -757,7 +746,7 @@ bool CrossModuleOptimization::canUseFromInline(DeclContext *declCtxt) {
if (everything)
return true;

if (!checkImports(declCtxt))
if (!M.getSwiftModule()->canBeUsedForCrossModuleOptimization(declCtxt))
return false;

/// If we are emitting a TBD file, the TBD file only contains public symbols
Expand All @@ -773,58 +762,6 @@ bool CrossModuleOptimization::canUseFromInline(DeclContext *declCtxt) {
return true;
}

bool CrossModuleOptimization::checkImports(DeclContext *ctxt) const {
ModuleDecl *moduleOfCtxt = ctxt->getParentModule();

// If the context defined in the same module - or is the same module, it's
// fine.
if (moduleOfCtxt == M.getSwiftModule())
return true;

ModuleDecl::ImportFilter filter;

if (isPackageCMOEnabled(M.getSwiftModule())) {
// When Package CMO is enabled, types imported with `package import`
// or `@_spiOnly import` into this module should be allowed to be
// serialized. These types may be used in APIs with `package` or
// higher access level, with or without `@_spi`, and such APIs should
// be serializable to allow direct access by another module if it's
// in the same package.
//
// However, types are from modules imported as `@_implementationOnly`
// should not be serialized, even if their defining modules are SDK
// or system modules. Since these types are intended to remain hidden
// from external clients, their metadata (e.g. field offsets) may be
// stripped, making it unavailable for look up at runtime. If serialized,
// the client will attempt to use the serialized accessor and fail
// because the metadata is missing, leading to a linker error.
//
// This issue applies to transitively imported types as well;
// `@_implementationOnly import Foundation` imports `ObjectiveC`
// indirectly, and metadata for types like `NSObject` from `ObjectiveC`
// can also be stripped, thus such types should not be allowed for
// serialization.
filter = { ModuleDecl::ImportFilterKind::ImplementationOnly };
} else {
// See if context is imported in a "regular" way, i.e. not with
// @_implementationOnly, `package import` or @_spiOnly.
filter = {
ModuleDecl::ImportFilterKind::ImplementationOnly,
ModuleDecl::ImportFilterKind::PackageOnly,
ModuleDecl::ImportFilterKind::SPIOnly
};
}
SmallVector<ImportedModule, 4> results;
M.getSwiftModule()->getImportedModules(results, filter);

auto &imports = M.getSwiftModule()->getASTContext().getImportCache();
for (auto &desc : results) {
if (imports.isImportedBy(moduleOfCtxt, desc.importedModule))
return false;
}
return true;
}

/// Returns true if the function \p func can be used from a serialized function.
bool CrossModuleOptimization::canUseFromInline(SILFunction *function) {
if (everything)
Expand Down
121 changes: 0 additions & 121 deletions test/SILOptimizer/package-cmo-import-filter.swift

This file was deleted.