Skip to content
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

Jump to bottom

[SR-13760] Existing calls to ObjC methods are unexpectedly 'async' #56157

Closed
kavon opened this issue Oct 21, 2020 · 2 comments
Closed

[SR-13760] Existing calls to ObjC methods are unexpectedly 'async' #56157

kavon opened this issue Oct 21, 2020 · 2 comments
Assignees
@kavon
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself concurrency Feature: umbrella label for concurrency language features

Comments

@kavon
Copy link
Contributor

kavon commented Oct 21, 2020

Previous ID SR-13760
Radar rdar://problem/70543421
Original Reporter @kavon
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment

MacOS 11 SDK, Xcode 12.2 Beta 3

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, Concurrency
Assignee @kavon
Priority Medium

md5: 25b07271a8912c747ceea50b3a43c7e7

Issue Description:

We get an unexpected error in the following code,

import AppKit

func syncFunc() {
  let sheet = NSWindow()
  let window = NSWindow()
  window.beginSheet(sheet)
}

at the synchronous call to NSWindow's beginSheet(_:completionHandler: ) method. The error states 'async' in a function that does not support concurrency.

According to the XCode Jump-to-Definition, the Swift declaration is:

open class NSWindow : ... {
// ...
@available(macOS 10.9, *)
    open func beginSheet(_ sheetWindow: NSWindow, completionHandler handler: ((NSApplication.ModalResponse) -> Void)? = nil)
// ...
}

I believe the 'async-looking' rule should be kicking in for this ObjC method to import two versions of the method (both async and sync) because of its declaration in NSWindow, which according to NSWindow.h for MacOS 11 SDK from XCode 12.2 beta 3 is:

// Combined ObjC sources
@interface NSWindow : T // where T is a whole lot of type names
// ...
- (void)beginSheet:(NSWindow *)sheetWindow completionHandler:(void (^ _Nullable)(NSModalResponse returnCode))handler API_AVAILABLE(macos(10.9));
// ...
@end

But, attempting to duplicate this declaration in a separate ObjC class called from Swift does not trigger the bug:

#import <AppKit/AppKit.h>

@interface MyNSWindow : NSWindow

- (void)myBeginSheet:(NSWindow *)sheetWindow completionHandler:(void (^ _Nullable)(NSModalResponse returnCode))handler API_AVAILABLE(macos(10.9));

@end

@implementation MyNSWindow

- (void)myBeginSheet:(NSWindow *)sheetWindow completionHandler:(void (^ _Nullable)(NSModalResponse returnCode))handler API_AVAILABLE(macos(10.9)) {
  return;
}

@end
// main.swift
func okMethods1() {
  let sheet = NSWindow()
  let window = MyNSWindow()
  window.myBeginSheet(sheet) // OK
}

The difference here is that Swift type synthesized from the declaration of MyNSWindow:myBeginSheet: says that sheetWindow is an {{NSWindow![](}}, not just an {{NSWindow}} as in {{NSWindow:myBeginSheet:}}, even though I just copied the method decl as-is from the header. The ) version of the type is what I would expect, because the nullability of the pointer is not specified. So, if you specify _Nonnull on MyNSWindow:myBeginSheet:'s sheetWindow then the error will be triggered on this example.

The next interesting aspect of this bug is that if you make a call to NSWindow:beginSheet from a subclass, that call-site is not considered an async call (nor is the method making that call):

import AppKit

open class SwiftNSWindow : NSWindow {

  @available(macOS 10.9, *)
  open func myBeginSheet(_ sheetWindow: NSWindow, completionHandler handler: ((NSApplication.ModalResponse) -> Void)? = nil) {
    // +-- OK... but according to the error below, beginSheet is async. Paradox.
    // v
    self.beginSheet(sheetWindow, completionHandler: handler)
  }
  
}

// reminder: synchronous method
func okMethods2() {
  let sheet = NSWindow()
  let window = SwiftNSWindow()
  window.myBeginSheet(sheet)  // <-- OK, so it must be sync.
  window.beginSheet(sheet)  // <-- Error: beginSheet is async.
}

Things get a bit weirder when you vary the the combination of pointer's type and nullability specifier when crafting a reduced ObjC example that triggers this bug. Here's the complete reduced example demonstrating this:

// Delegate.h

#import <Foundation/Foundation.h>

@interface Request : NSObject

@end

@interface Delegate : NSObject

  // Error because request is _Nonnull AND not exactly 'NSObject'
- (void)causeError:(Request * _Nonnull)request completionHandler:(void (^ _Nullable)(void))handler;

  // OK. Changed the request parameter's type to NSObject, but kept it _Nonnull.
- (void)ok0:(NSObject * _Nonnull)request completionHandler:(void (^ _Nullable)(void))handler;

  // OK. Request object's pointer nullability is _Nullable.
- (void)ok1:(Request * _Nullable)request completionHandler:(void (^ _Nullable)(void))handler;

  // OK. Request object's pointer nullability is unspecified.
- (void)ok2:(Request *)request completionHandler:(void (^ _Nullable)(void))handler;

@end
// Delegate.m

#import "Delegate.h"

@implementation Request

@end

@implementation Delegate

- (void)causeError:(Request * _Nonnull)request completionHandler:(void (^ _Nullable)(void))handler { return; }

- (void)ok0:(NSObject * _Nonnull)request completionHandler:(void (^ _Nullable)(void))handler { return; }

- (void)ok1:(Request * _Nullable)request completionHandler:(void (^ _Nullable)(void))handler { return; }

- (void)ok2:(Request *)request completionHandler:(void (^ _Nullable)(void))handler { return; }

@end
func okMethods3() {
  let r = Request()
  let d = Delegate()
  // none of these trigger an error
  d.ok0(r)
  d.ok1(r)
  d.ok2(r)
}

func brokenMethods() {
  let r = Request()
  let d = Delegate()
  d.causeError(r)   // <-- Error: 'async' in a function that does not support concurrency
}

Debugging Progress

First, to compare what's going on with the difference between compareError and ok0, I've dumped and trimmed the constraint solver output and attached them here. From what I gather, the most relevant difference is the order in which the overload choices are considered and how they are scored:

// considerations made for the NSObject version, which performs correctly.
// (a snippet from ok_NSObject_solver_debug.txt)

disjunction [[locator@0x7f918bbc8250 [UnresolvedDot@test.swift:47:5 -> member]]]:$T1 bound to decl __ObjC.(file).Delegate.causeError(_:completionHandler:) : (Delegate) -> (NSObject, (() -> Void)?) -> Void [[locator@0x7f918bbc8250 [UnresolvedDot@test.swift:47:5 -> member]]];
or $T1 bound to decl __ObjC.(file).Delegate.causeError : (Delegate) -> (NSObject) async -> Void [[locator@0x7f918bbc8250 [UnresolvedDot@test.swift:47:5 -> member]]];
  (Request) -> $T3 applicable fn $T1 [[locator@0x7f918bbc8518 [Call@test.swift:47:5 -> apply function]]];

...

  (attempting disjunction choice $T1 bound to decl __ObjC.(file).Delegate.causeError(_:completionHandler:) : (Delegate) -> (NSObject, (() -> Void)?) -> Void [[locator@0x7f918bbc8250 [UnresolvedDot@test.swift:47:5 -> member]]];
    (overload set choice binding $T1 := (NSObject, (() -> Void)?) -> Void)
    (found solution 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0)
  )
  (attempting disjunction choice $T1 bound to decl __ObjC.(file).Delegate.causeError : (Delegate) -> (NSObject) async -> Void [[locator@0x7f918bbc8250 [UnresolvedDot@test.swift:47:5 -> member]]];
    (increasing score due to async/synchronous mismatch)
    (overload set choice binding $T1 := (NSObject) async -> Void)
    (solution is worse than the best solution)
  )

-----------------------------
// considerations for the erroneous one that uses Request instead
// (a snippet from broken_Request_solver_debug.txt)

disjunction [[locator@0x7fc3655a3c50 [UnresolvedDot@test.swift:47:5 -> member]]]:$T1 bound to decl __ObjC.(file).Delegate.causeError(_:completionHandler:) : (Delegate) -> (Request, (() -> Void)?) -> Void [[locator@0x7fc3655a3c50 [UnresolvedDot@test.swift:47:5 -> member]]]; 
or $T1 bound to decl __ObjC.(file).Delegate.causeError : (Delegate) -> (Request) async -> Void [[locator@0x7fc3655a3c50 [UnresolvedDot@test.swift:47:5 -> member]]];
  (Request) -> $T3 applicable fn $T1 [[locator@0x7fc3655a3f18 [Call@test.swift:47:5 -> apply function]]];

...

  (attempting disjunction choice $T1 bound to decl __ObjC.(file).Delegate.causeError : (Delegate) -> (Request) async -> Void [[locator@0x7fc3655a3c50 [UnresolvedDot@test.swift:47:5 -> member]]];
    (increasing score due to async/synchronous mismatch)
    (overload set choice binding $T1 := (Request) async -> Void)
    (found solution 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0)
  )

Notice that the disjunction includes both async and sync versions, but in the case with Request, the first choice "attempted," which is the async one, ends up being the final choice; there doesn't seem to be any output indicating that it attempted the sync version in that case.
@typesanitizer
Copy link

@swift-ci create

@kavon
Copy link
Contributor Author

kavon commented Oct 29, 2020

Fixed in #34468

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kavon kavon

Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself concurrency Feature: umbrella label for concurrency language features
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kavon @typesanitizer