Merge pull request swiftlang#82161 from swiftlang/egorzhdan/template-param-nullability

egorzhdan · web-flow · commit f6e61728c5fb · 2025-06-17T16:51:47.000+01:00
[cxx-interop] Import nullability of templated function parameters correctly
diff --git a/lib/AST/ClangTypeConverter.cpp b/lib/AST/ClangTypeConverter.cpp
@@ -890,8 +890,15 @@ ClangTypeConverter::convertClangDecl(Type type, const clang::Decl *clangDecl) {
 
   if (auto clangTypeDecl = dyn_cast<clang::TypeDecl>(clangDecl)) {
     auto qualType = ctx.getTypeDeclType(clangTypeDecl);
-    if (type->isForeignReferenceType())
+    if (type->isForeignReferenceType()) {
       qualType = ctx.getPointerType(qualType);
+      auto nonNullAttr = new (ctx) clang::TypeNonNullAttr(
+          ctx,
+          clang::AttributeCommonInfo(
+              clang::SourceRange(), clang::AttributeCommonInfo::AT_TypeNonNull,
+              clang::AttributeCommonInfo::Form::Implicit()));
+      qualType = ctx.getAttributedType(nonNullAttr, qualType, qualType);
+    }
 
     return qualType.getUnqualifiedType();
   }
diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp
@@ -2458,6 +2458,13 @@ ClangImporter::Implementation::importParameterType(
   auto paramTy = desugarIfElaborated(param->getType());
   paramTy = desugarIfBoundsAttributed(paramTy);
 
+  // If this type has a _Nullable/_Nonnull attribute, drop it, since we already
+  // have that information in optionalityOfParam.
+  if (auto attributedTy = dyn_cast<clang::AttributedType>(paramTy)) {
+    if (attributedTy->getImmediateNullability())
+      clang::AttributedType::stripOuterNullability(paramTy);
+  }
+
   ImportTypeKind importKind = paramIsCompletionHandler
                                   ? ImportTypeKind::CompletionHandlerParameter
                                   : ImportTypeKind::Parameter;
@@ -2486,6 +2493,17 @@ ClangImporter::Implementation::importParameterType(
                                        pointerKind));
       return std::nullopt;
     }
+    switch (optionalityOfParam) {
+    case OTK_Optional:
+      swiftParamTy = OptionalType::get(swiftParamTy);
+      break;
+    case OTK_ImplicitlyUnwrappedOptional:
+      swiftParamTy = OptionalType::get(swiftParamTy);
+      isParamTypeImplicitlyUnwrapped = true;
+      break;
+    case OTK_None:
+      break;
+    }
   } else if (isa<clang::ReferenceType>(paramTy) &&
              isa<clang::TemplateTypeParmType>(paramTy->getPointeeType())) {
     // We don't support universal reference, bail.
@@ -2734,8 +2752,6 @@ ParameterList *ClangImporter::Implementation::importFunctionParameterList(
     }
 
     bool knownNonNull = !nonNullArgs.empty() && nonNullArgs[index];
-    // Specialized templates need to match the args/result exactly.
-    knownNonNull |= clangDecl->isFunctionTemplateSpecialization();
 
     // Check nullability of the parameter.
     OptionalTypeKind optionalityOfParam =
diff --git a/stdlib/public/Cxx/CxxSpan.swift b/stdlib/public/Cxx/CxxSpan.swift
@@ -66,7 +66,7 @@ public protocol CxxSpan<Element> {
   associatedtype Size: BinaryInteger
 
   init()
-  init(_ unsafePointer : UnsafePointer<Element>, _ count: Size)
+  init(_ unsafePointer: UnsafePointer<Element>!, _ count: Size)
 
   func size() -> Size
   func __dataUnsafe() -> UnsafePointer<Element>?
@@ -136,7 +136,7 @@ public protocol CxxMutableSpan<Element> {
   associatedtype Size: BinaryInteger
 
   init()
-  init(_ unsafeMutablePointer : UnsafeMutablePointer<Element>, _ count: Size)
+  init(_ unsafeMutablePointer: UnsafeMutablePointer<Element>!, _ count: Size)
 
   func size() -> Size
   func __dataUnsafe() -> UnsafeMutablePointer<Element>?
diff --git a/stdlib/public/Cxx/cxxshim/libcxxshim.h b/stdlib/public/Cxx/cxxshim/libcxxshim.h
@@ -1,2 +1,4 @@
 template <class From, class To>
-To __swift_interopStaticCast(From from) { return static_cast<To>(from); }
+To _Nonnull __swift_interopStaticCast(From _Nonnull from) {
+  return static_cast<To>(from);
+}
diff --git a/test/Interop/Cxx/stdlib/use-std-span-typechecker.swift b/test/Interop/Cxx/stdlib/use-std-span-typechecker.swift
@@ -15,7 +15,5 @@ arr.withUnsafeBufferPointer { ubpointer in
 arr.withUnsafeBufferPointer { ubpointer in 
     // FIXME: this crashes the compiler once we import span's templated ctors as Swift generics.
     let _ = ConstSpanOfInt(ubpointer.baseAddress, ubpointer.count)
-    // expected-error@-1 {{value of optional type 'UnsafePointer<Int32>?' must be unwrapped to a value of type 'UnsafePointer<Int32>'}}
-    // expected-note@-2 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}
-    // expected-note@-3 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
+    // expected-warning@-1 {{'init(_:_:)' is deprecated: use 'init(_:)' instead.}}
 }
diff --git a/test/Interop/Cxx/swiftify-import/bounds-attrs-in-template.swift b/test/Interop/Cxx/swiftify-import/bounds-attrs-in-template.swift
@@ -5,10 +5,10 @@
 // RUN: %target-swift-frontend -plugin-path %swift-plugin-dir -I %t/Inputs -cxx-interoperability-mode=default -enable-experimental-feature SafeInteropWrappers %t/template.swift -dump-macro-expansions -emit-ir -o %t/out -verify
 // RUN: %target-swift-ide-test -plugin-path %swift-plugin-dir -I %t/Inputs -cxx-interoperability-mode=default -enable-experimental-feature SafeInteropWrappers -print-module -module-to-print=Template -source-filename=x | %FileCheck %s
 
-// CHECK: func cb_template<T>(_ p: UnsafePointer<T>, _ size: Int{{.*}}) -> UnsafePointer<T>
-// CHECK: func eb_template<T>(_ p: UnsafePointer<T>, _ end: UnsafePointer<T>) -> UnsafePointer<T>
-// CHECK: func s_template<T>(_ p: UnsafePointer<T>) -> UnsafePointer<T>
-// CHECK: func ui_template<T>(_ p: UnsafePointer<T>) -> UnsafePointer<T>
+// CHECK: func cb_template<T>(_ p: UnsafePointer<T>!, _ size: Int{{.*}}) -> UnsafePointer<T>
+// CHECK: func eb_template<T>(_ p: UnsafePointer<T>!, _ end: UnsafePointer<T>!) -> UnsafePointer<T>
+// CHECK: func s_template<T>(_ p: UnsafePointer<T>!) -> UnsafePointer<T>
+// CHECK: func ui_template<T>(_ p: UnsafePointer<T>!) -> UnsafePointer<T>
 
 //--- Inputs/module.modulemap
 module Template {
diff --git a/test/Interop/Cxx/templates/Inputs/function-templates.h b/test/Interop/Cxx/templates/Inputs/function-templates.h
@@ -120,7 +120,9 @@ template <class T> bool constLvalueReferenceToBool(const T &t) { return t; }
 
 template <class T> void forwardingReference(T &&) {}
 
-template <class T> void PointerTemplateParameter(T*){}
+template <class T> bool pointerTemplateParameter(T *t) { return t; }
+template <class T> bool pointerTemplateParameterNonnull(T *_Nonnull t) { return t; }
+template <class T> bool pointerTemplateParameterNullable(T *_Nullable t) { return t; }
 
 template <typename F> void callFunction(F f) { f(); }
 template <typename F, typename T> void callFunctionWithParam(F f, T t) { f(t); }
diff --git a/test/Interop/Cxx/templates/defaulted-template-type-parameter-module-interface.swift b/test/Interop/Cxx/templates/defaulted-template-type-parameter-module-interface.swift
@@ -21,6 +21,6 @@
 // TODO-CHECK: func defaultedTemplatePointerTypeParam<T>(_ t: UnsafeMutablePointer<T>)
 // TODO-CHECK: func defaultedTemplatePointerPointerTypeParam<T>(_ t: UnsafeMutablePointer<OpaquePointer?>!)
 
-// CHECK: func defaultedTemplatePointerTypeParam<T>(_ t: UnsafeMutablePointer<T>)
+// CHECK: func defaultedTemplatePointerTypeParam<T>(_ t: UnsafeMutablePointer<T>!)
 // We don't support references to dependent types (rdar://89034440).
 // CHECK-NOT: defaultedTemplatePointerReferenceTypeParam
diff --git a/test/Interop/Cxx/templates/function-template-module-interface.swift b/test/Interop/Cxx/templates/function-template-module-interface.swift
@@ -20,7 +20,10 @@
 
 // CHECK: func lvalueReference<T>(_ ref: inout T)
 // CHECK: func constLvalueReference<T>(_: T)
-// CHECK: func PointerTemplateParameter<T>(_: UnsafeMutablePointer<T>)
+
+// CHECK: func pointerTemplateParameter<T>(_ t: UnsafeMutablePointer<T>!) -> Bool
+// CHECK: func pointerTemplateParameterNonnull<T>(_ t: UnsafeMutablePointer<T>) -> Bool
+// CHECK: func pointerTemplateParameterNullable<T>(_ t: UnsafeMutablePointer<T>?) -> Bool
 
 // CHECK: enum Orbiters {
 // CHECK:   static func galileo<T>(_: T)
diff --git a/test/Interop/Cxx/templates/function-template.swift b/test/Interop/Cxx/templates/function-template.swift
@@ -39,6 +39,26 @@ FunctionTemplateTestSuite.test("constLvalueReferenceToBool<T> where T == Bool")
   expectFalse(constLvalueReferenceToBool(false))
 }
 
+var nilPtr: UnsafeMutablePointer<CInt>? = nil
+var nilPtrIOU: UnsafeMutablePointer<CInt>! = nil
+var nonNilPtr: UnsafeMutablePointer<CInt> = .init(bitPattern: 123)!
+
+FunctionTemplateTestSuite.test("pointerTemplateParameter<T>") {
+  expectFalse(pointerTemplateParameter(nilPtr))
+  expectFalse(pointerTemplateParameter(nilPtrIOU))
+  expectTrue(pointerTemplateParameter(nonNilPtr))
+}
+
+FunctionTemplateTestSuite.test("pointerTemplateParameterNonnull<T>") {
+  expectTrue(pointerTemplateParameterNonnull(nonNilPtr))
+}
+
+FunctionTemplateTestSuite.test("pointerTemplateParameterNullable<T>") {
+  expectFalse(pointerTemplateParameterNullable(nilPtr))
+  expectFalse(pointerTemplateParameterNullable(nilPtrIOU))
+  expectTrue(pointerTemplateParameterNullable(nonNilPtr))
+}
+
 // TODO: Generics, Any, and Protocols should be tested here but need to be
 // better supported in ClangTypeConverter first.
 
diff --git a/test/Interop/Cxx/templates/template-instantiation-irgen.swift b/test/Interop/Cxx/templates/template-instantiation-irgen.swift
@@ -46,21 +46,20 @@ func takesPtrToStruct(x: UnsafePointer<PlainStruct>) { takesValue(x) }
 func takesPtrToClass(x: UnsafePointer<CxxClass>) { takesValue(x) }
 // CHECK: define {{.*}} void @{{.*}}takesPtrToClass{{.*}}
 
-// FIXME: this crashes because this round-trips to UnsafePointer<FRT?>
-// func takesPtrToFRT(x: UnsafePointer<FRT>) { takesValue(x) }
+func takesPtrToFRT(x: UnsafePointer<FRT>) { takesValue(x) }
+// CHECK: define {{.*}} void @{{.*}}takesPtrToFRT{{.*}}
 
 func takesMutPtrToStruct(x: UnsafeMutablePointer<PlainStruct>) { takesValue(x) }
 // CHECK: define {{.*}} void @{{.*}}takesMutPtrToStruct{{.*}}
 
 func takesMutPtrToClass(x: UnsafeMutablePointer<CxxClass>) { takesValue(x) }
 // CHECK: define {{.*}} void @{{.*}}takesMutPtrToClass{{.*}}
 
-// FIXME: this crashes because this round-trips to UnsafeMutablePointer<FRT?>
-// func takesMutPtrToFRT(x: UnsafeMutablePointer<FRT>) { takesValue(x) }
+func takesMutPtrToFRT(x: UnsafeMutablePointer<FRT>) { takesValue(x) }
+// CHECK: define {{.*}} void @{{.*}}takesMutPtrToFRT{{.*}}
 
 func takesCPtr() {
-  // FIXME: optional pointers are not yet supported but they should be; this crashes
-  // takesValue(intPtr)
+  takesValue(intPtr)
 
   // It's fine if we dereference it, though
   takesValue(intPtr!)
@@ -92,9 +91,6 @@ func takesSwiftClosureTakingCxxClass() { takesValue({(x: CxxClass) in takesValue
 func takesTakesCxxClass() { takesValue(takesCxxClass) }
 
 func takesSwiftClosureReturningFRT() { takesValue({() -> FRT in FRT()}) }
+func takesSwiftClosureTakingFRT() { takesValue({(x: FRT) in takesValue(x)}) }
 
-// FIXME: this crashes due to pointer round-tripping
-// func takesSwiftClosureTakingFRT() { takesValue({(x: FRT) in takesValue(x)}) }
-
-// FIXME: this crashes due to pointer round-tripping
-// func takesTakesFRT() { takesValue(takesFRT) }
+func takesTakesFRT() { takesValue(takesFRT) }

Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,5 @@ arr.withUnsafeBufferPointer { ubpointer in`
`15`	`15`	`arr.withUnsafeBufferPointer { ubpointer in`
`16`	`16`	`// FIXME: this crashes the compiler once we import span's templated ctors as Swift generics.`
`17`	`17`	`let _ = ConstSpanOfInt(ubpointer.baseAddress, ubpointer.count)`
`18`		`- // expected-error@-1 {{value of optional type 'UnsafePointer<Int32>?' must be unwrapped to a value of type 'UnsafePointer<Int32>'}}`
`19`		`- // expected-note@-2 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}`
`20`		`- // expected-note@-3 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}`
	`18`	`+ // expected-warning@-1 {{'init(_:_:)' is deprecated: use 'init(_:)' instead.}}`
`21`	`19`	`}`