Skip to content

Commit 1597c41

Browse files
author
jturcotti
committed
create the SendNonSendableExpr AST node that defers the emission of diagnostics from Sendable checking. Parts of TypeCheckConcurrency are refactored to defer these diagnostics, and tests are added to ensure the deferral happens as expected.
1 parent e240ec5 commit 1597c41

File tree

13 files changed

+527
-115
lines changed

13 files changed

+527
-115
lines changed

include/swift/AST/ActorIsolation.h

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,98 @@ bool safeToDropGlobalActor(
262262

263263
void simple_display(llvm::raw_ostream &out, const ActorIsolation &state);
264264

265+
/// A DeferredSendableDiagnostic wraps a list of closures that emit
266+
/// Diagnostics when called. It is used to allow the logic for forming
267+
/// those diagnostics to take place ahead of time, while delaying the
268+
/// actual emission until several passes later. In particular, diagnostics
269+
/// that identify NonSendable types being sent between isolation domains
270+
/// are deferred thus so that a later flow-sensitive SIL pass can eliminate
271+
/// diagnostics for sends that are provably safe.
272+
struct DeferredSendableDiagnostic {
273+
private:
274+
275+
// This field indicates whether any errors (as opposed to just warnings
276+
// and notes) are produced by this DeferredSendableDiagnostic instance.
277+
// This exists to allow existing control flow through the call stack in
278+
// ActorIsolationChecker's walk methods. Because that control flow wasn't
279+
// entirely principled, sometime the use of this field doesn't exactly
280+
// align with the presence of errors vs warnings.
281+
bool ProducesErrors;
282+
283+
// This field stores a vector, each entry of which is a closure that can be
284+
// called, in order, to emit diagnostics.
285+
std::vector<std::function<void()>> Diagnostics;
286+
287+
public:
288+
DeferredSendableDiagnostic()
289+
: ProducesErrors(false){}
290+
291+
// In general, an empty no-op closure should not be passed to
292+
// Diagnostic here, or ProducesDiagnostics will contain an
293+
// imprecise value.
294+
DeferredSendableDiagnostic(
295+
bool ProducesErrors, std::function<void()> Diagnostic)
296+
: ProducesErrors(ProducesErrors),
297+
Diagnostics({Diagnostic}) {
298+
assert(Diagnostic && "Empty diagnostics function");
299+
}
300+
301+
bool produceAndCheckForErrors() {
302+
produceDiagnostics();
303+
return producesErrors();
304+
}
305+
306+
bool producesErrors() const {
307+
return ProducesErrors;
308+
}
309+
310+
bool producesDiagnostics() const {
311+
return Diagnostics.size() != 0;
312+
}
313+
314+
// Idempotent operation: call the contained closures in Diagnostics in order,
315+
// and clear out the list so subsequent invocations are a no-op
316+
void produceDiagnostics() {
317+
for (auto Diagnostic : Diagnostics) {
318+
Diagnostic();
319+
}
320+
ProducesErrors = false;
321+
Diagnostics = {};
322+
}
323+
324+
void setProducesErrors(bool producesErrors) {
325+
ProducesErrors = producesErrors;
326+
}
327+
328+
// In general, an empty no-op closure should not be passed to
329+
// Diagnostic here, or ProducesDiagnostics will contain an
330+
// imprecise value.
331+
void addDiagnostic(std::function<void()> Diagnostic) {
332+
assert(Diagnostic && "Empty diagnostics function");
333+
334+
Diagnostics.push_back(Diagnostic);
335+
}
336+
337+
/// This variation on addErrorProducingDiagnostic should be called
338+
/// when the passed lambda will definitely through a diagnostic
339+
/// for the sake of maintaining existing control flow paths, it
340+
/// is not used everywhere.
341+
void addErrorProducingDiagnostic(std::function<void()> produceMoreDiagnostics) {
342+
addDiagnostic(produceMoreDiagnostics);
343+
setProducesErrors(true);
344+
}
345+
346+
// compose this DeferredSendableDiagnostic with another - calling their
347+
// wrapped Diagnostics closure in sequence and disjuncting their
348+
// respective ProducesErrors flags
349+
void followWith(DeferredSendableDiagnostic other) {
350+
for (auto Diagnostic : other.Diagnostics) {
351+
Diagnostics.push_back(Diagnostic);
352+
}
353+
ProducesErrors = ProducesErrors || other.ProducesErrors;
354+
}
355+
};
356+
265357
} // end namespace swift
266358

267359
#endif /* SWIFT_AST_ACTORISOLATIONSTATE_H */

include/swift/AST/Expr.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3084,6 +3084,20 @@ class LinearToDifferentiableFunctionExpr : public ImplicitConversionExpr {
30843084
}
30853085
};
30863086

3087+
class SendNonSendableExpr : public ImplicitConversionExpr {
3088+
DeferredSendableDiagnostic *Diagnostic;
3089+
public:
3090+
SendNonSendableExpr(
3091+
ASTContext &ctx, DeferredSendableDiagnostic diagnostic, Expr *sub,
3092+
Type type = Type());
3093+
3094+
void produceDiagnostics() { Diagnostic->produceDiagnostics(); }
3095+
3096+
static bool classof(const Expr *E) {
3097+
return E->getKind() == ExprKind::SendNonSendable;
3098+
}
3099+
};
3100+
30873101
/// Use an opaque type to abstract a value of the underlying concrete type,
30883102
/// possibly nested inside other types. E.g. can perform conversions "T --->
30893103
/// (opaque type)" and "S<T> ---> S<(opaque type)>".

include/swift/AST/ExprNodes.def

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ ABSTRACT_EXPR(ImplicitConversion, Expr)
188188
EXPR(DifferentiableFunctionExtractOriginal, ImplicitConversionExpr)
189189
EXPR(LinearFunctionExtractOriginal, ImplicitConversionExpr)
190190
EXPR(LinearToDifferentiableFunction, ImplicitConversionExpr)
191-
EXPR_RANGE(ImplicitConversion, Load, LinearToDifferentiableFunction)
191+
EXPR(SendNonSendable, ImplicitConversionExpr)
192+
EXPR_RANGE(ImplicitConversion, Load, SendNonSendable)
192193
ABSTRACT_EXPR(ExplicitCast, Expr)
193194
ABSTRACT_EXPR(CheckedCast, ExplicitCastExpr)
194195
EXPR(ForcedCheckedCast, CheckedCastExpr)

include/swift/Basic/Features.def

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ EXPERIMENTAL_FEATURE(OldOwnershipOperatorSpellings, true)
135135
EXPERIMENTAL_FEATURE(MoveOnlyEnumDeinits, true)
136136
EXPERIMENTAL_FEATURE(MoveOnlyTuples, true)
137137
EXPERIMENTAL_FEATURE(MoveOnlyResilientTypes, true)
138-
EXPERIMENTAL_FEATURE(MoveOnlyPartialConsumption, true)
139138

140139
EXPERIMENTAL_FEATURE(OneWayClosureParameters, false)
141140
EXPERIMENTAL_FEATURE(TypeWitnessSystemInference, false)
@@ -211,6 +210,9 @@ EXPERIMENTAL_FEATURE(ReferenceBindings, false)
211210
/// Enable the explicit 'import Builtin' and allow Builtin usage.
212211
EXPERIMENTAL_FEATURE(BuiltinModule, true)
213212

213+
/// Defer Sendable checking to SIL diagnostic phase.
214+
EXPERIMENTAL_FEATURE(DeferredSendableChecking, false)
215+
214216
// Enable strict concurrency.
215217
EXPERIMENTAL_FEATURE(StrictConcurrency, true)
216218

lib/AST/ASTDumper.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2205,6 +2205,12 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
22052205
printRec(E->getSubExpr());
22062206
PrintWithColorRAII(OS, ParenthesisColor) << ')';
22072207
}
2208+
void visitSendNonSendableExpr(SendNonSendableExpr *E) {
2209+
printCommon(E, "send_non_sendable_expr");
2210+
OS << '\n';
2211+
printRec(E->getSubExpr());
2212+
PrintWithColorRAII(OS, ParenthesisColor) << ')';
2213+
}
22082214
void visitConsumeExpr(ConsumeExpr *E) {
22092215
printCommon(E, "consume_expr");
22102216
OS << '\n';

lib/AST/ASTPrinter.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3445,6 +3445,10 @@ static bool usesFeatureParameterPacks(Decl *decl) {
34453445
return false;
34463446
}
34473447

3448+
static bool usesFeatureDeferredSendableChecking(Decl *decl) {
3449+
return false;
3450+
}
3451+
34483452
/// Suppress the printing of a particular feature.
34493453
static void suppressingFeature(PrintOptions &options, Feature feature,
34503454
llvm::function_ref<void()> action) {
@@ -5030,6 +5034,11 @@ void PrintAST::visitAwaitExpr(AwaitExpr *expr) {
50305034
visit(expr->getSubExpr());
50315035
}
50325036

5037+
void PrintAST::visitSendNonSendableExpr(SendNonSendableExpr *expr) {
5038+
Printer << "sendNonSendable ";
5039+
visit(expr->getSubExpr());
5040+
}
5041+
50335042
void PrintAST::visitConsumeExpr(ConsumeExpr *expr) {
50345043
Printer << "consume ";
50355044
visit(expr->getSubExpr());

lib/AST/Expr.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@ ConcreteDeclRef Expr::getReferencedDecl(bool stopAtParenExpr) const {
370370
PASS_THROUGH_REFERENCE(UnresolvedMemberChainResult, getSubExpr);
371371
PASS_THROUGH_REFERENCE(DotSelf, getSubExpr);
372372
PASS_THROUGH_REFERENCE(Await, getSubExpr);
373+
PASS_THROUGH_REFERENCE(SendNonSendable, getSubExpr);
373374
PASS_THROUGH_REFERENCE(Consume, getSubExpr);
374375
PASS_THROUGH_REFERENCE(Copy, getSubExpr);
375376
PASS_THROUGH_REFERENCE(Borrow, getSubExpr);
@@ -750,6 +751,7 @@ bool Expr::canAppendPostfixExpression(bool appendingPostfixOperator) const {
750751
case ExprKind::ForceTry:
751752
case ExprKind::OptionalTry:
752753
case ExprKind::InOut:
754+
case ExprKind::SendNonSendable:
753755
return false;
754756

755757
case ExprKind::RebindSelfInConstructor:
@@ -939,6 +941,7 @@ bool Expr::isValidParentOfTypeExpr(Expr *typeExpr) const {
939941
case ExprKind::Consume:
940942
case ExprKind::Copy:
941943
case ExprKind::Borrow:
944+
case ExprKind::SendNonSendable:
942945
case ExprKind::UnresolvedMemberChainResult:
943946
case ExprKind::Try:
944947
case ExprKind::ForceTry:
@@ -2784,4 +2787,9 @@ const UnifiedStatsReporter::TraceFormatter*
27842787
FrontendStatsTracer::getTraceFormatter<const Expr *>() {
27852788
return &TF;
27862789
}
2787-
2790+
SendNonSendableExpr::SendNonSendableExpr(ASTContext &ctx, DeferredSendableDiagnostic diagnostic, Expr *sub, Type type)
2791+
: ImplicitConversionExpr(ExprKind::SendNonSendable, sub, type) {
2792+
void *mem = ctx.Allocate(sizeof(DeferredSendableDiagnostic), alignof(DeferredSendableDiagnostic));
2793+
Diagnostic = new (mem) DeferredSendableDiagnostic(std::move(diagnostic));
2794+
ctx.addDestructorCleanup(*Diagnostic);
2795+
}

lib/SILGen/SILGenApply.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -903,6 +903,12 @@ class SILGenApply : public Lowering::ExprVisitor<SILGenApply> {
903903
}
904904

905905
SelfApplyExpr *getAsMethodSelfApply(Expr *e) {
906+
// we need to look through any SendNonSendableExpr's that could
907+
// wrap the SelfApplyExpr we're looking for
908+
if (auto *SNS = dyn_cast<SendNonSendableExpr>(e)) {
909+
SNS->produceDiagnostics();
910+
return getAsMethodSelfApply(SNS->getSubExpr());
911+
}
906912
auto *SAE = dyn_cast<SelfApplyExpr>(e);
907913
if (!SAE)
908914
return nullptr;

lib/SILGen/SILGenExpr.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,7 @@ namespace {
554554
RValue visitConsumeExpr(ConsumeExpr *E, SGFContext C);
555555
RValue visitCopyExpr(CopyExpr *E, SGFContext C);
556556
RValue visitMacroExpansionExpr(MacroExpansionExpr *E, SGFContext C);
557+
RValue visitSendNonSendableExpr(SendNonSendableExpr *E, SGFContext C);
557558
};
558559
} // end anonymous namespace
559560

@@ -6310,6 +6311,15 @@ RValue RValueEmitter::visitMacroExpansionExpr(MacroExpansionExpr *E,
63106311
return RValue();
63116312
}
63126313

6314+
RValue RValueEmitter::visitSendNonSendableExpr(
6315+
SendNonSendableExpr *E, SGFContext C) {
6316+
6317+
// produce deferred diagnostics
6318+
E->produceDiagnostics();
6319+
6320+
return visit(E->getSubExpr(), C);
6321+
}
6322+
63136323
RValue SILGenFunction::emitRValue(Expr *E, SGFContext C) {
63146324
assert(!E->getType()->hasLValueType() &&
63156325
"l-values must be emitted with emitLValue");

0 commit comments

Comments
 (0)