Skip to content

[Sema] Report use of implicitly imported decls in inlinable code #59799

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 7 commits into from
Jul 1, 2022
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
2 changes: 2 additions & 0 deletions SwiftCompilerSources/Sources/Basic/SourceLoc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
//
//===----------------------------------------------------------------------===//

import ASTBridging

public struct SourceLoc {
/// Points into a source file.
let locationInFile: UnsafePointer<UInt8>
Expand Down
21 changes: 16 additions & 5 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -2903,14 +2903,17 @@ ERROR(decl_from_hidden_module,none,
"in an extension with conditional conformances}2; "
"%select{%3 has been imported as implementation-only|"
"it is an SPI imported from %3|"
"it is SPI}4",
"it is SPI|"
"%3 was not imported by this file}4",
(DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
WARNING(decl_from_hidden_module_warn,none,
"cannot use %0 %1 %select{in SPI|as property wrapper in SPI|"
"as result builder in SPI|"
"cannot use %0 %1 %select{here|as property wrapper here|"
"as result builder here|"
"in an extension with public or '@usableFromInline' members|"
"in an extension with conditional conformances}2; "
"%select{%3 has been imported as implementation-only}4",
"%select{%3 has been imported as implementation-only|"
"<<ERROR>>|<<ERROR>>|"
"%3 was not imported by this file}4",
(DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))
ERROR(conformance_from_implementation_only_module,none,
"cannot use conformance of %0 to %1 %select{here|as property wrapper here|"
Expand Down Expand Up @@ -5781,7 +5784,15 @@ ERROR(inlinable_decl_ref_from_hidden_module,
none, "%0 %1 cannot be used in " FRAGILE_FUNC_KIND "2 "
"because %select{%3 was imported implementation-only|"
"it is an SPI imported from %3|"
"it is SPI}4",
"it is SPI|"
"%3 was not imported by this file}4",
(DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))

WARNING(inlinable_decl_ref_from_hidden_module_warn,
none, "%0 %1 cannot be used in " FRAGILE_FUNC_KIND "2 "
"because %select{<<ERROR>>|<<ERROR>>|<<ERROR>>|"
"%3 was not imported by this file}4"
"; this is an error in Swift 6",
(DescriptiveDeclKind, DeclName, unsigned, Identifier, unsigned))

ERROR(availability_macro_in_inlinable, none,
Expand Down
13 changes: 12 additions & 1 deletion include/swift/AST/SourceFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ namespace swift {

class PersistentParserState;

/// Kind of import affecting how a decl can be reexported.
/// This is a subset of \c DisallowedOriginKind.
///
/// \sa getRestrictedImportKind
enum class RestrictedImportKind {
ImplementationOnly,
Implicit,
None // No restriction, i.e. the module is imported publicly.
};

/// A file containing Swift source code.
///
/// This is a .swift or .sil file (or a virtual file, such as the contents of
Expand Down Expand Up @@ -336,7 +346,8 @@ class SourceFile final : public FileUnit {
/// If not, we can fast-path module checks.
bool hasImplementationOnlyImports() const;

bool isImportedImplementationOnly(const ModuleDecl *module) const;
/// Get the most permissive restriction applied to the imports of \p module.
RestrictedImportKind getRestrictedImportKind(const ModuleDecl *module) const;

/// Find all SPI names imported from \p importedModule by this file,
/// collecting the identifiers in \p spiGroups.
Expand Down
22 changes: 12 additions & 10 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2438,28 +2438,30 @@ bool SourceFile::hasTestableOrPrivateImport(
});
}

bool SourceFile::isImportedImplementationOnly(const ModuleDecl *module) const {
// Implementation-only imports are (currently) always source-file-specific,
// so if we don't have any, we know the search is complete.
if (!hasImplementationOnlyImports())
return false;

RestrictedImportKind SourceFile::getRestrictedImportKind(const ModuleDecl *module) const {
auto &imports = getASTContext().getImportCache();
RestrictedImportKind importKind = RestrictedImportKind::Implicit;

// Look at the imports of this source file.
for (auto &desc : *Imports) {
// Ignore implementation-only imports.
if (desc.options.contains(ImportFlags::ImplementationOnly))
if (desc.options.contains(ImportFlags::ImplementationOnly)) {
if (imports.isImportedBy(module, desc.module.importedModule))
importKind = RestrictedImportKind::ImplementationOnly;
continue;
}

// If the module is imported this way, it's not imported
// If the module is imported publicly, it's not imported
// implementation-only.
if (imports.isImportedBy(module, desc.module.importedModule))
return false;
return RestrictedImportKind::None;
}

// Now check this file's enclosing module in case there are re-exports.
return !imports.isImportedBy(module, getParentModule());
if (imports.isImportedBy(module, getParentModule()))
return RestrictedImportKind::None;

return importKind;
}

bool ModuleDecl::isImportedImplementationOnly(const ModuleDecl *module) const {
Expand Down
11 changes: 10 additions & 1 deletion lib/Sema/ResilienceDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,16 @@ TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,

D->diagnose(diag::kind_declared_here, DescriptiveDeclKind::Type);
} else {
ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_from_hidden_module,
// Only implicitly imported decls should be reported as a warning,
// and only for language versions below Swift 6.
assert(downgradeToWarning == DowngradeToWarning::No ||
originKind == DisallowedOriginKind::ImplicitlyImported &&
"Only implicitly imported decls should be reported as a warning.");
auto errorOrWarning = downgradeToWarning == DowngradeToWarning::Yes?
diag::inlinable_decl_ref_from_hidden_module_warn:
diag::inlinable_decl_ref_from_hidden_module;

ctx.Diags.diagnose(loc, errorOrWarning,
D->getDescriptiveKind(), D->getName(),
fragileKind.getSelector(), definingModule->getName(),
static_cast<unsigned>(originKind));
Expand Down
26 changes: 22 additions & 4 deletions lib/Sema/TypeCheckAccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1503,12 +1503,23 @@ swift::getDisallowedOriginKind(const Decl *decl,
downgradeToWarning = DowngradeToWarning::No;
ModuleDecl *M = decl->getModuleContext();
auto *SF = where.getDeclContext()->getParentSourceFile();
if (SF->isImportedImplementationOnly(M)) {

RestrictedImportKind howImported = SF->getRestrictedImportKind(M);
if (howImported != RestrictedImportKind::None) {
// Temporarily downgrade implementation-only exportability in SPI to
// a warning.
if (where.isSPI())
downgradeToWarning = DowngradeToWarning::Yes;

// Before Swift 6, implicit imports were not reported unless an
// implementation-only import was also present. Downgrade to a warning
// just in this case.
if (howImported == RestrictedImportKind::Implicit &&
!SF->getASTContext().isSwiftVersionAtLeast(6) &&
!SF->hasImplementationOnlyImports()) {
downgradeToWarning = DowngradeToWarning::Yes;
}

// Even if the current module is @_implementationOnly, Swift should
// not report an error in the cases where the decl is also exported from
// a non @_implementationOnly module. Thus, we check to see if there is
Expand All @@ -1529,9 +1540,12 @@ swift::getDisallowedOriginKind(const Decl *decl,
continue;
}
}
auto owningModule = redecl->getOwningModule();
if (!owningModule)
continue;
auto moduleWrapper =
decl->getASTContext().getClangModuleLoader()->getWrapperForModule(
redecl->getOwningModule());
owningModule);
auto visibleAccessPath =
find_if(sfImportedModules, [&moduleWrapper](auto importedModule) {
return importedModule.importedModule == moduleWrapper ||
Expand All @@ -1543,7 +1557,10 @@ swift::getDisallowedOriginKind(const Decl *decl,
}
}
}
// Implementation-only imported, cannot be reexported.

// Restrictively imported, cannot be reexported.
if (howImported == RestrictedImportKind::Implicit)
return DisallowedOriginKind::ImplicitlyImported;
return DisallowedOriginKind::ImplementationOnly;
} else if ((decl->isSPI() || decl->isAvailableAsSPI()) && !where.isSPI()) {
if (decl->isAvailableAsSPI() && !decl->isSPI()) {
Expand Down Expand Up @@ -1883,7 +1900,8 @@ class DeclAvailabilityChecker : public DeclVisitor<DeclAvailabilityChecker> {

const SourceFile *SF = refDecl->getDeclContext()->getParentSourceFile();
ModuleDecl *M = PGD->getModuleContext();
if (!SF->isImportedImplementationOnly(M))
RestrictedImportKind howImported = SF->getRestrictedImportKind(M);
if (howImported == RestrictedImportKind::None)
return;

auto &DE = PGD->getASTContext().Diags;
Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/TypeCheckAccess.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@ void checkAccessControl(Decl *D);
// Problematic origin of an exported type.
//
// This enum must be kept in sync with
// diag::inlinable_decl_ref_from_hidden_module,
// diag::decl_from_hidden_module and
// diag::conformance_from_implementation_only_module.
enum class DisallowedOriginKind : uint8_t {
ImplementationOnly,
SPIImported,
SPILocal,
ImplicitlyImported,
None
};

Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckDeclOverride.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2332,7 +2332,7 @@ void swift::checkImplementationOnlyOverride(const ValueDecl *VD) {
assert(SF && "checking a non-source declaration?");

ModuleDecl *M = overridden->getModuleContext();
if (SF->isImportedImplementationOnly(M)) {
if (SF->getRestrictedImportKind(M) == RestrictedImportKind::ImplementationOnly) {
VD->diagnose(diag::implementation_only_override_import_without_attr,
overridden->getDescriptiveKind())
.fixItInsert(VD->getAttributeInsertionLoc(false),
Expand Down
2 changes: 2 additions & 0 deletions stdlib/private/OSLog/OSLogFloatingPointTypes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
//
// "\(x, format: .fixed(precision: 10), privacy: .private\)"

import ObjectiveC

extension OSLogInterpolation {

/// Defines interpolation for expressions of type Float.
Expand Down
2 changes: 2 additions & 0 deletions stdlib/private/OSLog/OSLogIntegerTypes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
// 1. "\(x, format: .hex, privacy: .private, align: .right\)"
// 2. "\(x, format: .hex(minDigits: 10), align: .right(columns: 10)\)"

import ObjectiveC

extension OSLogInterpolation {

/// Defines interpolation for expressions of type Int.
Expand Down
2 changes: 2 additions & 0 deletions stdlib/private/OSLog/OSLogStringTypes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
// 1. "\(x, privacy: .private, align: .right\)"
// 2. "\(x, align: .right(columns: 10)\)"

import ObjectiveC

extension OSLogInterpolation {

/// Defines interpolation for expressions of type String.
Expand Down
8 changes: 4 additions & 4 deletions test/SPI/implementation_only_spi_import_exposability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ public protocol IOIProtocol {}

@_spi(A) @_implementationOnly import Lib

@_spi(B) public func leakSPIStruct(_ a: SPIStruct) -> SPIStruct { fatalError() } // expected-warning 2 {{cannot use struct 'SPIStruct' in SPI; 'Lib' has been imported as implementation-only}}
@_spi(B) public func leakIOIStruct(_ a: IOIStruct) -> IOIStruct { fatalError() } // expected-warning 2 {{cannot use struct 'IOIStruct' in SPI; 'Lib' has been imported as implementation-only}}
@_spi(B) public func leakSPIStruct(_ a: SPIStruct) -> SPIStruct { fatalError() } // expected-warning 2 {{cannot use struct 'SPIStruct' here; 'Lib' has been imported as implementation-only}}
@_spi(B) public func leakIOIStruct(_ a: IOIStruct) -> IOIStruct { fatalError() } // expected-warning 2 {{cannot use struct 'IOIStruct' here; 'Lib' has been imported as implementation-only}}

public struct PublicStruct : IOIProtocol, SPIProtocol { // expected-error {{cannot use protocol 'IOIProtocol' here; 'Lib' has been imported as implementation-only}}
// expected-error @-1 {{cannot use protocol 'SPIProtocol' here; 'Lib' has been imported as implementation-only}}
Expand All @@ -46,8 +46,8 @@ public struct PublicStruct : IOIProtocol, SPIProtocol { // expected-error {{cann
}

@_spi(B)
public struct LocalSPIStruct : IOIProtocol, SPIProtocol { // expected-warning {{cannot use protocol 'IOIProtocol' in SPI; 'Lib' has been imported as implementation-only}}
// expected-warning @-1 {{cannot use protocol 'SPIProtocol' in SPI; 'Lib' has been imported as implementation-only}}
public struct LocalSPIStruct : IOIProtocol, SPIProtocol { // expected-warning {{cannot use protocol 'IOIProtocol' here; 'Lib' has been imported as implementation-only}}
// expected-warning @-1 {{cannot use protocol 'SPIProtocol' here; 'Lib' has been imported as implementation-only}}
}

#endif
74 changes: 74 additions & 0 deletions test/Sema/implicit-import-in-inlinable-code.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/// Report the use in API of indirectly or implicitly imported decls.

// RUN: %empty-directory(%t)
// RUN: %{python} %utils/split_file.py -o %t %s

// RUN: %target-swift-frontend -emit-module %t/empty.swift -module-name empty -o %t/empty.swiftmodule
// RUN: %target-swift-frontend -emit-module %t/libA.swift -module-name libA -o %t/libA.swiftmodule
// RUN: %target-swift-frontend -emit-module %t/libB.swift -module-name libB -o %t/libB.swiftmodule -I %t

/// In pre-Swift 6, this is a warning where there's no implementation-only import present.
// RUN: %target-swift-frontend -emit-module %t/clientFileA-Swift5.swift %t/clientFileB.swift -module-name client -o %t/client.swiftmodule -I %t -verify

/// In pre-Swift 6, this remains an error when there's an implementation-only import present.
// RUN: %target-swift-frontend -emit-module %t/clientFileA-OldCheck.swift %t/clientFileB.swift -module-name client -o %t/client.swiftmodule -I %t -verify

/// In Swift 6, it's an error.
// RUN: %target-swift-frontend -emit-module %t/clientFileA-Swift6.swift %t/clientFileB.swift -module-name client -o %t/client.swiftmodule -I %t -verify -swift-version 6

// BEGIN empty.swift

// BEGIN libA.swift
public struct ImportedType {
public init() {}
}

// BEGIN libB.swift
import libA

extension ImportedType {
public func implicitlyImportedMethod() {}
}

/// Client module
// BEGIN clientFileA-Swift5.swift
import libA

@inlinable public func bar() {
let a = ImportedType()
a.implicitlyImportedMethod() // expected-warning {{instance method 'implicitlyImportedMethod()' cannot be used in an '@inlinable' function because 'libB' was not imported by this file; this is an error in Swift 6}}

// Expected implicit imports are still fine
a.localModuleMethod()
}

// BEGIN clientFileA-OldCheck.swift
import libA
@_implementationOnly import empty

@inlinable public func bar() {
let a = ImportedType()
a.implicitlyImportedMethod() // expected-error {{instance method 'implicitlyImportedMethod()' cannot be used in an '@inlinable' function because 'libB' was not imported by this file}}

// Expected implicit imports are still fine
a.localModuleMethod()
}

// BEGIN clientFileA-Swift6.swift
import libA

@inlinable public func bar() {
let a = ImportedType()
a.implicitlyImportedMethod() // expected-error {{instance method 'implicitlyImportedMethod()' cannot be used in an '@inlinable' function because 'libB' was not imported by this file}}

// Expected implicit imports are still fine
a.localModuleMethod()
}

// BEGIN clientFileB.swift
@_implementationOnly import libB
import libA
extension ImportedType {
public func localModuleMethod() {}
}