Skip to content

[silgen] Teach SILGen how to handle a corner case where the clang importer is improperly importing a method with a non-null NSError as throwing #59334

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
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
8 changes: 8 additions & 0 deletions include/swift/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ class CanType : public Type {
static bool isObjCExistentialTypeImpl(CanType type);
static bool isTypeErasedGenericClassTypeImpl(CanType type);
static CanType getOptionalObjectTypeImpl(CanType type);
static CanType wrapInOptionalTypeImpl(CanType type);
static CanType getReferenceStorageReferentImpl(CanType type);
static CanType getWithoutSpecifierTypeImpl(CanType type);

Expand Down Expand Up @@ -523,6 +524,13 @@ class CanType : public Type {

bool isForeignReferenceType(); // in Types.h

/// Return this type wrapped into an Optional type. E.x.: 'T' ->
/// 'Optional<T>'.
CanType wrapInOptionalType() const {
return wrapInOptionalTypeImpl(*this);
}

/// If this is a type Optional<T>, return T. Otherwise return CanType().
CanType getOptionalObjectType() const {
return getOptionalObjectTypeImpl(*this);
}
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -1232,6 +1232,9 @@ class alignas(1 << TypeAlignInBits) TypeBase
const ValueDecl *derivedDecl,
Type memberType);

/// If this type is T, return Optional<T>.
Type wrapInOptionalType() const;

/// Return T if this type is Optional<T>; otherwise, return the null type.
Type getOptionalObjectType();

Expand Down
11 changes: 11 additions & 0 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,17 @@ CanType CanType::getOptionalObjectTypeImpl(CanType type) {
return CanType();
}

Type TypeBase::wrapInOptionalType() const {
auto &ctx = getASTContext();
auto *result = BoundGenericEnumType::get(ctx.getOptionalDecl(), Type(),
{ getCanonicalType() });
return result;
}

CanType CanType::wrapInOptionalTypeImpl(CanType type) {
return type->wrapInOptionalType()->getCanonicalType();
}

Type TypeBase::getAnyPointerElementType(PointerTypeKind &PTK) {
auto &C = getASTContext();
if (isUnsafeMutableRawPointer()) {
Expand Down
6 changes: 2 additions & 4 deletions lib/SIL/IR/SILType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,8 @@ SILType SILType::getBuiltinWordType(const ASTContext &C) {
}

SILType SILType::getOptionalType(SILType type) {
auto &ctx = type.getASTContext();
auto optType = BoundGenericEnumType::get(ctx.getOptionalDecl(), Type(),
{ type.getASTType() });
return getPrimitiveType(CanType(optType), type.getCategory())
return getPrimitiveType(type.getASTType().wrapInOptionalType(),
type.getCategory())
.copyingMoveOnlyWrapper(type);
}

Expand Down
17 changes: 17 additions & 0 deletions lib/SILGen/ResultPlan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,23 @@ class ForeignErrorInitializationPlan final : public ResultPlan {

auto errorType =
CanType(unwrappedPtrType->getAnyPointerElementType(ptrKind));

// In cases when from swift, we call objc imported methods written like so:
//
// (1) - (BOOL)submit:(NSError *_Nonnull __autoreleasing *_Nullable)errorOut;
//
// the clang importer will successfully import the given method as having a
// non-null NSError. This doesn't follow the normal convention where we
// expect the NSError to be Optional<NSError>. In order to preserve source
// compatibility, we want to allow SILGen to handle this behavior. Luckily
// in this case, NSError and Optional<NSError> are layout compatible, so we
// can just pass in the Optional<NSError> and everything works.
if (auto nsErrorTy = SGF.getASTContext().getNSErrorType()->getCanonicalType()) {
if (errorType == nsErrorTy) {
errorType = errorType.wrapInOptionalType();
}
}

auto &errorTL = SGF.getTypeLowering(errorType);

// Allocate a temporary.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
@import Foundation;

@interface MyClass : NSObject {
}

- (BOOL)submit:(NSError *_Nonnull __autoreleasing *_Nullable)errorOut;

@end
30 changes: 30 additions & 0 deletions test/ClangImporter/throwing-mismarked-nonnullable-error.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// RUN: %target-swift-frontend -enable-objc-interop -import-objc-header %S/Inputs/throwing-mismarked-nonnullable-error.h %s -emit-sil | %FileCheck %s

// REQUIRES: objc_interop

// Make sure that we do not crash when importing a non-null NSError as
// throwing. We really just shouldn't expect this at all. I filed: rdar://94656178
// to track that work.

// CHECK-LABEL: sil hidden @$s4main1fyyF : $@convention(thin) () -> () {
// CHECK: [[STACK:%.*]] = alloc_stack [dynamic_lifetime] $Optional<NSError>
// CHECK: inject_enum_addr [[STACK]] : $*Optional<NSError>, #Optional.none!enumelt
// CHECK: [[FN:%.*]] = objc_method {{%[0-9]+}} : $MyClass, #MyClass.submit!foreign : (MyClass) -> () throws -> (), $@convention(objc_method) (Optional<AutoreleasingUnsafeMutablePointer<NSError>>, MyClass) -> ObjCBool
// CHECK: [[NEXT_STACK:%.*]] = alloc_stack $@sil_unmanaged Optional<NSError>
// CHECK: [[VAL:%.*]] = load [[STACK]] : $*Optional<NSError>
// CHECK: [[UNMANAGED:%.*]] = ref_to_unmanaged [[VAL]]
// CHECK: store [[UNMANAGED]] to [[NEXT_STACK]]
// CHECK: [[PTR:%.*]] = address_to_pointer [[NEXT_STACK]]
// CHECK: [[AUMP:%.*]] = struct $AutoreleasingUnsafeMutablePointer<NSError> ([[PTR]] :
// CHECK: [[OPT_AUMP:%.*]] = enum $Optional<AutoreleasingUnsafeMutablePointer<NSError>>, #Optional.some!enumelt, [[AUMP]] : $AutoreleasingUnsafeMutablePointer<NSError>
// CHECK: apply {{%.*}}([[OPT_AUMP]],
// CHECK: } // end sil function '$s4main1fyyF'

func f() {
let c = MyClass()
do {
try c.submit()
} catch {}
}

f()