Skip to content

Make overloads and witnesses permit @Sendable variance + Make @preconcurrency import hide sendable variance #42273

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
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
13 changes: 13 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -2155,6 +2155,13 @@ ERROR(witness_not_accessible_proto,none,
"requirement in %select{private|fileprivate|internal|public|%error}4 protocol "
"%5",
(RequirementKind, DeclName, bool, AccessLevel, AccessLevel, Identifier))
ERROR(witness_not_as_sendable,none,
"sendability of function types in %0 %1 does not match requirement in "
"protocol %2",
(DescriptiveDeclKind, DeclName, Identifier))
NOTE(less_sendable_reqt_here,none,
"expected sendability to match requirement here",
())
ERROR(witness_not_accessible_type,none,
"%select{initializer %1|method %1|%select{|setter for }2property %1"
"|subscript%select{| setter}2}0 must be as accessible as its enclosing "
Expand Down Expand Up @@ -2303,6 +2310,9 @@ NOTE(ambiguous_witnesses_type,none,
"multiple matching types named %0", (Identifier))
NOTE(protocol_witness_exact_match,none,
"candidate exactly matches%0", (StringRef))
NOTE(protocol_witness_non_sendable,none,
"candidate matches except for closure sendability%0%select{; this will be "
"an error in Swift 6|}1", (StringRef, bool))
NOTE(protocol_witness_renamed,none,
"rename to %0 to satisfy this requirement%1", (DeclName, StringRef))
NOTE(protocol_witness_kind_conflict,none,
Expand Down Expand Up @@ -2602,6 +2612,9 @@ WARNING(generic_param_usable_from_inline_warn,none,
ERROR(override_multiple_decls_base,none,
"declaration %0 cannot override more than one superclass declaration",
(DeclName))
ERROR(override_sendability_mismatch,none,
"declaration %0 has a type with different sendability from any potential "
"overrides", (DeclName))
ERROR(override_multiple_decls_arg_mismatch,none,
"declaration %0 has different argument labels from any potential "
"overrides", (DeclName))
Expand Down
4 changes: 3 additions & 1 deletion include/swift/AST/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,9 @@ enum class TypeMatchFlags {
/// to be non-escaping, but Swift currently does not.
IgnoreNonEscapingForOptionalFunctionParam = 1 << 4,
/// Allow compatible opaque archetypes.
AllowCompatibleOpaqueTypeArchetypes = 1 << 5
AllowCompatibleOpaqueTypeArchetypes = 1 << 5,
/// Ignore the @Sendable attributes on functions when matching types.
IgnoreFunctionSendability = 1 << 6,
};
using TypeMatchOptions = OptionSet<TypeMatchFlags>;

Expand Down
10 changes: 10 additions & 0 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3167,6 +3167,16 @@ static bool matchesFunctionType(CanAnyFunctionType fn1, CanAnyFunctionType fn2,
matchMode.contains(TypeMatchFlags::AllowABICompatible))) {
ext1 = ext1.withThrows(true);
}

// Removing '@Sendable' is ABI-compatible because there's nothing wrong with
// a function being sendable when it doesn't need to be.
if (!ext2.isSendable())
ext1 = ext1.withConcurrent(false);
}

if (matchMode.contains(TypeMatchFlags::IgnoreFunctionSendability)) {
ext1 = ext1.withConcurrent(false);
ext2 = ext2.withConcurrent(false);
}

// If specified, allow an escaping function parameter to override a
Expand Down
124 changes: 78 additions & 46 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -770,10 +770,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 @@ -782,7 +789,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 @@ -826,18 +833,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 @@ -857,7 +859,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 @@ -875,48 +877,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 @@ -933,6 +918,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 @@ -946,6 +933,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 @@ -1156,7 +1188,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 @@ -313,9 +313,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 @@ -330,7 +331,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 @@ -349,6 +351,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
39 changes: 37 additions & 2 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 @@ -518,6 +519,7 @@ static bool noteFixableMismatchedTypes(ValueDecl *decl, const ValueDecl *base) {
namespace {
enum class OverrideCheckingAttempt {
PerfectMatch,
MismatchedSendability,
MismatchedOptional,
MismatchedTypes,
BaseName,
Expand Down Expand Up @@ -547,6 +549,20 @@ static void diagnoseGeneralOverrideFailure(ValueDecl *decl,
diags.diagnose(decl, diag::override_multiple_decls_base,
decl->getName());
break;
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 @@ -886,6 +902,7 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
DeclName name;
switch (attempt) {
case OverrideCheckingAttempt::PerfectMatch:
case OverrideCheckingAttempt::MismatchedSendability:
case OverrideCheckingAttempt::MismatchedOptional:
case OverrideCheckingAttempt::MismatchedTypes:
name = decl->getName();
Expand Down Expand Up @@ -976,6 +993,8 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
matchMode |= TypeMatchFlags::AllowNonOptionalForIUOParam;
matchMode |= TypeMatchFlags::IgnoreNonEscapingForOptionalFunctionParam;
}
if (attempt == OverrideCheckingAttempt::MismatchedSendability)
matchMode |= TypeMatchFlags::IgnoreFunctionSendability;

auto declFnTy = getDeclComparisonType()->getAs<AnyFunctionType>();
auto parentDeclTy = getMemberTypeForComparison(parentDecl, decl);
Expand Down Expand Up @@ -1277,8 +1296,23 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
if (emittedMatchError)
return true;

if (attempt == OverrideCheckingAttempt::MismatchedSendability) {
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.
if (attempt != OverrideCheckingAttempt::PerfectMatch) {
else if (attempt != OverrideCheckingAttempt::PerfectMatch) {
OverrideMatch match{decl, /*isExact=*/false};
diagnoseGeneralOverrideFailure(decl, match, attempt);
}
Expand Down Expand Up @@ -1374,11 +1408,12 @@ bool swift::checkOverrides(ValueDecl *decl) {
switch (attempt) {
case OverrideCheckingAttempt::PerfectMatch:
break;
case OverrideCheckingAttempt::MismatchedOptional:
case OverrideCheckingAttempt::MismatchedSendability:
// Don't keep looking if the user didn't indicate it's an override.
if (!decl->getAttrs().hasAttribute<OverrideAttr>())
return false;
break;
case OverrideCheckingAttempt::MismatchedOptional:
case OverrideCheckingAttempt::MismatchedTypes:
break;
case OverrideCheckingAttempt::BaseName:
Expand Down
Loading