Skip to content

Commit 0575571

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 bd8e6bc commit 0575571

File tree

13 files changed

+527
-114
lines changed

13 files changed

+527
-114
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 & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,9 @@ EXPERIMENTAL_FEATURE(ReferenceBindings, false)
209209
/// Enable the explicit 'import Builtin' and allow Builtin usage.
210210
EXPERIMENTAL_FEATURE(BuiltinModule, true)
211211

212+
/// Defer Sendable checking to SIL diagnostic phase.
213+
EXPERIMENTAL_FEATURE(DeferredSendableChecking, false)
214+
212215
#undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
213216
#undef EXPERIMENTAL_FEATURE
214217
#undef UPCOMING_FEATURE

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
@@ -3431,6 +3431,10 @@ static bool usesFeatureParameterPacks(Decl *decl) {
34313431
return false;
34323432
}
34333433

3434+
static bool usesFeatureDeferredSendableChecking(Decl *decl) {
3435+
return false;
3436+
}
3437+
34343438
/// Suppress the printing of a particular feature.
34353439
static void suppressingFeature(PrintOptions &options, Feature feature,
34363440
llvm::function_ref<void()> action) {
@@ -5016,6 +5020,11 @@ void PrintAST::visitAwaitExpr(AwaitExpr *expr) {
50165020
visit(expr->getSubExpr());
50175021
}
50185022

5023+
void PrintAST::visitSendNonSendableExpr(SendNonSendableExpr *expr) {
5024+
Printer << "sendNonSendable ";
5025+
visit(expr->getSubExpr());
5026+
}
5027+
50195028
void PrintAST::visitConsumeExpr(ConsumeExpr *expr) {
50205029
Printer << "consume ";
50215030
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)