Skip to content

Commit 3f9ac2f

Browse files
committed
[silgen] Add support for correctly calling imported throwing methods on imported objc classes that are imported with a non-null NSError.
This for some time has been crashing in IRGen in non-asserts builds and hitting assertions in asserts builds. I think we were missing test coverage here and that is why this basic-ish thing has been broken. Basically at a high level, the actual expected ABI here is that the NSError is marked non-nullable which is different from the expected ABI. rdar://92755102
1 parent d56cb4a commit 3f9ac2f

File tree

3 files changed

+53
-0
lines changed

3 files changed

+53
-0
lines changed

lib/SILGen/ResultPlan.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -758,6 +758,23 @@ class ForeignErrorInitializationPlan final : public ResultPlan {
758758

759759
auto errorType =
760760
CanType(unwrappedPtrType->getAnyPointerElementType(ptrKind));
761+
762+
// In cases when from swift, we call objc imported methods written like so:
763+
//
764+
// (1) - (BOOL)submit:(NSError *_Nonnull __autoreleasing *_Nullable)errorOut;
765+
//
766+
// the clang importer will successfully import the given method as having a
767+
// non-null NSError. This doesn't follow the normal convention where we
768+
// expect the NSError to be Optional<NSError>. In order to preserve source
769+
// compatibility, we want to allow SILGen to handle this behavior. Luckily
770+
// in this case, NSError and Optional<NSError> are layout compatible, so we
771+
// can just pass in the Optional<NSError> and everything works.
772+
if (auto nsErrorTy = SGF.getASTContext().getNSErrorType()->getCanonicalType()) {
773+
if (errorType == nsErrorTy) {
774+
errorType = errorType.wrapInOptionalType();
775+
}
776+
}
777+
761778
auto &errorTL = SGF.getTypeLowering(errorType);
762779

763780
// Allocate a temporary.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
@import Foundation;
2+
3+
@interface MyClass : NSObject {
4+
}
5+
6+
- (BOOL)submit:(NSError *_Nonnull __autoreleasing *_Nullable)errorOut;
7+
8+
@end
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %target-swift-frontend -import-objc-header %S/Inputs/throwing-mismarked-nonnullable-error.h %s -emit-sil | %FileCheck %s
2+
3+
// Make sure that we do not crash when importing a non-null NSError as
4+
// throwing. We really just shouldn't expect this at all. I filed: rdar://94656178
5+
// to track that work.
6+
7+
// CHECK-LABEL: sil hidden @$s4main1fyyF : $@convention(thin) () -> () {
8+
// CHECK: [[STACK:%.*]] = alloc_stack [dynamic_lifetime] $Optional<NSError>
9+
// CHECK: inject_enum_addr [[STACK]] : $*Optional<NSError>, #Optional.none!enumelt
10+
// CHECK: [[FN:%.*]] = objc_method {{%[0-9]+}} : $MyClass, #MyClass.submit!foreign : (MyClass) -> () throws -> (), $@convention(objc_method) (Optional<AutoreleasingUnsafeMutablePointer<NSError>>, MyClass) -> ObjCBool
11+
// CHECK: [[NEXT_STACK:%.*]] = alloc_stack $@sil_unmanaged Optional<NSError>
12+
// CHECK: [[VAL:%.*]] = load [[STACK]] : $*Optional<NSError>
13+
// CHECK: [[UNMANAGED:%.*]] = ref_to_unmanaged [[VAL]]
14+
// CHECK: store [[UNMANAGED]] to [[NEXT_STACK]]
15+
// CHECK: [[PTR:%.*]] = address_to_pointer [[NEXT_STACK]]
16+
// CHECK: [[AUMP:%.*]] = struct $AutoreleasingUnsafeMutablePointer<NSError> ([[PTR]] :
17+
// CHECK: [[OPT_AUMP:%.*]] = enum $Optional<AutoreleasingUnsafeMutablePointer<NSError>>, #Optional.some!enumelt, [[AUMP]] : $AutoreleasingUnsafeMutablePointer<NSError>
18+
// CHECK: apply {{%.*}}([[OPT_AUMP]],
19+
// CHECK: } // end sil function '$s4main1fyyF'
20+
21+
func f() {
22+
let c = MyClass()
23+
do {
24+
try c.submit()
25+
} catch {}
26+
}
27+
28+
f()

0 commit comments

Comments
 (0)