Merge pull request #59334 from gottesmm/pr-499eee4e1aca5a8f3f38d1aeb7507c6e761eccd7

[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

Author: gottesmm

include/swift/AST/Type.h

@@ -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);
 
@@ -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);
   }

include/swift/AST/Types.h

@@ -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();
 

lib/AST/Type.cpp

@@ -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()) {

lib/SIL/IR/SILType.cpp

@@ -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);
 }
 

lib/SILGen/ResultPlan.cpp

@@ -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.

test/ClangImporter/Inputs/throwing-mismarked-nonnullable-error.h

@@ -0,0 +1,8 @@
+@import Foundation;
+
+@interface MyClass : NSObject {
+}
+
+- (BOOL)submit:(NSError *_Nonnull __autoreleasing *_Nullable)errorOut;
+
+@end

test/ClangImporter/throwing-mismarked-nonnullable-error.swift

@@ -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()