Skip to content

Make @preconcurrency import hide sendable variance #42255

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
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
124 changes: 78 additions & 46 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -650,10 +650,17 @@ DiagnosticBehavior SendableCheckContext::defaultDiagnosticBehavior() const {
return defaultSendableDiagnosticBehavior(fromDC->getASTContext().LangOpts);
}

/// Determine whether the given nominal type that is within the current module
/// has an explicit Sendable.
static bool hasExplicitSendableConformance(NominalTypeDecl *nominal) {
/// Determine whether the given nominal type has an explicit Sendable
/// conformance (regardless of its availability).
static bool hasExplicitSendableConformance(NominalTypeDecl *nominal,
bool applyModuleDefault = true) {
ASTContext &ctx = nominal->getASTContext();
auto nominalModule = nominal->getParentModule();

// In a concurrency-checked module, a missing conformance is equivalent to
// an explicitly unavailable one. If we want to apply this rule, do so now.
if (applyModuleDefault && nominalModule->isConcurrencyChecked())
return true;

// Look for any conformance to `Sendable`.
auto proto = ctx.getProtocol(KnownProtocolKind::Sendable);
Expand All @@ -662,7 +669,7 @@ static bool hasExplicitSendableConformance(NominalTypeDecl *nominal) {

// Look for a conformance. If it's present and not (directly) missing,
// we're done.
auto conformance = nominal->getParentModule()->lookupConformance(
auto conformance = nominalModule->lookupConformance(
nominal->getDeclaredInterfaceType(), proto, /*allowMissing=*/true);
return conformance &&
!(isa<BuiltinProtocolConformance>(conformance.getConcrete()) &&
Expand Down Expand Up @@ -706,18 +713,13 @@ static Optional<AttributedImport<ImportedModule>> findImportFor(
/// nominal type.
DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
NominalTypeDecl *nominal) const {
// Determine whether the type was explicitly non-Sendable.
auto nominalModule = nominal->getParentModule();
bool isExplicitlyNonSendable = nominalModule->isConcurrencyChecked() ||
hasExplicitSendableConformance(nominal);

// Determine whether this nominal type is visible via a @preconcurrency
// import.
auto import = findImportFor(nominal, fromDC);
auto sourceFile = fromDC->getParentSourceFile();

// When the type is explicitly non-Sendable...
auto sourceFile = fromDC->getParentSourceFile();
if (isExplicitlyNonSendable) {
if (hasExplicitSendableConformance(nominal)) {
// @preconcurrency imports downgrade the diagnostic to a warning in Swift 6,
if (import && import->options.contains(ImportFlags::Preconcurrency)) {
if (sourceFile)
Expand All @@ -737,7 +739,7 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
if (sourceFile)
sourceFile->setImportUsedPreconcurrency(*import);

return nominalModule->getASTContext().LangOpts.isSwiftVersionAtLeast(6)
return nominal->getASTContext().LangOpts.isSwiftVersionAtLeast(6)
? DiagnosticBehavior::Warning
: DiagnosticBehavior::Ignore;
}
Expand All @@ -755,48 +757,31 @@ DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
return defaultBehavior;
}

/// Produce a diagnostic for a single instance of a non-Sendable type where
/// a Sendable type is required.
static bool diagnoseSingleNonSendableType(
Type type, SendableCheckContext fromContext, SourceLoc loc,
llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {

bool swift::diagnoseSendabilityErrorBasedOn(
NominalTypeDecl *nominal, SendableCheckContext fromContext,
llvm::function_ref<bool(DiagnosticBehavior)> diagnose) {
auto behavior = DiagnosticBehavior::Unspecified;

auto module = fromContext.fromDC->getParentModule();
ASTContext &ctx = module->getASTContext();
auto nominal = type->getAnyNominal();
if (nominal) {
behavior = fromContext.diagnosticBehavior(nominal);
} else {
behavior = fromContext.defaultDiagnosticBehavior();
}

bool wasSuppressed = diagnose(type, behavior);

if (behavior == DiagnosticBehavior::Ignore || wasSuppressed) {
// Don't emit any other diagnostics.
} else if (type->is<FunctionType>()) {
ctx.Diags.diagnose(loc, diag::nonsendable_function_type);
} else if (nominal && nominal->getParentModule() == module) {
// If the nominal type is in the current module, suggest adding
// `Sendable` if it might make sense. Otherwise, just complain.
if (isa<StructDecl>(nominal) || isa<EnumDecl>(nominal)) {
auto note = nominal->diagnose(
diag::add_nominal_sendable_conformance,
nominal->getDescriptiveKind(), nominal->getName());
addSendableFixIt(nominal, note, /*unchecked=*/false);
} else {
nominal->diagnose(
diag::non_sendable_nominal, nominal->getDescriptiveKind(),
nominal->getName());
}
} else if (nominal) {
// Note which nominal type does not conform to `Sendable`.
nominal->diagnose(
diag::non_sendable_nominal, nominal->getDescriptiveKind(),
nominal->getName());
bool wasSuppressed = diagnose(behavior);

bool emittedDiagnostics =
behavior != DiagnosticBehavior::Ignore && !wasSuppressed;

// When the type is explicitly Sendable *or* explicitly non-Sendable, we
// assume it has been audited and `@preconcurrency` is not recommended even
// though it would actually affect the diagnostic.
bool nominalIsImportedAndHasImplicitSendability =
nominal &&
nominal->getParentModule() != fromContext.fromDC->getParentModule() &&
!hasExplicitSendableConformance(nominal);

if (emittedDiagnostics && nominalIsImportedAndHasImplicitSendability) {
// This type was imported from another module; try to find the
// corresponding import.
Optional<AttributedImport<swift::ImportedModule>> import;
Expand All @@ -813,6 +798,8 @@ static bool diagnoseSingleNonSendableType(
import->importLoc.isValid() && sourceFile &&
!sourceFile->hasImportUsedPreconcurrency(*import)) {
SourceLoc importLoc = import->importLoc;
ASTContext &ctx = nominal->getASTContext();

ctx.Diags.diagnose(
importLoc, diag::add_predates_concurrency_import,
ctx.LangOpts.isSwiftVersionAtLeast(6),
Expand All @@ -826,6 +813,51 @@ static bool diagnoseSingleNonSendableType(
return behavior == DiagnosticBehavior::Unspecified && !wasSuppressed;
}

/// Produce a diagnostic for a single instance of a non-Sendable type where
/// a Sendable type is required.
static bool diagnoseSingleNonSendableType(
Type type, SendableCheckContext fromContext, SourceLoc loc,
llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {

auto module = fromContext.fromDC->getParentModule();
auto nominal = type->getAnyNominal();

return diagnoseSendabilityErrorBasedOn(nominal, fromContext,
[&](DiagnosticBehavior behavior) {
bool wasSuppressed = diagnose(type, behavior);

// Don't emit the following notes if we didn't have any diagnostics to
// attach them to.
if (wasSuppressed || behavior == DiagnosticBehavior::Ignore)
return true;

if (type->is<FunctionType>()) {
module->getASTContext().Diags
.diagnose(loc, diag::nonsendable_function_type);
} else if (nominal && nominal->getParentModule() == module) {
// If the nominal type is in the current module, suggest adding
// `Sendable` if it might make sense. Otherwise, just complain.
if (isa<StructDecl>(nominal) || isa<EnumDecl>(nominal)) {
auto note = nominal->diagnose(
diag::add_nominal_sendable_conformance,
nominal->getDescriptiveKind(), nominal->getName());
addSendableFixIt(nominal, note, /*unchecked=*/false);
} else {
nominal->diagnose(
diag::non_sendable_nominal, nominal->getDescriptiveKind(),
nominal->getName());
}
} else if (nominal) {
// Note which nominal type does not conform to `Sendable`.
nominal->diagnose(
diag::non_sendable_nominal, nominal->getDescriptiveKind(),
nominal->getName());
}

return false;
});
}

bool swift::diagnoseNonSendableTypes(
Type type, SendableCheckContext fromContext, SourceLoc loc,
llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose) {
Expand Down Expand Up @@ -1036,7 +1068,7 @@ void swift::diagnoseMissingExplicitSendable(NominalTypeDecl *nominal) {
return;

// If the conformance is explicitly stated, do nothing.
if (hasExplicitSendableConformance(nominal))
if (hasExplicitSendableConformance(nominal, /*applyModuleDefault=*/false))
return;

// Diagnose it.
Expand Down
28 changes: 25 additions & 3 deletions lib/Sema/TypeCheckConcurrency.h
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,10 @@ struct SendableCheckContext {
///
/// \param diagnose Emit a diagnostic indicating that the current type
/// is non-Sendable, with the suggested behavior limitation. Returns \c true
/// if an error was produced.
/// if it did not emit any diagnostics.
///
/// \returns \c true if any diagnostics were produced, \c false otherwise.
/// \returns \c true if any errors were produced, \c false if no diagnostics or
/// only warnings and notes were produced.
bool diagnoseNonSendableTypes(
Type type, SendableCheckContext fromContext, SourceLoc loc,
llvm::function_ref<bool(Type, DiagnosticBehavior)> diagnose);
Expand All @@ -370,7 +371,8 @@ namespace detail {
/// Diagnose any non-Sendable types that occur within the given type, using
/// the given diagnostic.
///
/// \returns \c true if any errors were produced, \c false otherwise.
/// \returns \c true if any errors were produced, \c false if no diagnostics or
/// only warnings and notes were produced.
template<typename ...DiagArgs>
bool diagnoseNonSendableTypes(
Type type, SendableCheckContext fromContext, SourceLoc loc,
Expand All @@ -389,6 +391,26 @@ bool diagnoseNonSendableTypes(
});
}

/// Diagnose this sendability error with behavior based on the import of
/// \p nominal . For instance, depending on how \p nominal is imported into
/// \p fromContext , the diagnostic behavior limitation may be lower or the
/// compiler might emit a fix-it adding \c \@preconcurrency to the \c import .
///
/// \param nominal The declaration whose import we should check, or \c nullptr
/// to get behavior for a non-nominal type.
///
/// \param fromContext The context where the error will be emitted.
///
/// \param diagnose Emit a diagnostic indicating a sendability problem,
/// with the suggested behavior limitation. Returns \c true
/// if it did \em not emit any diagnostics.
///
/// \returns \c true if any errors were produced, \c false if no diagnostics or
/// only warnings and notes were produced.
bool diagnoseSendabilityErrorBasedOn(
NominalTypeDecl *nominal, SendableCheckContext fromContext,
llvm::function_ref<bool(DiagnosticBehavior)> diagnose);

/// Given a set of custom attributes, pick out the global actor attributes
/// and perform any necessary resolution and diagnostics, returning the
/// global actor attribute and type it refers to (or \c None).
Expand Down
37 changes: 27 additions & 10 deletions lib/Sema/TypeCheckDeclOverride.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//===----------------------------------------------------------------------===//
#include "MiscDiagnostics.h"
#include "TypeCheckAvailability.h"
#include "TypeCheckConcurrency.h"
#include "TypeCheckDecl.h"
#include "TypeCheckObjC.h"
#include "TypeChecker.h"
Expand Down Expand Up @@ -548,12 +549,20 @@ static void diagnoseGeneralOverrideFailure(ValueDecl *decl,
diags.diagnose(decl, diag::override_multiple_decls_base,
decl->getName());
break;
case OverrideCheckingAttempt::MismatchedSendability:
// FIXME: suppress if any matches brought in via @preconcurrency import?
diags.diagnose(decl, diag::override_sendability_mismatch,
decl->getName())
.warnUntilSwiftVersion(6);
case OverrideCheckingAttempt::MismatchedSendability: {
SendableCheckContext fromContext(decl->getDeclContext(),
SendableCheck::Explicit);
auto baseDeclClass =
decl->getOverriddenDecl()->getDeclContext()->getSelfClassDecl();

diagnoseSendabilityErrorBasedOn(baseDeclClass, fromContext,
[&](DiagnosticBehavior limit) {
diags.diagnose(decl, diag::override_sendability_mismatch, decl->getName())
.limitBehavior(limit);
return false;
});
break;
}
case OverrideCheckingAttempt::BaseName:
diags.diagnose(decl, diag::override_multiple_decls_arg_mismatch,
decl->getName());
Expand Down Expand Up @@ -1288,11 +1297,19 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
return true;

if (attempt == OverrideCheckingAttempt::MismatchedSendability) {
// FIXME: suppress if any matches brought in via @preconcurrency import?
diags.diagnose(decl, diag::override_sendability_mismatch,
decl->getName())
.warnUntilSwiftVersion(6);
diags.diagnose(baseDecl, diag::overridden_here);
SendableCheckContext fromContext(decl->getDeclContext(),
SendableCheck::Explicit);
auto baseDeclClass = baseDecl->getDeclContext()->getSelfClassDecl();

diagnoseSendabilityErrorBasedOn(baseDeclClass, fromContext,
[&](DiagnosticBehavior limit) {
diags.diagnose(decl, diag::override_sendability_mismatch,
decl->getName())
.limitBehavior(limit);
diags.diagnose(baseDecl, diag::overridden_here)
.limitBehavior(limit);
return false;
});
}
// Catch-all to make sure we don't silently accept something we shouldn't.
else if (attempt != OverrideCheckingAttempt::PerfectMatch) {
Expand Down
37 changes: 25 additions & 12 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4154,19 +4154,32 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
requirement->getName());
});
}
if (best.Kind == MatchKind::RequiresNonSendable)
diagnoseOrDefer(requirement, getASTContext().isSwiftVersionAtLeast(6),
[this, requirement, witness]
(NormalProtocolConformance *conformance) {
auto &diags = DC->getASTContext().Diags;
if (best.Kind == MatchKind::RequiresNonSendable) {
SendableCheckContext sendFrom(witness->getDeclContext(),
SendableCheck::Explicit);

diags.diagnose(getLocForDiagnosingWitness(conformance, witness),
diag::witness_not_as_sendable,
witness->getDescriptiveKind(), witness->getName(),
conformance->getProtocol()->getName())
.warnUntilSwiftVersion(6);
diags.diagnose(requirement, diag::less_sendable_reqt_here);
});
auto behavior = sendFrom.diagnosticBehavior(Conformance->getProtocol());
if (behavior != DiagnosticBehavior::Ignore) {
bool isError = behavior < DiagnosticBehavior::Warning;

diagnoseOrDefer(requirement, isError,
[this, requirement, witness, sendFrom](
NormalProtocolConformance *conformance) {
diagnoseSendabilityErrorBasedOn(conformance->getProtocol(), sendFrom,
[&](DiagnosticBehavior limit) {
auto &diags = DC->getASTContext().Diags;
diags.diagnose(getLocForDiagnosingWitness(conformance, witness),
diag::witness_not_as_sendable,
witness->getDescriptiveKind(), witness->getName(),
conformance->getProtocol()->getName())
.limitBehavior(limit);
diags.diagnose(requirement, diag::less_sendable_reqt_here)
.limitBehavior(limit);
return false;
});
});
}
}

auto check = checkWitness(requirement, best);

Expand Down
12 changes: 10 additions & 2 deletions test/Concurrency/Inputs/NonStrictModule.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
public class NonStrictClass { }

public struct NonStrictStruct { }

open class NonStrictClass {
open func send(_ body: @Sendable () -> Void) {}
open func dontSend(_ body: () -> Void) {}
}

public protocol NonStrictProtocol {
func send(_ body: @Sendable () -> Void)
func dontSend(_ body: () -> Void)
}
10 changes: 10 additions & 0 deletions test/Concurrency/Inputs/StrictModule.swift
Original file line number Diff line number Diff line change
@@ -1 +1,11 @@
public struct StrictStruct: Hashable { }

open class StrictClass {
open func send(_ body: @Sendable () -> Void) {}
open func dontSend(_ body: () -> Void) {}
}

public protocol StrictProtocol {
func send(_ body: @Sendable () -> Void)
func dontSend(_ body: () -> Void)
}
Loading