Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
24 changes: 24 additions & 0 deletions include/swift/AST/DiagnosticEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,30 @@ namespace swift {
InFlightDiagnostic &limitBehaviorUntilSwiftVersion(
DiagnosticBehavior limit, unsigned majorVersion);

/// Limits the diagnostic behavior to \c limit accordingly if
/// preconcurrency applies. Otherwise, the behavior limit only applies
/// prior to the given language mode.
///
/// `@preconcurrency` applied to a nominal declaration or an import
/// statement will limit concurrency diagnostic behavior based on the
/// strict concurrency checking level. Under minimal checking,
/// preconcurrency will suppress the diagnostics. With strict concurrency
/// checking, including when building with the Swift 6 language mode,
/// preconcurrency errors are downgraded to warnings. This allows libraries
/// to stage in concurrency annotations even after their clients have
/// migrated to Swift 6 using `@preconcurrency` alongside the newly added
/// `@Sendable` or `@MainActor` annotations.
InFlightDiagnostic
&limitBehaviorWithPreconcurrency(DiagnosticBehavior limit,
bool preconcurrency,
unsigned languageMode = 6) {
if (preconcurrency) {
return limitBehavior(limit);
}

return limitBehaviorUntilSwiftVersion(limit, languageMode);
}

/// Limit the diagnostic behavior to warning until the specified version.
///
/// This helps stage in fixes for stricter diagnostics as warnings
Expand Down
12 changes: 9 additions & 3 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,16 @@ getConcurrencyFixBehavior(ConstraintSystem &cs, ConstraintKind constraintKind,

// For a @preconcurrency callee outside of a strict concurrency
// context, ignore.
if (cs.hasPreconcurrencyCallee(locator) &&
!contextRequiresStrictConcurrencyChecking(
cs.DC, GetClosureType{cs}, ClosureIsolatedByPreconcurrency{cs}))
if (cs.hasPreconcurrencyCallee(locator)) {
// Preconcurrency failures are always downgraded to warnings, even in
// Swift 6 mode.
if (contextRequiresStrictConcurrencyChecking(
cs.DC, GetClosureType{cs}, ClosureIsolatedByPreconcurrency{cs})) {
return FixBehavior::DowngradeToWarning;
}

return FixBehavior::Suppress;
}

// Otherwise, warn until Swift 6.
if (!cs.getASTContext().LangOpts.isSwiftVersionAtLeast(6))
Expand Down
37 changes: 25 additions & 12 deletions lib/Sema/TypeCheckAvailability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ static bool diagnoseSubstitutionMapAvailability(
SourceLoc loc, SubstitutionMap subs, const ExportContext &where,
Type depTy = Type(), Type replacementTy = Type(),
bool warnIfConformanceUnavailablePreSwift6 = false,
bool suppressParameterizationCheckForOptional = false);
bool suppressParameterizationCheckForOptional = false,
bool preconcurrency = false);

/// Diagnose uses of unavailable declarations in types.
static bool
Expand Down Expand Up @@ -3990,8 +3991,12 @@ bool ExprAvailabilityWalker::diagnoseDeclRefAvailability(
}

if (R.isValid()) {
if (diagnoseSubstitutionMapAvailability(R.Start, declRef.getSubstitutions(),
Where)) {
if (diagnoseSubstitutionMapAvailability(
R.Start, declRef.getSubstitutions(), Where,
Type(), Type(),
/*warnIfConformanceUnavailablePreSwift6*/false,
/*suppressParameterizationCheckForOptional*/false,
/*preconcurrency*/D->preconcurrency())) {
return true;
}
}
Expand Down Expand Up @@ -4489,7 +4494,8 @@ class ProblematicTypeFinder : public TypeDeclFinder {
/*depTy=*/Type(),
/*replacementTy=*/Type(),
/*warnIfConformanceUnavailablePreSwift6=*/false,
/*suppressParameterizationCheckForOptional=*/ty->isOptional());
/*suppressParameterizationCheckForOptional=*/ty->isOptional(),
/*preconcurrency*/ty->getAnyNominal()->preconcurrency());
return Action::Continue;
}

Expand Down Expand Up @@ -4539,17 +4545,19 @@ void swift::diagnoseTypeAvailability(const TypeRepr *TR, Type T, SourceLoc loc,
}

static void diagnoseMissingConformance(
SourceLoc loc, Type type, ProtocolDecl *proto, const DeclContext *fromDC) {
SourceLoc loc, Type type, ProtocolDecl *proto, const DeclContext *fromDC,
bool preconcurrency) {
assert(proto->isSpecificProtocol(KnownProtocolKind::Sendable));
diagnoseMissingSendableConformance(loc, type, fromDC);
diagnoseMissingSendableConformance(loc, type, fromDC, preconcurrency);
}

bool
swift::diagnoseConformanceAvailability(SourceLoc loc,
ProtocolConformanceRef conformance,
const ExportContext &where,
Type depTy, Type replacementTy,
bool warnIfConformanceUnavailablePreSwift6) {
bool warnIfConformanceUnavailablePreSwift6,
bool preconcurrency) {
assert(!where.isImplicit());

if (conformance.isInvalid() || conformance.isAbstract())
Expand All @@ -4561,7 +4569,8 @@ swift::diagnoseConformanceAvailability(SourceLoc loc,
for (auto patternConf : pack->getPatternConformances()) {
diagnosed |= diagnoseConformanceAvailability(
loc, patternConf, where, depTy, replacementTy,
warnIfConformanceUnavailablePreSwift6);
warnIfConformanceUnavailablePreSwift6,
preconcurrency);
}
return diagnosed;
}
Expand All @@ -4581,7 +4590,8 @@ swift::diagnoseConformanceAvailability(SourceLoc loc,
if (auto builtinConformance = dyn_cast<BuiltinProtocolConformance>(rootConf)){
if (builtinConformance->isMissing()) {
diagnoseMissingConformance(loc, builtinConformance->getType(),
builtinConformance->getProtocol(), DC);
builtinConformance->getProtocol(), DC,
preconcurrency);
}
}

Expand Down Expand Up @@ -4640,7 +4650,8 @@ swift::diagnoseConformanceAvailability(SourceLoc loc,
SubstitutionMap subConformanceSubs = concreteConf->getSubstitutionMap();
if (diagnoseSubstitutionMapAvailability(loc, subConformanceSubs, where,
depTy, replacementTy,
warnIfConformanceUnavailablePreSwift6))
warnIfConformanceUnavailablePreSwift6,
preconcurrency))
return true;

return false;
Expand All @@ -4649,12 +4660,14 @@ swift::diagnoseConformanceAvailability(SourceLoc loc,
bool diagnoseSubstitutionMapAvailability(
SourceLoc loc, SubstitutionMap subs, const ExportContext &where, Type depTy,
Type replacementTy, bool warnIfConformanceUnavailablePreSwift6,
bool suppressParameterizationCheckForOptional) {
bool suppressParameterizationCheckForOptional,
bool preconcurrency) {
bool hadAnyIssues = false;
for (ProtocolConformanceRef conformance : subs.getConformances()) {
if (diagnoseConformanceAvailability(loc, conformance, where,
depTy, replacementTy,
warnIfConformanceUnavailablePreSwift6))
warnIfConformanceUnavailablePreSwift6,
preconcurrency))
hadAnyIssues = true;
}

Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/TypeCheckAvailability.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ diagnoseConformanceAvailability(SourceLoc loc,
const ExportContext &context,
Type depTy=Type(),
Type replacementTy=Type(),
bool warnIfConformanceUnavailablePreSwift6 = false);
bool warnIfConformanceUnavailablePreSwift6 = false,
bool preconcurrency = false);

/// Diagnose uses of unavailable declarations. Returns true if a diagnostic
/// was emitted.
Expand Down
74 changes: 56 additions & 18 deletions lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,10 @@ static bool shouldDiagnoseExistingDataRaces(const DeclContext *dc) {
});
}

bool SendableCheckContext::isExplicitSendableConformance() const {
bool SendableCheckContext::warnInMinimalChecking() const {
if (preconcurrencyContext)
return false;

if (!conformanceCheck)
return false;

Expand All @@ -797,7 +800,7 @@ bool SendableCheckContext::isExplicitSendableConformance() const {
DiagnosticBehavior SendableCheckContext::defaultDiagnosticBehavior() const {
// If we're not supposed to diagnose existing data races from this context,
// ignore the diagnostic entirely.
if (!isExplicitSendableConformance() &&
if (!warnInMinimalChecking() &&
!shouldDiagnoseExistingDataRaces(fromDC))
return DiagnosticBehavior::Ignore;

Expand All @@ -816,9 +819,7 @@ SendableCheckContext::implicitSendableDiagnosticBehavior() const {
LLVM_FALLTHROUGH;

case StrictConcurrency::Minimal:
// Explicit Sendable conformances always diagnose, even when strict
// strict checking is disabled.
if (isExplicitSendableConformance())
if (warnInMinimalChecking())
return DiagnosticBehavior::Warning;

return DiagnosticBehavior::Ignore;
Expand All @@ -832,6 +833,13 @@ SendableCheckContext::implicitSendableDiagnosticBehavior() const {
/// nominal type.
DiagnosticBehavior SendableCheckContext::diagnosticBehavior(
NominalTypeDecl *nominal) const {
// If we're in a preconcurrency context, don't override the default behavior
// based on explicit conformances. For example, a @preconcurrency @Sendable
// closure should not warn about an explicitly unavailable Sendable
// conformance in minimal checking.
if (preconcurrencyContext)
return defaultDiagnosticBehavior();

if (hasExplicitSendableConformance(nominal))
return DiagnosticBehavior::Warning;

Expand Down Expand Up @@ -1229,9 +1237,10 @@ bool swift::diagnoseNonSendableTypesInReference(
}

void swift::diagnoseMissingSendableConformance(
SourceLoc loc, Type type, const DeclContext *fromDC) {
SourceLoc loc, Type type, const DeclContext *fromDC, bool preconcurrency) {
SendableCheckContext sendableContext(fromDC, preconcurrency);
diagnoseNonSendableTypes(
type, fromDC, /*inDerivedConformance*/Type(),
type, sendableContext, /*inDerivedConformance*/Type(),
loc, diag::non_sendable_type);
}

Expand Down Expand Up @@ -2759,11 +2768,16 @@ namespace {
continue;

auto *closure = localFunc.getAbstractClosureExpr();
auto *explicitClosure = dyn_cast_or_null<ClosureExpr>(closure);

bool preconcurrency = false;
if (explicitClosure) {
preconcurrency = explicitClosure->isIsolatedByPreconcurrency();
}

// Diagnose a `self` capture inside an escaping `sending`
// `@Sendable` closure in a deinit, which almost certainly
// means `self` would escape deinit at runtime.
auto *explicitClosure = dyn_cast_or_null<ClosureExpr>(closure);
auto *dc = getDeclContext();
if (explicitClosure && isa<DestructorDecl>(dc) &&
!explicitClosure->getType()->isNoEscape() &&
Expand All @@ -2773,7 +2787,8 @@ namespace {
if (var && var->isSelfParameter()) {
ctx.Diags.diagnose(explicitClosure->getLoc(),
diag::self_capture_deinit_task)
.warnUntilSwiftVersion(6);
.limitBehaviorWithPreconcurrency(DiagnosticBehavior::Warning,
preconcurrency);
}
}

Expand Down Expand Up @@ -2801,6 +2816,9 @@ namespace {
if (type->hasError())
continue;

SendableCheckContext sendableContext(getDeclContext(),
preconcurrency);

if (closure && closure->isImplicit()) {
auto *patternBindingDecl = getTopPatternBindingDecl();
if (patternBindingDecl && patternBindingDecl->isAsyncLet()) {
Expand All @@ -2811,20 +2829,20 @@ namespace {

// Fallback to a generic implicit capture missing sendable
// conformance diagnostic.
diagnoseNonSendableTypes(type, getDeclContext(),
diagnoseNonSendableTypes(type, sendableContext,
/*inDerivedConformance*/Type(),
capture.getLoc(),
diag::implicit_non_sendable_capture,
decl->getName());
} else if (fnType->isSendable()) {
diagnoseNonSendableTypes(type, getDeclContext(),
diagnoseNonSendableTypes(type, sendableContext,
/*inDerivedConformance*/Type(),
capture.getLoc(),
diag::non_sendable_capture,
decl->getName(),
/*closure=*/closure != nullptr);
} else {
diagnoseNonSendableTypes(type, getDeclContext(),
diagnoseNonSendableTypes(type, sendableContext,
/*inDerivedConformance*/Type(),
capture.getLoc(),
diag::non_sendable_isolated_capture,
Expand Down Expand Up @@ -4066,7 +4084,12 @@ namespace {
if (!mayExecuteConcurrentlyWith(dc, findCapturedDeclContext(value)))
return false;

SendableCheckContext sendableBehavior(dc);
bool preconcurrency = false;
if (auto *closure = dyn_cast<ClosureExpr>(dc)) {
preconcurrency = closure->isIsolatedByPreconcurrency();
}

SendableCheckContext sendableBehavior(dc, preconcurrency);
auto limit = sendableBehavior.defaultDiagnosticBehavior();

// Check whether this is a local variable, in which case we can
Expand Down Expand Up @@ -4096,7 +4119,7 @@ namespace {
ctx.Diags
.diagnose(loc, diag::concurrent_access_of_inout_param,
param->getName())
.limitBehaviorUntilSwiftVersion(limit, 6);
.limitBehaviorWithPreconcurrency(limit, preconcurrency);
return true;
}
}
Expand All @@ -4111,7 +4134,7 @@ namespace {
loc, diag::concurrent_access_of_local_capture,
parent.dyn_cast<LoadExpr *>(),
var)
.limitBehaviorUntilSwiftVersion(limit, 6);
.limitBehaviorWithPreconcurrency(limit, preconcurrency);
return true;
}

Expand All @@ -4121,7 +4144,7 @@ namespace {

func->diagnose(diag::local_function_executed_concurrently, func)
.fixItInsert(func->getAttributeInsertionLoc(false), "@Sendable ")
.limitBehaviorUntilSwiftVersion(limit, 6);
.limitBehaviorWithPreconcurrency(limit, preconcurrency);

// Add the @Sendable attribute implicitly, so we don't diagnose
// again.
Expand All @@ -4131,7 +4154,8 @@ namespace {
}

// Concurrent access to some other local.
ctx.Diags.diagnose(loc, diag::concurrent_access_local, value);
ctx.Diags.diagnose(loc, diag::concurrent_access_local, value)
.limitBehaviorWithPreconcurrency(limit, preconcurrency);
value->diagnose(
diag::kind_declared_here, value->getDescriptiveKind());
return true;
Expand Down Expand Up @@ -6194,7 +6218,8 @@ bool swift::contextRequiresStrictConcurrencyChecking(
const DeclContext *dc,
llvm::function_ref<Type(const AbstractClosureExpr *)> getType,
llvm::function_ref<bool(const ClosureExpr *)> isolatedByPreconcurrency) {
switch (dc->getASTContext().LangOpts.StrictConcurrencyLevel) {
auto concurrencyLevel = dc->getASTContext().LangOpts.StrictConcurrencyLevel;
switch (concurrencyLevel) {
case StrictConcurrency::Complete:
return true;

Expand All @@ -6214,7 +6239,20 @@ bool swift::contextRequiresStrictConcurrencyChecking(

// Don't take any more cues if this only got its type information by
// being provided to a `@preconcurrency` operation.
//
// FIXME: contextRequiresStrictConcurrencyChecking is called from
// within the constraint system, but closures are only set to be isolated
// by preconcurrency in solution application because it's dependent on
// overload resolution. The constraint system either needs to check its
// own state on the current path, or not make type inference decisions based
// on concurrency checking level.
if (isolatedByPreconcurrency(explicitClosure)) {
Comment on lines +6243 to 6249
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why isolatedByPreconcurrency is a closure that's passed in, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and it looks like ClosureIsolatedByPreconcurrency (used by constraint system calls) does check the constraint system state, but I ran into a case while debugging where this line of code returned an unexpected result mid constraint solving. It didn't seem to be causing any issues for the preconcurrency downgrade behavior, so I left this comment for myself to figure out what happened there after I land this.

// If we're in minimal checking, preconcurrency always suppresses
// diagnostics. Targeted checking will still produce diagnostics if
// the outer context has adopted explicit concurrency features.
if (concurrencyLevel == StrictConcurrency::Minimal)
return false;

dc = dc->getParent();
continue;
}
Expand Down
Loading