Skip to content

Commit d99e331

Browse files
committed
Add implementation of _unavailableFromAsync check
The core part of the check runs as follows 1. Are we in an async context 2. Does the decl we're calling have the unavailableFromAsync attribute 3. Emit a diagnostic There are a couple challenges that muddy the implementation. First, single-expression closures are typechecked differently than multi-expression closures. The single-expression closure is never added to the closure stack. We have to record it separately upon entry to verify separately. This is necessary for `_ = { foo() }()` where `foo` is unavailable from async, and the surrounding context is async. The second challenge is regarding when things get typechecked. A type must be assigned to AbstractClosureExprs to determine whether they are an async context or not. Unfortunately, availability checking runs before types are fully assigned in some cases. This specifically happens when working with result builders in specific conditions. I haven't been able to figure out how to reduce the issue further.
1 parent 694fad4 commit d99e331

File tree

7 files changed

+141
-10
lines changed

7 files changed

+141
-10
lines changed

docs/ReferenceGuides/UnderscoredAttributes.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,3 +634,8 @@ calls a runtime function which allocates memory or locks, respectively.
634634
The `@_noLocks` attribute implies `@_noAllocation` because a memory allocation
635635
also locks.
636636

637+
## `@_unavailableFromAsync`
638+
639+
Marks a synchronous API as being unavailable from asynchronous contexts. Direct
640+
usage of annotated API from asynchronous contexts will result in a warning from
641+
the compiler.

include/swift/AST/Attr.def

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,12 @@ CONTEXTUAL_SIMPLE_DECL_ATTR(_const, CompileTimeConst,
699699
ABIStableToAdd | ABIStableToRemove | APIBreakingToAdd | APIStableToRemove,
700700
126)
701701

702+
SIMPLE_DECL_ATTR(_unavailableFromAsync, UnavailableFromAsync,
703+
OnFunc | OnConstructor |
704+
ABIStableToAdd | ABIStableToRemove |
705+
APIBreakingToAdd | APIStableToRemove,
706+
127)
707+
702708
// If you're adding a new underscored attribute here, please document it in
703709
// docs/ReferenceGuides/UnderscoredAttributes.md.
704710

include/swift/AST/DiagnosticsSema.def

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4722,6 +4722,15 @@ ERROR(actor_isolation_superclass_mismatch,none,
47224722
"%0 class %1 has different actor isolation from %2 superclass %3",
47234723
(ActorIsolation, DeclName, ActorIsolation, DeclName))
47244724

4725+
ERROR(async_decl_must_be_available_from_async,none,
4726+
"asynchronous %0 must be available from asynchronous contexts",
4727+
(DescriptiveDeclKind))
4728+
ERROR(async_named_decl_must_be_available_from_async,none,
4729+
"asynchronous %0 %1 must be available from asynchronous contexts",
4730+
(DescriptiveDeclKind, DeclName))
4731+
ERROR(async_unavailable_decl,none,
4732+
"%0 %1 is unavailable from asynchronous contexts", (DescriptiveDeclKind, DeclBaseName))
4733+
47254734
//------------------------------------------------------------------------------
47264735
// MARK: Type Check Types
47274736
//------------------------------------------------------------------------------

lib/Sema/TypeCheckAttr.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,8 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
270270
void visitNonisolatedAttr(NonisolatedAttr *attr);
271271

272272
void visitNoImplicitCopyAttr(NoImplicitCopyAttr *attr);
273+
274+
void visitUnavailableFromAsyncAttr(UnavailableFromAsyncAttr *attr);
273275
};
274276

275277
} // end anonymous namespace
@@ -2081,8 +2083,8 @@ SynthesizeMainFunctionRequest::evaluate(Evaluator &evaluator,
20812083
}
20822084

20832085
auto where = ExportContext::forDeclSignature(D);
2084-
diagnoseDeclAvailability(mainFunction, attr->getRange(), nullptr,
2085-
where, None);
2086+
diagnoseDeclAvailability(mainFunction, attr->getRange(), nullptr, where, None,
2087+
/*containingClosure*/ nullptr);
20862088

20872089
auto *const func = FuncDecl::createImplicit(
20882090
context, StaticSpellingKind::KeywordStatic,
@@ -5672,6 +5674,23 @@ void AttributeChecker::visitReasyncAttr(ReasyncAttr *attr) {
56725674
attr->setInvalid();
56735675
}
56745676

5677+
void AttributeChecker::visitUnavailableFromAsyncAttr(
5678+
UnavailableFromAsyncAttr *attr) {
5679+
if (DeclContext *dc = dyn_cast<DeclContext>(D)) {
5680+
if (dc->isAsyncContext()) {
5681+
if (ValueDecl *vd = dyn_cast<ValueDecl>(D)) {
5682+
D->getASTContext().Diags.diagnose(
5683+
D->getLoc(), diag::async_named_decl_must_be_available_from_async,
5684+
D->getDescriptiveKind(), vd->getName());
5685+
} else {
5686+
D->getASTContext().Diags.diagnose(
5687+
D->getLoc(), diag::async_decl_must_be_available_from_async,
5688+
D->getDescriptiveKind());
5689+
}
5690+
}
5691+
}
5692+
}
5693+
56755694
namespace {
56765695

56775696
class ClosureAttributeChecker

lib/Sema/TypeCheckAvailability.cpp

Lines changed: 96 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2767,6 +2767,7 @@ class ExprAvailabilityWalker : public ASTWalker {
27672767
ASTContext &Context;
27682768
MemberAccessContext AccessContext = MemberAccessContext::Getter;
27692769
SmallVector<const Expr *, 16> ExprStack;
2770+
SmallVector<ClosureExpr *, 4> singleExprClosureStack;
27702771
const ExportContext &Where;
27712772

27722773
public:
@@ -2847,24 +2848,47 @@ class ExprAvailabilityWalker : public ASTWalker {
28472848
E->getLoc(), Where);
28482849
}
28492850

2851+
// multi-statement closures are collected by ExprWalker::rewriteFunction
2852+
// and checked by ExprWalker::processDelayed in CSApply.cpp.
2853+
// Single-statement closures only have the attributes checked
2854+
// by TypeChecker::checkClosureAttributes in that rewriteFunction.
2855+
// As a result, we never see single-statement closures as the decl context
2856+
// in out 'where' here. By keeping a stack of closures, we can see if
2857+
// we're inside of one of these closures and go from there.
2858+
if (ClosureExpr *closure = dyn_cast<ClosureExpr>(E)) {
2859+
if (closure->hasSingleExpressionBody())
2860+
singleExprClosureStack.push_back(closure);
2861+
}
2862+
28502863
return visitChildren();
28512864
}
28522865

28532866
Expr *walkToExprPost(Expr *E) override {
28542867
assert(ExprStack.back() == E);
28552868
ExprStack.pop_back();
28562869

2870+
if (ClosureExpr *closure = dyn_cast<ClosureExpr>(E)) {
2871+
if (closure->hasSingleExpressionBody()) {
2872+
assert(closure == singleExprClosureStack.back() &&
2873+
"Popping wrong closure");
2874+
singleExprClosureStack.pop_back();
2875+
}
2876+
}
2877+
28572878
return E;
28582879
}
28592880

28602881
std::pair<bool, Stmt *> walkToStmtPre(Stmt *S) override {
2882+
28612883
// We end up here when checking the output of the result builder transform,
28622884
// which includes closures that are not "separately typechecked" and yet
28632885
// contain statements and declarations. We need to walk them recursively,
28642886
// since these availability for these statements is not diagnosed from
28652887
// typeCheckStmt() as usual.
2866-
diagnoseStmtAvailability(S, Where.getDeclContext(),
2867-
/*walkRecursively=*/true);
2888+
DeclContext *dc = singleExprClosureStack.empty()
2889+
? Where.getDeclContext()
2890+
: singleExprClosureStack.back();
2891+
diagnoseStmtAvailability(S, dc, /*walkRecursively=*/true);
28682892
return std::make_pair(false, S);
28692893
}
28702894

@@ -3043,8 +3067,8 @@ class ExprAvailabilityWalker : public ASTWalker {
30433067

30443068
Flags &= DeclAvailabilityFlag::ForInout;
30453069
Flags |= DeclAvailabilityFlag::ContinueOnPotentialUnavailability;
3046-
if (diagnoseDeclAvailability(D, ReferenceRange, /*call*/nullptr,
3047-
Where, Flags))
3070+
if (diagnoseDeclAvailability(D, ReferenceRange, /*call*/ nullptr, Where,
3071+
Flags, nullptr))
30483072
return;
30493073
}
30503074
};
@@ -3067,7 +3091,9 @@ bool ExprAvailabilityWalker::diagnoseDeclRefAvailability(
30673091
return true;
30683092
}
30693093

3070-
diagnoseDeclAvailability(D, R, call, Where, Flags);
3094+
const ClosureExpr *closure =
3095+
singleExprClosureStack.empty() ? nullptr : singleExprClosureStack.back();
3096+
diagnoseDeclAvailability(D, R, call, Where, Flags, closure);
30713097

30723098
if (R.isValid()) {
30733099
if (diagnoseSubstitutionMapAvailability(R.Start, declRef.getSubstitutions(),
@@ -3079,12 +3105,71 @@ bool ExprAvailabilityWalker::diagnoseDeclRefAvailability(
30793105
return false;
30803106
}
30813107

3108+
/// Diagnose uses of API annotated '@unavailableFromAsync' when used from
3109+
/// asynchronous contexts.
3110+
/// Returns true if a diagnostic was emitted, false otherwise.
3111+
static bool
3112+
diagnoseDeclUnavailableFromAsync(const ValueDecl *D, SourceRange R,
3113+
const Expr *call, const ExportContext &Where,
3114+
const ClosureExpr *singleExprClosure) {
3115+
// FIXME: I don't think this is right, but I don't understand the issue well
3116+
// enough to fix it properly. If the decl context is an abstract
3117+
// closure, we need it to have a type assigned to it before we can
3118+
// determine whether it is an asynchronous context. It will crash
3119+
// when we go to check without one. In TypeChecker::typeCheckExpression
3120+
// (TypeCheckConstraints.cpp:403), we apply a solution before calling
3121+
// `performSyntacticDiagnosticsForTarget`, which eventually calls
3122+
// down to this function. Under most circumstances, the context that
3123+
// we're in is typechecked at that point and has a type assigned.
3124+
// When working with specific result builders, the solution applied
3125+
// results in an expression with an unset type. In these cases, the
3126+
// application makes its way into `ConstraintSystem::applySolution` for
3127+
// closures (CSClosure.cpp:1356). The type is computed, but is
3128+
// squirreled away in the constrain system to be applied once the
3129+
// checks (including this one) approve of the decls within the decl
3130+
// context before applying the type to the expression. It might be
3131+
// possible to drive the constraint solver through the availability
3132+
// checker and into us so that we can ask for it, but that feels wrong
3133+
// too.
3134+
// This behavior is demonstrated by the first use of the `tuplify`
3135+
// function in `testExistingPatternsInCaseStatements` in
3136+
// `test/Constraints/result_builder.swift`.
3137+
const AbstractClosureExpr *declCtxAsExpr =
3138+
dyn_cast<AbstractClosureExpr>(Where.getDeclContext());
3139+
if (declCtxAsExpr && !declCtxAsExpr->getType()) {
3140+
return false;
3141+
}
3142+
3143+
// If we're directly in a sync closure, then we are not in an async context.
3144+
// If we're directly in a sync closure, or we're not in a closure and the
3145+
// outer context is not async, we are in a sync context.
3146+
const bool inSingleClosure = singleExprClosure;
3147+
const bool inSyncClosure =
3148+
inSingleClosure && !singleExprClosure->isAsyncContext();
3149+
const bool inSyncContext =
3150+
!inSingleClosure && !Where.getDeclContext()->isAsyncContext();
3151+
if (inSyncClosure || inSyncContext)
3152+
return false;
3153+
if (!D->getAttrs().hasAttribute<UnavailableFromAsyncAttr>())
3154+
return false;
3155+
3156+
ASTContext &ctx = Where.getDeclContext()->getASTContext();
3157+
SourceLoc diagLoc = call ? call->getLoc() : R.Start;
3158+
ctx.Diags
3159+
.diagnose(diagLoc, diag::async_unavailable_decl, D->getDescriptiveKind(),
3160+
D->getBaseName())
3161+
.warnUntilSwiftVersion(6);
3162+
D->diagnose(diag::decl_declared_here, D->getName());
3163+
return true;
3164+
}
3165+
30823166
/// Diagnose uses of unavailable declarations. Returns true if a diagnostic
30833167
/// was emitted.
30843168
bool swift::diagnoseDeclAvailability(const ValueDecl *D, SourceRange R,
30853169
const Expr *call,
30863170
const ExportContext &Where,
3087-
DeclAvailabilityFlags Flags) {
3171+
DeclAvailabilityFlags Flags,
3172+
const ClosureExpr *singleExprClosure) {
30883173
assert(!Where.isImplicit());
30893174

30903175
// Generic parameters are always available.
@@ -3112,6 +3197,9 @@ bool swift::diagnoseDeclAvailability(const ValueDecl *D, SourceRange R,
31123197
if (diagnoseExplicitUnavailability(D, R, Where, call, Flags))
31133198
return true;
31143199

3200+
if (diagnoseDeclUnavailableFromAsync(D, R, call, Where, singleExprClosure))
3201+
return true;
3202+
31153203
// Make sure not to diagnose an accessor's deprecation if we already
31163204
// complained about the property/subscript.
31173205
bool isAccessorWithDeprecatedStorage =
@@ -3360,7 +3448,8 @@ class TypeReprAvailabilityWalker : public ASTWalker {
33603448
bool checkComponentIdentTypeRepr(ComponentIdentTypeRepr *ITR) {
33613449
if (auto *typeDecl = ITR->getBoundDecl()) {
33623450
auto range = ITR->getNameLoc().getSourceRange();
3363-
if (diagnoseDeclAvailability(typeDecl, range, nullptr, where, flags))
3451+
if (diagnoseDeclAvailability(typeDecl, range, nullptr, where, flags,
3452+
nullptr))
33643453
return true;
33653454
}
33663455

lib/Sema/TypeCheckAvailability.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ namespace swift {
2727
class ApplyExpr;
2828
class AvailableAttr;
2929
class Expr;
30+
class ClosureExpr;
3031
class InFlightDiagnostic;
3132
class Decl;
3233
class ProtocolConformanceRef;
@@ -232,7 +233,8 @@ diagnoseSubstitutionMapAvailability(SourceLoc loc,
232233
/// was emitted.
233234
bool diagnoseDeclAvailability(const ValueDecl *D, SourceRange R,
234235
const Expr *call, const ExportContext &where,
235-
DeclAvailabilityFlags flags = None);
236+
DeclAvailabilityFlags flags = None,
237+
const ClosureExpr *singleExprClosure = nullptr);
236238

237239
void diagnoseUnavailableOverride(ValueDecl *override,
238240
const ValueDecl *base,

lib/Sema/TypeCheckDeclOverride.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,6 +1559,7 @@ namespace {
15591559
UNINTERESTING_ATTR(InheritActorContext)
15601560
UNINTERESTING_ATTR(Isolated)
15611561
UNINTERESTING_ATTR(NoImplicitCopy)
1562+
UNINTERESTING_ATTR(UnavailableFromAsync)
15621563

15631564
UNINTERESTING_ATTR(TypeSequence)
15641565
UNINTERESTING_ATTR(CompileTimeConst)

0 commit comments

Comments
 (0)