Skip to content

Commit cba6b5f

Browse files
hamishknighthamishknight
committed
[Sema] Downgrade noasync diagnostic to warning for closure macro args
Downgrade to a warning until the next language mode. This is necessary since we previously missed coercing macro arguments to parameter types, resulting in cases where closure arguments weren't being treated as `async` when they should have been. rdar://149328745
1 parent 095fdb6 commit cba6b5f

File tree

4 files changed

+153
-23
lines changed

4 files changed

+153
-23
lines changed

‎include/swift/AST/Expr.h

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
267267
Kind : 2
268268
);
269269

270-
SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1+1+1,
270+
SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1+1+1+1,
271271
/// True if closure parameters were synthesized from anonymous closure
272272
/// variables.
273273
HasAnonymousClosureVars : 1,
@@ -295,7 +295,12 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
295295
/// isolation checks when it either specifies or inherits isolation
296296
/// and was passed as an argument to a function that is not fully
297297
/// concurrency checked.
298-
RequiresDynamicIsolationChecking : 1
298+
RequiresDynamicIsolationChecking : 1,
299+
300+
/// Whether this closure was type-checked as an argument to a macro. This
301+
/// is only populated after type-checking, and only exists for diagnostic
302+
/// logic. Do not add more uses of this.
303+
IsMacroArgument : 1
299304
);
300305

301306
SWIFT_INLINE_BITFIELD_FULL(BindOptionalExpr, Expr, 16,
@@ -4316,6 +4321,7 @@ class ClosureExpr : public AbstractClosureExpr {
43164321
Bits.ClosureExpr.IsPassedToSendingParameter = false;
43174322
Bits.ClosureExpr.NoGlobalActorAttribute = false;
43184323
Bits.ClosureExpr.RequiresDynamicIsolationChecking = false;
4324+
Bits.ClosureExpr.IsMacroArgument = false;
43194325
}
43204326

43214327
SourceRange getSourceRange() const;
@@ -4394,6 +4400,17 @@ class ClosureExpr : public AbstractClosureExpr {
43944400
Bits.ClosureExpr.RequiresDynamicIsolationChecking = value;
43954401
}
43964402

4403+
/// Whether this closure was type-checked as an argument to a macro. This
4404+
/// is only populated after type-checking, and only exists for diagnostic
4405+
/// logic. Do not add more uses of this.
4406+
bool isMacroArgument() const {
4407+
return Bits.ClosureExpr.IsMacroArgument;
4408+
}
4409+
4410+
void setIsMacroArgument(bool value = true) {
4411+
Bits.ClosureExpr.IsMacroArgument = value;
4412+
}
4413+
43974414
/// Determine whether this closure expression has an
43984415
/// explicitly-specified result type.
43994416
bool hasExplicitResultType() const { return ArrowLoc.isValid(); }

‎lib/Sema/CSApply.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6050,28 +6050,32 @@ static bool hasCurriedSelf(ConstraintSystem &cs, ConcreteDeclRef callee,
60506050
static void applyContextualClosureFlags(Expr *expr, bool implicitSelfCapture,
60516051
bool inheritActorContext,
60526052
bool isPassedToSendingParameter,
6053-
bool requiresDynamicIsolationChecking) {
6053+
bool requiresDynamicIsolationChecking,
6054+
bool isMacroArg) {
60546055
if (auto closure = dyn_cast<ClosureExpr>(expr)) {
60556056
closure->setAllowsImplicitSelfCapture(implicitSelfCapture);
60566057
closure->setInheritsActorContext(inheritActorContext);
60576058
closure->setIsPassedToSendingParameter(isPassedToSendingParameter);
60586059
closure->setRequiresDynamicIsolationChecking(
60596060
requiresDynamicIsolationChecking);
6061+
closure->setIsMacroArgument(isMacroArg);
60606062
return;
60616063
}
60626064

60636065
if (auto captureList = dyn_cast<CaptureListExpr>(expr)) {
60646066
applyContextualClosureFlags(captureList->getClosureBody(),
60656067
implicitSelfCapture, inheritActorContext,
60666068
isPassedToSendingParameter,
6067-
requiresDynamicIsolationChecking);
6069+
requiresDynamicIsolationChecking,
6070+
isMacroArg);
60686071
}
60696072

60706073
if (auto identity = dyn_cast<IdentityExpr>(expr)) {
60716074
applyContextualClosureFlags(identity->getSubExpr(), implicitSelfCapture,
60726075
inheritActorContext,
60736076
isPassedToSendingParameter,
6074-
requiresDynamicIsolationChecking);
6077+
requiresDynamicIsolationChecking,
6078+
isMacroArg);
60756079
}
60766080
}
60776081

@@ -6196,19 +6200,22 @@ ArgumentList *ExprRewriter::coerceCallArguments(
61966200
}();
61976201

61986202
auto applyFlagsToArgument = [&paramInfo,
6199-
&closuresRequireDynamicIsolationChecking](
6203+
&closuresRequireDynamicIsolationChecking,
6204+
&locator](
62006205
unsigned paramIdx, Expr *argument) {
62016206
if (!isClosureLiteralExpr(argument))
62026207
return;
62036208

62046209
bool isImplicitSelfCapture = paramInfo.isImplicitSelfCapture(paramIdx);
62056210
bool inheritsActorContext = paramInfo.inheritsActorContext(paramIdx);
62066211
bool isPassedToSendingParameter = paramInfo.isSendingParameter(paramIdx);
6212+
bool isMacroArg = isExpr<MacroExpansionExpr>(locator.getAnchor());
62076213

62086214
applyContextualClosureFlags(argument, isImplicitSelfCapture,
62096215
inheritsActorContext,
62106216
isPassedToSendingParameter,
6211-
closuresRequireDynamicIsolationChecking);
6217+
closuresRequireDynamicIsolationChecking,
6218+
isMacroArg);
62126219
};
62136220

62146221
// Quickly test if any further fix-ups for the argument types are necessary.

‎lib/Sema/TypeCheckAvailability.cpp

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -276,16 +276,18 @@ static bool shouldAllowReferenceToUnavailableInSwiftDeclaration(
276276
return false;
277277
}
278278

279-
// Utility function to help determine if noasync diagnostics are still
280-
// appropriate even if a `DeclContext` returns `false` from `isAsyncContext()`.
281-
static bool shouldTreatDeclContextAsAsyncForDiagnostics(const DeclContext *DC) {
282-
if (auto *D = DC->getAsDecl())
283-
if (auto *FD = dyn_cast<FuncDecl>(D))
279+
/// Retrieve the innermost DeclContext that should be consulted for noasync
280+
/// checking.
281+
static const DeclContext *
282+
getInnermostDeclContextForNoAsync(const DeclContext *DC) {
283+
if (auto *D = DC->getAsDecl()) {
284+
if (auto *FD = dyn_cast<FuncDecl>(D)) {
284285
if (FD->isDeferBody())
285286
// If this is a defer body, we should delegate to its parent.
286-
return shouldTreatDeclContextAsAsyncForDiagnostics(DC->getParent());
287-
288-
return DC->isAsyncContext();
287+
return getInnermostDeclContextForNoAsync(DC->getParent());
288+
}
289+
}
290+
return DC;
289291
}
290292

291293
/// A class that walks the AST to find the innermost (i.e., deepest) node that
@@ -2758,7 +2760,8 @@ static bool
27582760
diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
27592761
const Expr *call, const ExportContext &Where) {
27602762
// If we are not in an (effective) async context, don't check it
2761-
if (!shouldTreatDeclContextAsAsyncForDiagnostics(Where.getDeclContext()))
2763+
auto *noAsyncDC = getInnermostDeclContextForNoAsync(Where.getDeclContext());
2764+
if (!noAsyncDC->isAsyncContext())
27622765
return false;
27632766

27642767
ASTContext &ctx = Where.getDeclContext()->getASTContext();
@@ -2774,14 +2777,27 @@ diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
27742777
}
27752778
}
27762779

2780+
// In Swift 6 we previously didn't coerce macro arguments to parameter types,
2781+
// so closure arguments may be treated as async in cases where they weren't in
2782+
// Swift 6. As such we need to warn if the use is within a closure macro
2783+
// argument until the next language mode.
2784+
auto shouldWarnUntilFutureVersion = [&]() {
2785+
auto *CE = dyn_cast<ClosureExpr>(noAsyncDC);
2786+
return CE && CE->isMacroArgument();
2787+
};
2788+
27772789
// @available(noasync) spelling
27782790
if (auto attr = D->getNoAsyncAttr()) {
27792791
SourceLoc diagLoc = call ? call->getLoc() : R.Start;
27802792
auto diag = ctx.Diags.diagnose(diagLoc, diag::async_unavailable_decl, D,
27812793
attr->getMessage());
2782-
diag.warnUntilSwiftVersion(6);
2783-
diag.limitBehaviorWithPreconcurrency(DiagnosticBehavior::Warning,
2784-
D->preconcurrency());
2794+
if (D->preconcurrency()) {
2795+
diag.limitBehavior(DiagnosticBehavior::Warning);
2796+
} else if (shouldWarnUntilFutureVersion()) {
2797+
diag.warnUntilFutureSwiftVersion();
2798+
} else {
2799+
diag.warnUntilSwiftVersion(6);
2800+
}
27852801

27862802
if (!attr->getRename().empty()) {
27872803
fixItAvailableAttrRename(diag, R, D, attr->getRename(), call);
@@ -2797,10 +2813,16 @@ diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
27972813
// @_unavailableFromAsync spelling
27982814
const UnavailableFromAsyncAttr *attr =
27992815
D->getAttrs().getAttribute<UnavailableFromAsyncAttr>();
2800-
SourceLoc diagLoc = call ? call->getLoc() : R.Start;
2801-
ctx.Diags
2802-
.diagnose(diagLoc, diag::async_unavailable_decl, D, attr->Message)
2803-
.warnUntilSwiftVersion(6);
2816+
{
2817+
SourceLoc diagLoc = call ? call->getLoc() : R.Start;
2818+
auto diag = ctx.Diags.diagnose(diagLoc, diag::async_unavailable_decl, D,
2819+
attr->Message);
2820+
if (shouldWarnUntilFutureVersion()) {
2821+
diag.warnUntilFutureSwiftVersion();
2822+
} else {
2823+
diag.warnUntilSwiftVersion(6);
2824+
}
2825+
}
28042826
D->diagnose(diag::decl_declared_here, D);
28052827
return true;
28062828
}

‎test/Macros/issue-80835.swift

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/Inputs/syntax_macro_definitions.swift
3+
4+
// RUN: %target-typecheck-verify-swift -swift-version 6 -load-plugin-library %t/%target-library-name(MacroDefinition) -verify-additional-prefix swift6-
5+
// RUN: %target-typecheck-verify-swift -swift-version 7 -load-plugin-library %t/%target-library-name(MacroDefinition) -verify-additional-prefix swift7-
6+
7+
// REQUIRES: swift_swift_parser
8+
// REQUIRES: swift7
9+
10+
// https://github.com/swiftlang/swift/issues/80835
11+
12+
@available(*, noasync)
13+
func noasyncFn() {}
14+
15+
@_unavailableFromAsync
16+
func unavailableFromAsyncFn() {} // expected-note {{'unavailableFromAsyncFn()' declared here}}
17+
18+
@freestanding(expression)
19+
macro asyncMacro(_ fn: () async -> Void) = #externalMacro(module: "MacroDefinition", type: "GenericToVoidMacro")
20+
21+
@freestanding(declaration)
22+
macro asyncMacroDecl(_ fn: () async -> Void) = #externalMacro(module: "MacroDefinition", type: "EmptyDeclarationMacro")
23+
24+
@attached(peer)
25+
macro AsyncMacro(_ fn: () async -> Void) = #externalMacro(module: "MacroDefinition", type: "WrapperMacro")
26+
27+
func takesAsyncFn(_ fn: () async -> Void) {}
28+
29+
#asyncMacro {
30+
defer {
31+
noasyncFn()
32+
// expected-swift7-error@-1 {{global function 'noasyncFn' is unavailable from asynchronous contexts}}
33+
// expected-swift6-warning@-2 {{global function 'noasyncFn' is unavailable from asynchronous contexts; this will be an error in a future Swift language mode}}
34+
}
35+
noasyncFn()
36+
// expected-swift7-error@-1 {{global function 'noasyncFn' is unavailable from asynchronous contexts}}
37+
// expected-swift6-warning@-2 {{global function 'noasyncFn' is unavailable from asynchronous contexts; this will be an error in a future Swift language mode}}
38+
39+
unavailableFromAsyncFn()
40+
// expected-swift7-error@-1 {{global function 'unavailableFromAsyncFn' is unavailable from asynchronous contexts}}
41+
// expected-swift6-warning@-2 {{global function 'unavailableFromAsyncFn' is unavailable from asynchronous contexts; this will be an error in a future Swift language mode}}
42+
43+
// This was always an error.
44+
let _: () async -> Void = {
45+
noasyncFn()
46+
// expected-error@-1 {{global function 'noasyncFn' is unavailable from asynchronous contexts}}
47+
}
48+
}
49+
50+
// This was always an error.
51+
takesAsyncFn {
52+
noasyncFn()
53+
// expected-error@-1 {{global function 'noasyncFn' is unavailable from asynchronous contexts}}
54+
}
55+
56+
#asyncMacroDecl {
57+
noasyncFn()
58+
// expected-swift7-error@-1 {{global function 'noasyncFn' is unavailable from asynchronous contexts}}
59+
// expected-swift6-warning@-2 {{global function 'noasyncFn' is unavailable from asynchronous contexts; this will be an error in a future Swift language mode}}
60+
}
61+
62+
typealias Magic<T> = T
63+
64+
// expected-swift7-error@+2 {{global function 'noasyncFn' is unavailable from asynchronous contexts}}
65+
// expected-swift6-warning@+1 {{global function 'noasyncFn' is unavailable from asynchronous contexts; this will be an error in a future Swift language mode}}
66+
@AsyncMacro({ noasyncFn() })
67+
func foo() {
68+
#asyncMacro(({
69+
noasyncFn()
70+
// expected-swift7-error@-1 {{global function 'noasyncFn' is unavailable from asynchronous contexts}}
71+
// expected-swift6-warning@-2 {{global function 'noasyncFn' is unavailable from asynchronous contexts; this will be an error in a future Swift language mode}}
72+
}))
73+
74+
#asyncMacroDecl(({
75+
noasyncFn()
76+
// expected-swift7-error@-1 {{global function 'noasyncFn' is unavailable from asynchronous contexts}}
77+
// expected-swift6-warning@-2 {{global function 'noasyncFn' is unavailable from asynchronous contexts; this will be an error in a future Swift language mode}}
78+
}))
79+
80+
// This was never treated as async.
81+
#asyncMacro({
82+
noasyncFn()
83+
} as Magic)
84+
}

0 commit comments

Comments
 (0)