Skip to content

Disallow async IBAction methods #59205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1451,6 +1451,8 @@ NOTE(fixit_rename_in_swift,none,
"change Swift name to %0", (DeclName))
NOTE(fixit_rename_in_objc,none,
"change Objective-C selector to %0", (ObjCSelector))
NOTE(remove_async_add_task,none,
"remove 'async' and wrap in 'Task' to use concurrency in %0", (DeclName))
ERROR(no_objc_tagged_pointer_not_class_protocol,none,
"@unsafe_no_objc_tagged_pointer can only be applied to class protocols",
())
Expand All @@ -1464,11 +1466,12 @@ ERROR(cdecl_empty_name,none,
"@_cdecl symbol name cannot be empty", ())
ERROR(cdecl_throws,none,
"raising errors from @_cdecl functions is not supported", ())
ERROR(cdecl_async,none,
"@_cdecl functions cannot be asynchronous", ())

ERROR(attr_methods_only,none,
"only methods can be declared %0", (DeclAttribute))
ERROR(attr_decl_async,none,
"@%0 %1 cannot be asynchronous", (StringRef, DescriptiveDeclKind))

ERROR(access_control_in_protocol,none,
"%0 modifier cannot be used in protocols", (DeclAttribute))
NOTE(access_control_in_protocol_detail,none,
Expand Down
76 changes: 76 additions & 0 deletions lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,75 @@ void AttributeChecker::visitDynamicAttr(DynamicAttr *attr) {
diagnoseAndRemoveAttr(attr, diag::dynamic_with_transparent);
}

/// Replaces asynchronous IBActionAttr/IBSegueActionAttr function declarations
/// with a synchronous function. The body of the original function is moved
/// inside of a task executed on the MainActor
static void emitFixItIBActionRemoveAsync(ASTContext &ctx, const FuncDecl &FD) {
// If we don't have an async loc for some reason, things will explode
if (!FD.getAsyncLoc())
return;

std::string replacement = "";

// attributes, function name and everything up to `async` (exclusive)
replacement +=
CharSourceRange(ctx.SourceMgr, FD.getSourceRangeIncludingAttrs().Start,
FD.getAsyncLoc())
.str();

CharSourceRange returnType = Lexer::getCharSourceRangeFromSourceRange(
ctx.SourceMgr, FD.getResultTypeSourceRange());

// If we have a return type, include that here
if (returnType.isValid()) {
replacement +=
(llvm::Twine("-> ") + Lexer::getCharSourceRangeFromSourceRange(
ctx.SourceMgr, FD.getResultTypeSourceRange())
.str())
.str();
}

if (!FD.hasBody()) {
// If we don't have any body, the sourcelocs won't work and will result in
// crashes, so just swap out what we can

SourceLoc endLoc =
returnType.isValid() ? returnType.getEnd() : FD.getAsyncLoc();
ctx.Diags
.diagnose(FD.getAsyncLoc(), diag::remove_async_add_task, FD.getName())
.fixItReplace(
SourceRange(FD.getSourceRangeIncludingAttrs().Start, endLoc),
replacement);
return;
}

if (returnType.isValid())
replacement += " "; // insert space between type name and lbrace

replacement += "{\nTask { @MainActor in";

// If the body of the function is just "{}", there isn't anything to wrap.
// stepping over the braces to grab just the body will result in the `Start`
// location of the source range to come after the `End` of the range, and we
// will overflow. Dance around this by just appending the end of the fix to
// the replacement.
if (FD.getBody()->getLBraceLoc() !=
FD.getBody()->getRBraceLoc().getAdvancedLocOrInvalid(-1)) {
// We actually have a body, so add that to the string
CharSourceRange functionBody(
ctx.SourceMgr, FD.getBody()->getLBraceLoc().getAdvancedLocOrInvalid(1),
FD.getBody()->getRBraceLoc().getAdvancedLocOrInvalid(-1));
replacement += functionBody.str();
}
replacement += " }\n}";

ctx.Diags
.diagnose(FD.getAsyncLoc(), diag::remove_async_add_task, FD.getName())
.fixItReplace(SourceRange(FD.getSourceRangeIncludingAttrs().Start,
FD.getBody()->getRBraceLoc()),
replacement);
}

static bool
validateIBActionSignature(ASTContext &ctx, DeclAttribute *attr,
const FuncDecl *FD, unsigned minParameters,
Expand All @@ -501,6 +570,13 @@ validateIBActionSignature(ASTContext &ctx, DeclAttribute *attr,
valid = false;
}

if (FD->isAsyncContext()) {
ctx.Diags.diagnose(FD->getAsyncLoc(), diag::attr_decl_async,
attr->getAttrName(), FD->getDescriptiveKind());
emitFixItIBActionRemoveAsync(ctx, *FD);
valid = false;
}

// We don't need to check here that parameter or return types are
// ObjC-representable; IsObjCRequest will validate that.

Expand Down
5 changes: 3 additions & 2 deletions lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2972,8 +2972,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
if (isRepresentableInObjC(FD, reason, asyncConvention, errorConvention)) {
if (FD->hasAsync()) {
FD->setForeignAsyncConvention(*asyncConvention);
getASTContext().Diags.diagnose(CDeclAttr->getLocation(),
diag::cdecl_async);
getASTContext().Diags.diagnose(
CDeclAttr->getLocation(), diag::attr_decl_async,
CDeclAttr->getAttrName(), FD->getDescriptiveKind());
} else if (FD->hasThrows()) {
FD->setForeignErrorConvention(*errorConvention);
getASTContext().Diags.diagnose(CDeclAttr->getLocation(),
Expand Down
2 changes: 1 addition & 1 deletion test/attr/attr_cdecl_async.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@

// REQUIRES: concurrency

@_cdecl("async") // expected-error{{@_cdecl functions cannot be asynchronous}}
@_cdecl("async") // expected-error{{@_cdecl global function cannot be asynchronous}}
func asynchronous() async { }

20 changes: 20 additions & 0 deletions test/attr/attr_ibaction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,26 @@ class IBActionWrapperTy {
func moreMagic(_: AnyObject) -> () {} // no-warning
@objc @IBAction
func evenMoreMagic(_: AnyObject) -> () {} // no-warning

@available(SwiftStdlib 5.5, *)
@IBAction
func asyncIBActionNoSpace(_: AnyObject) async -> () {}
// expected-error@-1 {{@IBAction instance method cannot be async}}
// expected-note@-2 {{remove 'async' and wrap in 'Task' to use concurrency in 'asyncIBActionNoSpace'}}{{45:3-47:57=@available(SwiftStdlib 5.5, *)\n @IBAction\n func asyncIBActionNoSpace(_: AnyObject) -> () {\nTask { @MainActor in \}\n\}}}

@available(SwiftStdlib 5.5, *)
@IBAction
func asyncIBActionWithFullBody(_: AnyObject) async {
print("Hello World")
}
// expected-error@-3 {{@IBAction instance method cannot be async}}
// expected-note@-4 {{remove 'async' and wrap in 'Task' to use concurrency in 'asyncIBActionWithFullBody'}}{{51:3-55:4=@available(SwiftStdlib 5.5, *)\n @IBAction\n func asyncIBActionWithFullBody(_: AnyObject) {\nTask { @MainActor in\n print("Hello World")\n \}\n\}}}

@available(SwiftStdlib 5.5, *) @IBAction func asyncIBActionNoBody(_: AnyObject) async
// expected-error@-1 {{expected '{' in body of function declaration}}
// expected-error@-2 {{@IBAction instance method cannot be asynchronous}}
// expected-note@-3 {{remove 'async' and wrap in 'Task' to use concurrency in 'asyncIBActionNoBody}}{{3-88=@available(SwiftStdlib 5.5, *) @IBAction func asyncIBActionNoBody(_: AnyObject)}}

}

struct S { }
Expand Down
5 changes: 5 additions & 0 deletions test/attr/attr_ibsegueaction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ class IBActionWrapperTy {
func moreMagic(_: AnyObject, _: AnyObject, _: AnyObject) -> AnyObject {} // no-warning
@objc @IBSegueAction
func evenMoreMagic(_: AnyObject, _: AnyObject, _: AnyObject) -> AnyObject {} // no-warning

@available(SwiftStdlib 5.5, *) @IBSegueAction
func process(_: AnyObject, _: AnyObject, _: AnyObject) async -> AnyObject { }
// expected-error@-1 {{@IBSegueAction instance method cannot be asynchronous}}
// expected-note@-2 {{remove 'async' and wrap in 'Task' to use concurrency in 'process'}}{{49:3-50:80=@available(SwiftStdlib 5.5, *) @IBSegueAction\n func process(_: AnyObject, _: AnyObject, _: AnyObject) -> AnyObject {\nTask { @MainActor in \}\n\}}}
}

struct S { }
Expand Down