Skip to content

Commit 365f0af

Browse files
DougGregorDougGregor
committed
Downgrade concurrency-related function type errors in existing code
When in "existing" Swift code that is Swift 5.x and has not adopted concurrency, allow mismatches in function types that only involve ABI-neutral concurrency changes (e.g., adding `@Sendable` or removing a global actor) by downgrading the diagnostic to a warning. This improves the story for incremental adoption of concurrency in an existing code base. As part of this, generalize the facility for downgrading an error to a warning when performing diagnostics in the constraint solver, using the new diagnostic behavior limits rather than duplicating diagnostics.
1 parent f9636af commit 365f0af

File tree

9 files changed

+89
-41
lines changed

9 files changed

+89
-41
lines changed

‎include/swift/Sema/CSFix.h

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -650,9 +650,9 @@ class ContextualMismatch : public ConstraintFix {
650650
Type LHS, RHS;
651651

652652
ContextualMismatch(ConstraintSystem &cs, Type lhs, Type rhs,
653-
ConstraintLocator *locator)
654-
: ConstraintFix(cs, FixKind::ContextualMismatch, locator), LHS(lhs),
655-
RHS(rhs) {}
653+
ConstraintLocator *locator, bool warning)
654+
: ConstraintFix(cs, FixKind::ContextualMismatch, locator, warning),
655+
LHS(lhs), RHS(rhs) {}
656656

657657
protected:
658658
ContextualMismatch(ConstraintSystem &cs, FixKind kind, Type lhs, Type rhs,
@@ -741,9 +741,9 @@ class MarkExplicitlyEscaping final : public ContextualMismatch {
741741
/// Mark function type as being part of a global actor.
742742
class MarkGlobalActorFunction final : public ContextualMismatch {
743743
MarkGlobalActorFunction(ConstraintSystem &cs, Type lhs, Type rhs,
744-
ConstraintLocator *locator)
744+
ConstraintLocator *locator, bool warning)
745745
: ContextualMismatch(cs, FixKind::MarkGlobalActorFunction, lhs, rhs,
746-
locator) {
746+
locator, warning) {
747747
}
748748

749749
public:
@@ -752,7 +752,8 @@ class MarkGlobalActorFunction final : public ContextualMismatch {
752752
bool diagnose(const Solution &solution, bool asNote = false) const override;
753753

754754
static MarkGlobalActorFunction *create(ConstraintSystem &cs, Type lhs,
755-
Type rhs, ConstraintLocator *locator);
755+
Type rhs, ConstraintLocator *locator,
756+
bool warning);
756757

757758
static bool classof(ConstraintFix *fix) {
758759
return fix->getKind() == FixKind::MarkGlobalActorFunction;
@@ -787,9 +788,10 @@ class ForceOptional final : public ContextualMismatch {
787788
/// function types, repair it by adding @Sendable attribute.
788789
class AddSendableAttribute final : public ContextualMismatch {
789790
AddSendableAttribute(ConstraintSystem &cs, FunctionType *fromType,
790-
FunctionType *toType, ConstraintLocator *locator)
791+
FunctionType *toType, ConstraintLocator *locator,
792+
bool warning)
791793
: ContextualMismatch(cs, FixKind::AddSendableAttribute, fromType, toType,
792-
locator) {
794+
locator, warning) {
793795
assert(fromType->isSendable() != toType->isSendable());
794796
}
795797

@@ -801,7 +803,8 @@ class AddSendableAttribute final : public ContextualMismatch {
801803
static AddSendableAttribute *create(ConstraintSystem &cs,
802804
FunctionType *fromType,
803805
FunctionType *toType,
804-
ConstraintLocator *locator);
806+
ConstraintLocator *locator,
807+
bool warning);
805808

806809
static bool classof(ConstraintFix *fix) {
807810
return fix->getKind() == FixKind::AddSendableAttribute;

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@ template <typename... ArgTypes>
102102
InFlightDiagnostic
103103
FailureDiagnostic::emitDiagnosticAt(ArgTypes &&... Args) const {
104104
auto &DE = getASTContext().Diags;
105-
return DE.diagnose(std::forward<ArgTypes>(Args)...);
105+
auto behavior = isWarning ? DiagnosticBehavior::Warning
106+
: DiagnosticBehavior::Unspecified;
107+
return std::move(DE.diagnose(std::forward<ArgTypes>(Args)...)
108+
.limitBehavior(behavior));
106109
}
107110

108111
Expr *FailureDiagnostic::findParentExpr(const Expr *subExpr) const {

‎lib/Sema/CSDiagnostics.h

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,17 @@ class FunctionArgApplyInfo;
4242
class FailureDiagnostic {
4343
const Solution &S;
4444
ConstraintLocator *Locator;
45+
bool isWarning;
4546

4647
public:
47-
FailureDiagnostic(const Solution &solution, ConstraintLocator *locator)
48-
: S(solution), Locator(locator) {}
48+
FailureDiagnostic(const Solution &solution, ConstraintLocator *locator,
49+
bool isWarning = false)
50+
: S(solution), Locator(locator), isWarning(isWarning) {}
4951

50-
FailureDiagnostic(const Solution &solution, ASTNode anchor)
51-
: FailureDiagnostic(solution, solution.getConstraintLocator(anchor)) {}
52+
FailureDiagnostic(const Solution &solution, ASTNode anchor,
53+
bool isWarning = false)
54+
: FailureDiagnostic(solution, solution.getConstraintLocator(anchor),
55+
isWarning) { }
5256

5357
virtual ~FailureDiagnostic();
5458

@@ -601,20 +605,21 @@ class ContextualFailure : public FailureDiagnostic {
601605

602606
public:
603607
ContextualFailure(const Solution &solution, Type lhs, Type rhs,
604-
ConstraintLocator *locator)
608+
ConstraintLocator *locator, bool isWarning = false)
605609
: ContextualFailure(
606610
solution,
607611
locator->isForContextualType()
608612
? locator->castLastElementTo<LocatorPathElt::ContextualType>()
609613
.getPurpose()
610614
: solution.getConstraintSystem().getContextualTypePurpose(
611615
locator->getAnchor()),
612-
lhs, rhs, locator) {}
616+
lhs, rhs, locator, isWarning) {}
613617

614618
ContextualFailure(const Solution &solution, ContextualTypePurpose purpose,
615-
Type lhs, Type rhs, ConstraintLocator *locator)
616-
: FailureDiagnostic(solution, locator), CTP(purpose), RawFromType(lhs),
617-
RawToType(rhs) {
619+
Type lhs, Type rhs, ConstraintLocator *locator,
620+
bool isWarning = false)
621+
: FailureDiagnostic(solution, locator, isWarning), CTP(purpose),
622+
RawFromType(lhs), RawToType(rhs) {
618623
assert(lhs && "Expected a valid 'from' type");
619624
assert(rhs && "Expected a valid 'to' type");
620625
}
@@ -753,8 +758,9 @@ class AttributedFuncToTypeConversionFailure final : public ContextualFailure {
753758

754759
AttributedFuncToTypeConversionFailure(const Solution &solution, Type fromType,
755760
Type toType, ConstraintLocator *locator,
756-
AttributeKind attributeKind)
757-
: ContextualFailure(solution, fromType, toType, locator),
761+
AttributeKind attributeKind,
762+
bool isWarning = false)
763+
: ContextualFailure(solution, fromType, toType, locator, isWarning),
758764
attributeKind(attributeKind) {}
759765

760766
bool diagnoseAsError() override;
@@ -777,8 +783,9 @@ class AttributedFuncToTypeConversionFailure final : public ContextualFailure {
777783
class DroppedGlobalActorFunctionAttr final : public ContextualFailure {
778784
public:
779785
DroppedGlobalActorFunctionAttr(const Solution &solution, Type fromType,
780-
Type toType, ConstraintLocator *locator)
781-
: ContextualFailure(solution, fromType, toType, locator) {}
786+
Type toType, ConstraintLocator *locator,
787+
bool isWarning)
788+
: ContextualFailure(solution, fromType, toType, locator, isWarning) { }
782789

783790
bool diagnoseAsError() override;
784791
};

‎lib/Sema/CSFix.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -201,39 +201,41 @@ MarkExplicitlyEscaping::create(ConstraintSystem &cs, Type lhs, Type rhs,
201201
bool MarkGlobalActorFunction::diagnose(const Solution &solution,
202202
bool asNote) const {
203203
DroppedGlobalActorFunctionAttr failure(
204-
solution, getFromType(), getToType(), getLocator());
204+
solution, getFromType(), getToType(), getLocator(), isWarning());
205205
return failure.diagnose(asNote);
206206
}
207207

208208
MarkGlobalActorFunction *
209209
MarkGlobalActorFunction::create(ConstraintSystem &cs, Type lhs, Type rhs,
210-
ConstraintLocator *locator) {
210+
ConstraintLocator *locator, bool warning) {
211211
if (locator->isLastElement<LocatorPathElt::ApplyArgToParam>())
212212
locator = cs.getConstraintLocator(
213213
locator, LocatorPathElt::ArgumentAttribute::forGlobalActor());
214214

215-
return new (cs.getAllocator()) MarkGlobalActorFunction(cs, lhs, rhs, locator);
215+
return new (cs.getAllocator()) MarkGlobalActorFunction(
216+
cs, lhs, rhs, locator, warning);
216217
}
217218

218219
bool AddSendableAttribute::diagnose(const Solution &solution,
219220
bool asNote) const {
220221
AttributedFuncToTypeConversionFailure failure(
221222
solution, getFromType(), getToType(), getLocator(),
222-
AttributedFuncToTypeConversionFailure::Concurrent);
223+
AttributedFuncToTypeConversionFailure::Concurrent, isWarning());
223224
return failure.diagnose(asNote);
224225
}
225226

226227
AddSendableAttribute *
227228
AddSendableAttribute::create(ConstraintSystem &cs,
228-
FunctionType *fromType,
229-
FunctionType *toType,
230-
ConstraintLocator *locator) {
229+
FunctionType *fromType,
230+
FunctionType *toType,
231+
ConstraintLocator *locator,
232+
bool warning) {
231233
if (locator->isLastElement<LocatorPathElt::ApplyArgToParam>())
232234
locator = cs.getConstraintLocator(
233235
locator, LocatorPathElt::ArgumentAttribute::forConcurrent());
234236

235237
return new (cs.getAllocator()) AddSendableAttribute(
236-
cs, fromType, toType, locator);
238+
cs, fromType, toType, locator, warning);
237239
}
238240
bool RelabelArguments::diagnose(const Solution &solution, bool asNote) const {
239241
LabelingFailure failure(solution, getLocator(), getLabels());
@@ -369,7 +371,8 @@ bool ContextualMismatch::diagnoseForAmbiguity(
369371
ContextualMismatch *ContextualMismatch::create(ConstraintSystem &cs, Type lhs,
370372
Type rhs,
371373
ConstraintLocator *locator) {
372-
return new (cs.getAllocator()) ContextualMismatch(cs, lhs, rhs, locator);
374+
return new (cs.getAllocator()) ContextualMismatch(
375+
cs, lhs, rhs, locator, /*warning=*/false);
373376
}
374377

375378
bool AllowWrappedValueMismatch::diagnose(const Solution &solution, bool asError) const {

‎lib/Sema/CSSimplify.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
//===----------------------------------------------------------------------===//
1717

1818
#include "CSDiagnostics.h"
19+
#include "TypeCheckConcurrency.h"
1920
#include "swift/AST/ASTPrinter.h"
2021
#include "swift/AST/Decl.h"
2122
#include "swift/AST/ExistentialLayout.h"
@@ -2170,6 +2171,22 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
21702171
increaseScore(SK_SyncInAsync);
21712172
}
21722173

2174+
/// Whether to downgrade to a concurrency warning.
2175+
auto isConcurrencyWarning = [&] {
2176+
if (contextUsesConcurrencyFeatures(DC) ||
2177+
Context.LangOpts.isSwiftVersionAtLeast(6))
2178+
return false;
2179+
2180+
switch (kind) {
2181+
case ConstraintKind::Conversion:
2182+
case ConstraintKind::ArgumentConversion:
2183+
return true;
2184+
2185+
default:
2186+
return false;
2187+
}
2188+
};
2189+
21732190
// A @Sendable function can be a subtype of a non-@Sendable function.
21742191
if (func1->isSendable() != func2->isSendable()) {
21752192
// Cannot add '@Sendable'.
@@ -2178,7 +2195,8 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
21782195
return getTypeMatchFailure(locator);
21792196

21802197
auto *fix = AddSendableAttribute::create(
2181-
*this, func1, func2, getConstraintLocator(locator));
2198+
*this, func1, func2, getConstraintLocator(locator),
2199+
isConcurrencyWarning());
21822200
if (recordFix(fix))
21832201
return getTypeMatchFailure(locator);
21842202
}
@@ -2212,7 +2230,8 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
22122230
return getTypeMatchFailure(locator);
22132231

22142232
auto *fix = MarkGlobalActorFunction::create(
2215-
*this, func1, func2, getConstraintLocator(locator));
2233+
*this, func1, func2, getConstraintLocator(locator),
2234+
isConcurrencyWarning());
22162235

22172236
if (recordFix(fix))
22182237
return getTypeMatchFailure(locator);

‎test/Concurrency/actor_isolation.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ func testGlobalActorClosures() {
403403
return 17
404404
}
405405

406-
acceptConcurrentClosure { @SomeGlobalActor in 5 } // expected-error{{converting function value of type '@SomeGlobalActor @Sendable () -> Int' to '@Sendable () -> Int' loses global actor 'SomeGlobalActor'}}
406+
acceptConcurrentClosure { @SomeGlobalActor in 5 } // expected-warning{{converting function value of type '@SomeGlobalActor @Sendable () -> Int' to '@Sendable () -> Int' loses global actor 'SomeGlobalActor'}}
407407
}
408408

409409
@available(SwiftStdlib 5.1, *)

‎test/Concurrency/global_actor_function_types.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ struct OtherGlobalActor {
1616
func testConversions(f: @escaping @SomeGlobalActor (Int) -> Void, g: @escaping (Int) -> Void) {
1717
let _: Int = f // expected-error{{cannot convert value of type '@SomeGlobalActor (Int) -> Void' to specified type 'Int'}}
1818

19-
let _: (Int) -> Void = f // expected-error{{converting function value of type '@SomeGlobalActor (Int) -> Void' to '(Int) -> Void' loses global actor 'SomeGlobalActor'}}
19+
let _: (Int) -> Void = f // expected-warning{{converting function value of type '@SomeGlobalActor (Int) -> Void' to '(Int) -> Void' loses global actor 'SomeGlobalActor'}}
2020
let _: @SomeGlobalActor (Int) -> Void = g // okay
2121

2222
// FIXME: this could be better.
@@ -119,7 +119,7 @@ func testTypesNonConcurrencyContext() { // expected-note{{add '@SomeGlobalActor'
119119
let f1 = onSomeGlobalActor // expected-note{{calls to let 'f1' from outside of its actor context are implicitly asynchronous}}
120120
let f2 = onSomeGlobalActorUnsafe
121121

122-
let _: () -> Int = f1 // expected-error{{converting function value of type '@SomeGlobalActor () -> Int' to '() -> Int' loses global actor 'SomeGlobalActor'}}
122+
let _: () -> Int = f1 // expected-warning{{converting function value of type '@SomeGlobalActor () -> Int' to '() -> Int' loses global actor 'SomeGlobalActor'}}
123123
let _: () -> Int = f2
124124

125125
_ = f1() // expected-error{{call to global actor 'SomeGlobalActor'-isolated let 'f1' in a synchronous nonisolated context}}

‎test/Concurrency/sendable_checking.swift

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,32 @@ extension NS3: Sendable { }
2424
@available(SwiftStdlib 5.1, *)
2525
func acceptCV<T: Sendable>(_: T) { }
2626

27+
func acceptSendableFn(_: @Sendable @escaping () -> Void) { }
28+
2729
@available(SwiftStdlib 5.1, *)
28-
func testCV(ns1: NS1, ns1array: [NS1], ns2: NS2, ns3: NS3) {
30+
func testCV(
31+
ns1: NS1, ns1array: [NS1], ns2: NS2, ns3: NS3, fn: @escaping () -> Void
32+
// expected-note@-1{{parameter 'fn' is implicitly non-sendable}}
33+
) {
2934
acceptCV(ns1)
3035
acceptCV(ns1array)
3136
acceptCV(ns2)
3237
acceptCV(ns3)
38+
acceptCV(fn)
39+
acceptSendableFn(fn) // expected-warning{{passing non-sendable parameter 'fn' to function expecting a @Sendable closure}}
3340
}
3441

3542
@available(SwiftStdlib 5.1, *)
36-
func testCV(ns1: NS1, ns1array: [NS1], ns2: NS2, ns3: NS3) async {
43+
func testCV(
44+
ns1: NS1, ns1array: [NS1], ns2: NS2, ns3: NS3, fn: @escaping () -> Void
45+
// expected-note@-1{{parameter 'fn' is implicitly non-sendable}}
46+
) async {
3747
acceptCV(ns1) // expected-warning{{conformance of 'NS1' to 'Sendable' is unavailable}}
3848
acceptCV(ns1array) // expected-warning{{conformance of 'NS1' to 'Sendable' is unavailable}}
3949
acceptCV(ns2) // expected-warning{{type 'NS2' does not conform to the 'Sendable' protocol}}
4050
acceptCV(ns3) // expected-warning{{conformance of 'NS3' to 'Sendable' is only available in macOS 11.0 or newer}}
4151
// expected-note@-1{{add 'if #available' version check}}
52+
acceptCV(fn) // expected-warning{{type '() -> Void' does not conform to the 'Sendable' protocol}}
53+
// expected-note@-1{{a function type must be marked '@Sendable' to conform to 'Sendable'}}
54+
acceptSendableFn(fn) // expected-error{{passing non-sendable parameter 'fn' to function expecting a @Sendable closure}}
4255
}

‎test/attr/attr_concurrent.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func passingConcurrentOrNot(
2929
f(ncfn)
3030

3131
acceptsConcurrent(cfn) // okay
32-
acceptsConcurrent(ncfn) // expected-error{{passing non-sendable parameter 'ncfn' to function expecting a @Sendable closure}}
32+
acceptsConcurrent(ncfn) // expected-warning{{passing non-sendable parameter 'ncfn' to function expecting a @Sendable closure}}
3333
acceptsNonConcurrent(cfn) // okay
3434
acceptsNonConcurrent(ncfn) // okay
3535

@@ -50,7 +50,7 @@ func closures() {
5050
})
5151

5252
let closure1 = { $0 + 1 } // inferred to be non-sendable
53-
acceptsConcurrent(closure1) // expected-error{{converting non-sendable function value to '@Sendable (Int) -> Int' may introduce data races}}
53+
acceptsConcurrent(closure1) // expected-warning{{converting non-sendable function value to '@Sendable (Int) -> Int' may introduce data races}}
5454
}
5555

5656
// Mutation of captured locals from within @Sendable functions.

0 commit comments

Comments
 (0)