-
Notifications
You must be signed in to change notification settings - Fork 166
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
Improve error handling in Objective-C and Swift bindings #555
base: main
Are you sure you want to change the base?
Conversation
We're about to add a bunch of "try"s around this code. `assert` doesn't support `try`, but XCTAssert does. We use a few variants like XCTAssert Equal/True/Nil.
NSException indicates an unrecoverable runtime exception, and as a result cannot be caught in Swift. While it's possible to catch in Objective-C, it's still less ergonomic than the typical (NSError**) argument. This change moves the Objective-C API to using the (NSError**) style errors, and a throwing Swift API with an error enum. All methods that could reasonably throw have been converted, even though some do not currently throw, so that future changes, or errors surfaced from the C++ code, can be propagated without breaking changes.
- (Boolean)contains:(USearchKey)key error:(NSError**)error { | ||
return _native->contains(key); | ||
} | ||
|
||
- (UInt32)count:(USearchKey)key { | ||
- (UInt32)count:(USearchKey)key error:(NSError**)error { | ||
return static_cast<UInt32>(_native->count(key)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no way to highlight those functions as noexcept
in C++ terms? That why they probably still be accessed as computed properties of the class instance, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. If you think the implementation of these will never throw, and don't want to reserve the ability to throw in the API then I can drop these though.
This is a breaking change to the Objective-C and Swift bindings.
Why? – Because
NSException
which is currently used, is designed for unrecoverable runtime exceptions, not control flow. In Objective-C this is awkward to handle, and in Swift it's impossible to catch.This change switches from throwing
NSException
s to returningNSError
s, in the style recommended for handling cross-language errors in the Apple documentation: https://developer.apple.com/documentation/swift/handling-cocoa-errors-in-swift. This is worth a breaking change in order to accurately represent the possibilities of errors in the API contracts in both languages.The Objective-C APIs include an
(NSError**)error
argument, allowing callsites to pass an empty error pointer to be populated for error handling. This is a very common pattern in Objective-C APIs.In Swift, this is translated to a throwing error in the normal Swift way:
Additionally, the error enum is bridged to Swift so that errors can be disambiguated.
Fixes: #554