Skip to content
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
6 changes: 6 additions & 0 deletions include/swift/AST/DiagnosticEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,12 @@ namespace swift {
/// emitted as a warning, but a note will still be emitted as a note.
InFlightDiagnostic &limitBehavior(DiagnosticBehavior limit);

/// Prevent the diagnostic from behaving more severely than \p limit or the
/// previously set limit. Use to compose conditions to downgrade a
/// diagnostic in succession.
InFlightDiagnostic
&limitBehaviorIfMorePermissive(DiagnosticBehavior limit);

/// Conditionally prevent the diagnostic from behaving more severely than \p
/// limit. If the condition is false, no limit is imposed.
InFlightDiagnostic &limitBehaviorIf(bool shouldLimit,
Expand Down
5 changes: 3 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1272,8 +1272,9 @@ ERROR(module_not_compiled_with_library_evolution,none,
(Identifier, Identifier))
GROUPED_WARNING(implementation_only_requires_library_evolution,
ImplementationOnlyDeprecated,none,
"using '@_implementationOnly' without enabling library evolution "
"for %0 may lead to instability during execution",
"safely use '@_implementationOnly' without library evolution "
"by setting '-enable-experimental-feature CheckImplementationOnly' "
"for %0",
(Identifier))
GROUPED_WARNING(implementation_only_deprecated,
ImplementationOnlyDeprecated,none,
Expand Down
20 changes: 8 additions & 12 deletions lib/AST/AccessRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,15 @@ AccessLevelRequest::evaluate(Evaluator &evaluator, ValueDecl *D) const {
// Special case for dtors and enum elements: inherit from container
if (D->getKind() == DeclKind::Destructor ||
D->getKind() == DeclKind::EnumElement) {
if (D->hasInterfaceType() && D->isInvalid()) {
return AccessLevel::Private;
} else {
auto container = dyn_cast<NominalTypeDecl>(DC);
if (D->getKind() == DeclKind::Destructor && !container) {
// A destructor in an extension means @_objcImplementation. An
// @_objcImplementation class's deinit is only called by the ObjC thunk,
// if at all, so it is nonpublic.
return AccessLevel::Internal;
}

return std::max(container->getFormalAccess(), AccessLevel::Internal);
auto container = dyn_cast<NominalTypeDecl>(DC);
if (D->getKind() == DeclKind::Destructor && !container) {
// A destructor in an extension means @_objcImplementation. An
// @_objcImplementation class's deinit is only called by the ObjC thunk,
// if at all, so it is nonpublic.
return AccessLevel::Internal;
}

return std::max(container->getFormalAccess(), AccessLevel::Internal);
}

switch (DC->getContextKind()) {
Expand Down
4 changes: 1 addition & 3 deletions lib/AST/Availability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1014,8 +1014,6 @@ ExportedLevel swift::isExported(const ValueDecl *VD) {

// Is this a type exposed by default in a non-resilient module?
if (isa<NominalTypeDecl>(VD) &&
VD->getASTContext().LangOpts.hasFeature(
Feature::CheckImplementationOnly) &&
VD->getDeclContext()->getParentModule()->getResilienceStrategy() !=
ResilienceStrategy::Resilient &&
!VD->getAttrs().hasAttribute<ImplementationOnlyAttr>())
Expand Down Expand Up @@ -1064,5 +1062,5 @@ ExportedLevel swift::isExported(const ExtensionDecl *ED) {
for (const Decl *D : ED->getMembers())
membersExported = std::max(membersExported, isExported(D));

return membersExported;
return std::min(exported, membersExported);
}
2 changes: 1 addition & 1 deletion lib/AST/AvailabilityScopeBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ class AvailabilityScopeBuilder : private ASTWalker {
if (decl->isSPI())
return true;

return isExported(decl) == ExportedLevel::None;
return isExported(decl) != ExportedLevel::Exported;
}

/// Returns the source range which should be refined by declaration. This
Expand Down
3 changes: 1 addition & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2808,8 +2808,7 @@ ExportedLevel VarDecl::isLayoutExposedToClients() const {
return ExportedLevel::None;

auto M = getDeclContext()->getParentModule();
if (getASTContext().LangOpts.hasFeature(Feature::CheckImplementationOnly) &&
M->getResilienceStrategy() != ResilienceStrategy::Resilient) {
if (M->getResilienceStrategy() != ResilienceStrategy::Resilient) {
// Non-resilient module expose layouts by default.
return ExportedLevel::ImplicitlyExported;
} else {
Expand Down
7 changes: 7 additions & 0 deletions lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,13 @@ InFlightDiagnostic::limitBehavior(DiagnosticBehavior limit) {
return *this;
}

InFlightDiagnostic &
InFlightDiagnostic::limitBehaviorIfMorePermissive(DiagnosticBehavior limit) {
auto prev = getDiag().getBehaviorLimit();
getDiag().setBehaviorLimit(prev.merge(limit));
return *this;
}

InFlightDiagnostic &
InFlightDiagnostic::limitBehaviorUntilLanguageMode(DiagnosticBehavior limit,
unsigned majorVersion) {
Expand Down
4 changes: 2 additions & 2 deletions lib/Sema/ImportResolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -929,8 +929,8 @@ void UnboundImport::validateResilience(NullablePtr<ModuleDecl> topLevelModule,
diag::implementation_only_deprecated);
inFlight.fixItReplace(import.implementationOnlyRange, "internal");
}
} else if ( // Non-resilient client
!shouldSuppressNonResilientImplementationOnlyImportDiagnostic(
} else if (!ctx.LangOpts.hasFeature(Feature::CheckImplementationOnly) &&
!shouldSuppressNonResilientImplementationOnlyImportDiagnostic(
targetName.str(), importerName.str())) {
ctx.Diags.diagnose(import.importLoc,
diag::implementation_only_requires_library_evolution,
Expand Down
39 changes: 23 additions & 16 deletions lib/Sema/ResilienceDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
auto ignoredDowngradeToWarning = DowngradeToWarning::No;
auto originKind =
getDisallowedOriginKind(D, where, ignoredDowngradeToWarning);
if (where.canReferenceOrigin(originKind))
auto commonBehavior = where.behaviorForReferenceToOrigin(originKind);
if (commonBehavior == DiagnosticBehavior::Ignore)
return false;

// As an exception, if the import of the module that defines the desugared
Expand All @@ -197,15 +198,17 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
TAD, definingModule->getNameStr(), D->getNameStr(),
static_cast<unsigned>(*reason), definingModule->getName(),
static_cast<unsigned>(originKind))
.warnUntilLanguageModeIf(warnPreSwift6, 6);
.warnUntilLanguageModeIf(warnPreSwift6, 6)
.limitBehaviorIfMorePermissive(commonBehavior);
} else {
ctx.Diags
.diagnose(loc,
diag::inlinable_typealias_desugars_to_type_from_hidden_module,
TAD, definingModule->getNameStr(), D->getNameStr(),
fragileKind.getSelector(), definingModule->getName(),
static_cast<unsigned>(originKind))
.warnUntilLanguageModeIf(warnPreSwift6, 6);
.warnUntilLanguageModeIf(warnPreSwift6, 6)
.limitBehaviorIfMorePermissive(commonBehavior);
}
D->diagnose(diag::kind_declared_here, DescriptiveDeclKind::Type);

Expand Down Expand Up @@ -249,7 +252,13 @@ static bool shouldDiagnoseDeclAccess(const ValueDecl *D,
switch (*reason) {
case ExportabilityReason::ExtensionWithPublicMembers:
case ExportabilityReason::ExtensionWithConditionalConformances:
return true;
// Allow public members in extensions of implicitly exported types.
// Extensions cannot define stored variables avoiding the memory layout
// concerns and we don't print swiftinterfaces in non-library-evolution
// mode.
// We should be able to always allow this if it's correctly guarded
// at generating module interfaces and generated headers.
return where.getExportedLevel() == ExportedLevel::Exported;
case ExportabilityReason::Inheritance:
case ExportabilityReason::ImplicitlyPublicInheritance:
return isa<ProtocolDecl>(D);
Expand Down Expand Up @@ -301,7 +310,8 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
}
});

if (where.canReferenceOrigin(originKind))
auto commonBehavior = where.behaviorForReferenceToOrigin(originKind);
if (commonBehavior == DiagnosticBehavior::Ignore)
return false;

auto fragileKind = where.getFragileFunctionKind();
Expand Down Expand Up @@ -361,21 +371,16 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
static_cast<unsigned>(*reason),
definingModule->getName(),
static_cast<unsigned>(originKind))
.limitBehavior(limit);
.limitBehavior(limit.merge(commonBehavior));

D->diagnose(diag::kind_declared_here, D->getDescriptiveKind());
} else {
// Only implicitly imported decls should be reported as a warning,
// and only for language versions below Swift 6.
assert(downgradeToWarning == DowngradeToWarning::No ||
originKind == DisallowedOriginKind::MissingImport &&
"Only implicitly imported decls should be reported as a warning.");

ctx.Diags.diagnose(loc, diag::inlinable_decl_ref_from_hidden_module, D,
fragileKind.getSelector(), definingModule->getName(),
static_cast<unsigned>(originKind))
.warnUntilLanguageModeIf(downgradeToWarning == DowngradeToWarning::Yes,
6);
6)
.limitBehaviorIfMorePermissive(commonBehavior);

if (originKind == DisallowedOriginKind::MissingImport &&
downgradeToWarning == DowngradeToWarning::Yes)
Expand Down Expand Up @@ -447,7 +452,8 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
});

auto originKind = getDisallowedOriginKind(ext, where);
if (where.canReferenceOrigin(originKind))
auto commonBehavior = where.behaviorForReferenceToOrigin(originKind);
if (commonBehavior == DiagnosticBehavior::Ignore)
return false;

auto reason = where.getExportabilityReason();
Expand All @@ -463,8 +469,9 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
(warnIfConformanceUnavailablePreSwift6 &&
originKind != DisallowedOriginKind::SPIOnly &&
originKind != DisallowedOriginKind::NonPublicImport) ||
originKind == DisallowedOriginKind::MissingImport,
6);
originKind == DisallowedOriginKind::MissingImport,
6)
.limitBehaviorIfMorePermissive(commonBehavior);

if (!ctx.LangOpts.hasFeature(Feature::StrictAccessControl) &&
originKind == DisallowedOriginKind::MissingImport &&
Expand Down
2 changes: 0 additions & 2 deletions lib/Sema/TypeCheckAccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2169,8 +2169,6 @@ swift::getDisallowedOriginKind(const Decl *decl,

// Implementation-only memory layouts for non-library-evolution mode.
if (isa<NominalTypeDecl>(decl) &&
Context.LangOpts.hasFeature(
Feature::CheckImplementationOnly) &&
decl->getAttrs().hasAttribute<ImplementationOnlyAttr>())
return DisallowedOriginKind::ImplementationOnlyMemoryLayout;

Expand Down
19 changes: 15 additions & 4 deletions lib/Sema/TypeCheckAvailability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,11 @@ bool ExportContext::mustOnlyReferenceExportedDecls() const {
return Exported || FragileKind.kind != FragileFunctionKind::None;
}

bool ExportContext::canReferenceOrigin(DisallowedOriginKind originKind) const {
DiagnosticBehavior
ExportContext::behaviorForReferenceToOrigin(DisallowedOriginKind originKind)
const {
if (originKind == DisallowedOriginKind::None)
return true;
return DiagnosticBehavior::Ignore;

// Exportability checks for non-library-evolution mode have less restrictions
// than the library-evolution ones. Implicitly always emit into client code
Expand All @@ -253,7 +255,7 @@ bool ExportContext::canReferenceOrigin(DisallowedOriginKind originKind) const {
case DisallowedOriginKind::SPIOnly:
case DisallowedOriginKind::SPIImported:
case DisallowedOriginKind::SPILocal:
return true;
return DiagnosticBehavior::Ignore;
case DisallowedOriginKind::MissingImport:
case DisallowedOriginKind::InternalBridgingHeaderImport:
case DisallowedOriginKind::ImplementationOnly:
Expand All @@ -263,7 +265,16 @@ bool ExportContext::canReferenceOrigin(DisallowedOriginKind originKind) const {
}
}

return false;
// Exportability checking for non-library-evolution was introduced late,
// downgrade errors to warnings by default.
auto &ctx = DC->getASTContext();
if (getExportedLevel() == ExportedLevel::ImplicitlyExported &&
originKind != DisallowedOriginKind::ImplementationOnlyMemoryLayout &&
!ctx.LangOpts.hasFeature(Feature::CheckImplementationOnly) &&
!ctx.isLanguageModeAtLeast(7))
return DiagnosticBehavior::Warning;

return DiagnosticBehavior::Error;
}

std::optional<ExportabilityReason>
Expand Down
7 changes: 4 additions & 3 deletions lib/Sema/TypeCheckAvailability.h
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,10 @@ class ExportContext {
/// because it is the function body context of an inlinable function.
bool mustOnlyReferenceExportedDecls() const;

/// If true, the context reference a dependency of \p originKind without
/// restriction.
bool canReferenceOrigin(DisallowedOriginKind originKind) const;
/// Level of restriction to references from the context to an \p originKind.
/// This check is shared by different diagnostics.
DiagnosticBehavior
behaviorForReferenceToOrigin(DisallowedOriginKind originKind) const;

/// Get the ExportabilityReason for diagnostics. If this is 'None', there
/// are no restrictions on referencing unexported declarations.
Expand Down
2 changes: 1 addition & 1 deletion test/Index/enum_case_with_invalid_associated_type.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
enum MyEnum {
// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s | %FileCheck %s
case test(artifactID: String, hostTriple: Triple)
// CHECK: enumerator(less_than_private)/Swift | test(artifactID:hostTriple:)
// CHECK: enumerator(internal)/Swift | test(artifactID:hostTriple:)
// CHECK: param/Swift | artifactID
// CHECK: param/Swift | hostTriple
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
// REQUIRES: embedded_stdlib_cross_compiling

@_implementationOnly internal import directs
// expected-warning @-1 {{using '@_implementationOnly' without enabling library evolution for 'main' may lead to instability during execution}}
// expected-warning @-1 {{safely use '@_implementationOnly' without library evolution by setting '-enable-experimental-feature CheckImplementationOnly' for 'main'}}
// expected-note @-2 11 {{struct 'StructFromDirect' imported as 'internal' from 'directs' here}}
// expected-note @-3 6 {{initializer 'init()' imported as 'internal' from 'directs' here}}
import indirects
Expand Down Expand Up @@ -174,9 +174,9 @@ public struct ExposedLayoutPublic {
// expected-error @-1 {{cannot use struct 'StructFromDirect' in a property declaration marked public or in a '@frozen' or '@usableFromInline' context; 'directs' has been imported as implementation-only}}
// expected-note @-2 {{struct 'StructFromDirect' is imported by this file as 'internal' from 'directs'}}

private var privateField: StructFromDirect
private var privateField: StructFromDirect // expected-warning {{cannot use struct 'StructFromDirect' in a property declaration member of a type not marked '@_implementationOnly'; 'directs' has been imported as implementation-only}}
}

private struct ExposedLayoutPrivate {
private var privateField: StructFromDirect
private var privateField: StructFromDirect // expected-warning {{cannot use struct 'StructFromDirect' in a property declaration member of a type not marked '@_implementationOnly'; 'directs' has been imported as implementation-only}}
}
43 changes: 35 additions & 8 deletions test/Sema/access-level-import-classic-exportability.swift
Original file line number Diff line number Diff line change
@@ -1,30 +1,52 @@
// RUN: %empty-directory(%t)
// RUN: split-file --leading-lines %s %t

/// Build the libraries.
/// Non-library-evolution.
// RUN: %target-swift-frontend -emit-module %t/PublicLib.swift -o %t
// RUN: %target-swift-frontend -emit-module %t/PackageLib.swift -o %t
// RUN: %target-swift-frontend -emit-module %t/InternalLib.swift -o %t \
// RUN: -package-name pkg
// RUN: %target-swift-frontend -emit-module %t/FileprivateLib.swift -o %t
// RUN: %target-swift-frontend -emit-module %t/PrivateLib.swift -o %t

/// Check diagnostics.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -package-name pkg -swift-version 5 \
// RUN: -verify -verify-ignore-unrelated

/// CheckImplementationOnly doesn't change the results.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -package-name pkg -swift-version 5 \
// RUN: -enable-experimental-feature CheckImplementationOnly \
// RUN: -verify -verify-ignore-unrelated


/// Library-evolution.
// RUN: %target-swift-frontend -emit-module %t/PublicLib.swift -o %t \
// RUN: -enable-library-evolution
// RUN: %target-swift-frontend -emit-module %t/PackageLib.swift -o %t \
// RUN: -enable-library-evolution
// RUN: %target-swift-frontend -emit-module %t/InternalLib.swift -o %t \
// RUN: -enable-library-evolution
// RUN: -enable-library-evolution -package-name pkg
// RUN: %target-swift-frontend -emit-module %t/FileprivateLib.swift -o %t \
// RUN: -enable-library-evolution
// RUN: %target-swift-frontend -emit-module %t/PrivateLib.swift -o %t \
// RUN: -enable-library-evolution

/// Check diagnostics.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -package-name TestPackage -swift-version 5 \
// RUN: -enable-experimental-feature AccessLevelOnImport -verify -verify-ignore-unrelated
// RUN: -package-name pkg -swift-version 5 \
// RUN: -enable-library-evolution \
// RUN: -verify -verify-ignore-unrelated

/// Check diagnostics with library-evolution.
/// CheckImplementationOnly doesn't change the results.
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
// RUN: -package-name TestPackage -swift-version 5 \
// RUN: -package-name pkg -swift-version 5 \
// RUN: -enable-library-evolution \
// RUN: -enable-experimental-feature AccessLevelOnImport -verify -verify-ignore-unrelated
// RUN: -enable-experimental-feature CheckImplementationOnly \
// RUN: -verify -verify-ignore-unrelated

// REQUIRES: swift_feature_AccessLevelOnImport
// REQUIRES: swift_feature_CheckImplementationOnly

//--- PublicLib.swift
public struct PublicImportType {
Expand All @@ -41,6 +63,8 @@ public struct InternalImportType {
public init() {}
}

package struct InternalImportPackageType {}

//--- FileprivateLib.swift
public struct FileprivateImportType {
public init() {}
Expand Down Expand Up @@ -141,6 +165,9 @@ extension Array: InternalConstrainedExtensionProtoInFileprivate where Element ==
extension InternalImportType {
fileprivate func fileprivateMethod() {}
}
extension InternalImportPackageType { // No error
public func foo() {}
}

private protocol InternalConstrainedExtensionProtoInPrivate {}
extension Array: InternalConstrainedExtensionProtoInPrivate where Element == InternalImportType {}
Expand Down
Loading