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

SIL verification failed with ObjC protocol that throws #69421

Open
ellishg opened this issue Oct 25, 2023 · 5 comments
Open

SIL verification failed with ObjC protocol that throws #69421

ellishg opened this issue Oct 25, 2023 · 5 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels

Comments

@ellishg
Copy link
Contributor

ellishg commented Oct 25, 2023

Description
Swift is crashing when trying to compile an ObjC protocol that throws. The crash seems to go away when s has _Nullable.

Steps to reproduce

$ cat foo.h
#import <Foundation/Foundation.h>
@protocol PFoo<NSObject>
- (void)yesCrash:(void (^)(NSString *_Nonnull s, NSError *_Nullable e))completionHandler;
- (void)noCrash:(void (^)(NSString *_Nullable s, NSError *_Nullable e))completionHandler;
@end

$ cat foo.swift
import Foundation
@objc public final class Foo : NSObject, PFoo {
    public func yesCrash() async throws -> String { "hi" }
    public func noCrash() async throws -> String { "hi" }
}

$ swiftc foo.swift -import-objc-header foo.h -c
SIL verification failed: EnumInst must return an enum: ud
Verifying instruction:
->   %34 = enum $NSString, #Optional.none!enumelt // user: %36
     %36 = apply %30(%34, %35) : $@convention(block) (NSString, Optional<NSError>) -> ()
In function:
// @objc closure #1 in Foo.yesCrash()
sil shared [thunk] [ossa] @$s3foo3FooC8yesCrashSSyYaKFyyYacfU_To : $@convention(thin) @Sendable @async (Optional<@convention(block) (NSString, Optional<NSError>) -> ()>, Foo) -> () {
// %0                                             // user: %2
// %1                                             // user: %3
bb0(%0 : @unowned $Optional<@convention(block) (NSString, Optional<NSError>) -> ()>, %1 : @unowned $Foo):
  %2 = copy_block %0 : $Optional<@convention(block) (NSString, Optional<NSError>) -> ()> // users: %41, %27, %23, %8
  %3 = copy_value %1 : $Foo                       // users: %45, %4
  %4 = begin_borrow %3 : $Foo                     // users: %44, %6
  // function_ref Foo.yesCrash()
  %5 = function_ref @$s3foo3FooC8yesCrashSSyYaKF : $@convention(method) @async (@guaranteed Foo) -> (@owned String, @error any Error) // user: %6
  try_apply %5(%4) : $@convention(method) @async (@guaranteed Foo) -> (@owned String, @error any Error), normal bb1, error bb5 // id: %6

// %7                                             // users: %24, %13
bb1(%7 : @owned $String):                         // Preds: bb0
  %8 = begin_borrow %2 : $Optional<@convention(block) (NSString, Optional<NSError>) -> ()> // users: %22, %9
  switch_enum %8 : $Optional<@convention(block) (NSString, Optional<NSError>) -> ()>, case #Optional.some!enumelt: bb3, case #Optional.none!enumelt: bb2 // id: %9

bb2:                                              // Preds: bb1
  br bb4                                          // id: %10

// %11                                            // user: %18
bb3(%11 : @guaranteed $@convention(block) (NSString, Optional<NSError>) -> ()): // Preds: bb1
  // function_ref String._bridgeToObjectiveC()
  %12 = function_ref @$sSS10FoundationE19_bridgeToObjectiveCSo8NSStringCyF : $@convention(method) (@guaranteed String) -> @owned NSString // user: %14
  %13 = begin_borrow %7 : $String                 // users: %15, %14
  %14 = apply %12(%13) : $@convention(method) (@guaranteed String) -> @owned NSString // users: %20, %16
  end_borrow %13 : $String                        // id: %15
  %16 = begin_borrow %14 : $NSString              // users: %19, %18
  %17 = enum $Optional<NSError>, #Optional.none!enumelt // user: %18
  %18 = apply %11(%16, %17) : $@convention(block) (NSString, Optional<NSError>) -> ()
  end_borrow %16 : $NSString                      // id: %19
  destroy_value %14 : $NSString                   // id: %20
  br bb4                                          // id: %21

bb4:                                              // Preds: bb3 bb2
  end_borrow %8 : $Optional<@convention(block) (NSString, Optional<NSError>) -> ()> // id: %22
  destroy_value %2 : $Optional<@convention(block) (NSString, Optional<NSError>) -> ()> // id: %23
  destroy_value %7 : $String                      // id: %24
  br bb9                                          // id: %25

// %26                                            // users: %42, %32
bb5(%26 : @owned $any Error):                     // Preds: bb0
  %27 = begin_borrow %2 : $Optional<@convention(block) (NSString, Optional<NSError>) -> ()> // users: %40, %28
  switch_enum %27 : $Optional<@convention(block) (NSString, Optional<NSError>) -> ()>, case #Optional.some!enumelt: bb7, case #Optional.none!enumelt: bb6 // id: %28

bb6:                                              // Preds: bb5
  br bb8                                          // id: %29

// %30                                            // user: %36
bb7(%30 : @guaranteed $@convention(block) (NSString, Optional<NSError>) -> ()): // Preds: bb5
  // function_ref _convertErrorToNSError(_:)
  %31 = function_ref @$s10Foundation22_convertErrorToNSErrorySo0E0Cs0C0_pF : $@convention(thin) (@guaranteed any Error) -> @owned NSError // user: %32
  %32 = apply %31(%26) : $@convention(thin) (@guaranteed any Error) -> @owned NSError // user: %33
  %33 = enum $Optional<NSError>, #Optional.some!enumelt, %32 : $NSError // users: %38, %35
  %34 = enum $NSString, #Optional.none!enumelt    // user: %36
  %35 = begin_borrow %33 : $Optional<NSError>     // users: %37, %36
  %36 = apply %30(%34, %35) : $@convention(block) (NSString, Optional<NSError>) -> ()
  end_borrow %35 : $Optional<NSError>             // id: %37
  destroy_value %33 : $Optional<NSError>          // id: %38
  br bb8                                          // id: %39

bb8:                                              // Preds: bb7 bb6
  end_borrow %27 : $Optional<@convention(block) (NSString, Optional<NSError>) -> ()> // id: %40
  destroy_value %2 : $Optional<@convention(block) (NSString, Optional<NSError>) -> ()> // id: %41
  destroy_value %26 : $any Error                  // id: %42
  br bb9                                          // id: %43

bb9:                                              // Preds: bb8 bb4
  end_borrow %4 : $Foo                            // id: %44
  destroy_value %3 : $Foo                         // id: %45
  return undef : $()                              // id: %46
} // end

Expected behavior
The code should compile without issues.

Environment

  • Swift compiler version info 1.75.2 Apple Swift version 5.8.1 (swiftlang-5.8.0.124.5 clang-1403.0.22.11.100)
  • Xcode version info Xcode 14.3.1
  • Deployment target: iOS 13.4
@ellishg ellishg added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels labels Oct 25, 2023
@ellishg
Copy link
Contributor Author

ellishg commented Oct 25, 2023

CC @beccadax I've been told you work closely with ObjC interop. Could you take a look?

@beccadax
Copy link
Contributor

Hi @ellishg,

The s parameter actually has to be nullable for the code to be correct. After all, if s cannot be nil, what is Swift supposed to pass to it when yesCrash() throws an error?

Of course, that doesn't mean this obscure SIL verification crash is the right way for Swift to handle this code. My first thought is that Swift should simply not import the async variant of the method, which would force Foo to implement the completion-handler-based version of yesCrash(_:) instead. Do you have a different idea for this?

@ellishg
Copy link
Contributor Author

ellishg commented Oct 26, 2023

Hi @ellishg,

The s parameter actually has to be nullable for the code to be correct. After all, if s cannot be nil, what is Swift supposed to pass to it when yesCrash() throws an error?

Ah that makes sense. Is it possible to report an error when trying to import methods like yesCrash:?

Of course, that doesn't mean this obscure SIL verification crash is the right way for Swift to handle this code. My first thought is that Swift should simply not import the async variant of the method, which would force Foo to implement the completion-handler-based version of yesCrash(_:) instead. Do you have a different idea for this?

It seems to me that the async variant works well in most cases, but this special case ends up being an error. Or are you saying we could avoid importing the async variant only for yesCrash(_:)?

@beccadax
Copy link
Contributor

Yes, only for yesCrash(_:) (and other methods where the result or error parameter is _Nonnull).

@ellishg
Copy link
Contributor Author

ellishg commented Oct 27, 2023

Yes, only for yesCrash(_:) (and other methods where the result or error parameter is _Nonnull).

That sounds reasonable to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. triage needed This issue needs more specific labels
Projects
None yet
Development

No branches or pull requests

2 participants