[C++-Interop] Import const &T function parameters as @in_guaranteed

plotfi · plotfi · commit 834fe9aa3e38 · 2022-09-09T20:49:52.000-04:00
When passing value types as a const-ref to a C++ API, ensure that the
caller is tasked with handling the lifetime of the instance passed in.
diff --git a/lib/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp
@@ -2702,6 +2702,8 @@ static bool isCFTypedef(const TypeLowering &tl, clang::QualType type) {
 static ParameterConvention getIndirectCParameterConvention(clang::QualType type) {
   // Non-trivial C++ types would be Indirect_Inout (at least in Itanium).
   // A trivial const * parameter in C should be considered @in.
+  if (type->isReferenceType() && type->getPointeeType().isConstQualified())
+    return ParameterConvention::Indirect_In_Guaranteed;
   return ParameterConvention::Indirect_In;
 }
 
diff --git a/lib/SILGen/SILGenBridging.cpp b/lib/SILGen/SILGenBridging.cpp
@@ -2225,7 +2225,8 @@ void SILGenFunction::emitForeignToNativeThunk(SILDeclRef thunk) {
                                                 foreignFormalType,
                                                 foreignLoweredTy);
         // Handle C pointer arguments imported as indirect `self` arguments.
-        if (foreignParam.getConvention() == ParameterConvention::Indirect_In) {
+        if (foreignParam.getConvention() == ParameterConvention::Indirect_In ||
+            foreignParam.getConvention() == ParameterConvention::Indirect_In_Guaranteed) {
           auto temp = emitTemporaryAllocation(fd, bridged.getType());
           bridged.forwardInto(*this, fd, temp);
           bridged = emitManagedBufferWithCleanup(temp);
diff --git a/test/Interop/Cxx/reference/Inputs/module.modulemap b/test/Interop/Cxx/reference/Inputs/module.modulemap
@@ -13,3 +13,9 @@ module ConstRefParameter {
   requires objc
   requires cplusplus
 }
+
+module ConstRefCxxObjCCtorInitParameter {
+  header "reference-silgen-cxx-objc-ctors+init.h"
+  requires objc
+  requires cplusplus
+}
diff --git a/test/Interop/Cxx/reference/Inputs/reference-silgen-cxx-objc-ctors+init.h b/test/Interop/Cxx/reference/Inputs/reference-silgen-cxx-objc-ctors+init.h
@@ -0,0 +1,18 @@
+
+
+@interface NSObject
+@end
+
+struct PrimitiveWrapperT { int value; };
+
+struct CxxEmbeddedT {
+  PrimitiveWrapperT V;
+  CxxEmbeddedT() = delete;
+  CxxEmbeddedT(const PrimitiveWrapperT &V) noexcept : V(V) {}
+};
+
+
+@interface ObjCSwiftBridgeT : NSObject
+- (instancetype)init  __attribute__((unavailable));
+- (instancetype)initWithEmbedded:(const CxxEmbeddedT &)embedded __attribute__((objc_designated_initializer));
+@end
diff --git a/test/Interop/Cxx/reference/const-ref-parameter.swift b/test/Interop/Cxx/reference/const-ref-parameter.swift
@@ -36,42 +36,34 @@ func testFunction() {
 
 // RUN: %target-swift-frontend -c -enable-experimental-cxx-interop -enable-objc-interop -I %S/Inputs %s -emit-silgen -o - | %FileCheck %s
 
-// COM: FIXME: should it be @in_guaranteed OptionsStruct? https://github.com/apple/swift/issues/60601
 // CHECK: [[FN1:%[0-9]+]] = function_ref @$sSo19OptionsConsumerObjCC7optionsABSo0A6StructV_tcfC : $@convention(method) (OptionsStruct, @thick OptionsConsumerObjC.Type) -> @owned OptionsConsumerObjC
 // CHECK-NEXT: apply [[FN1]]
 // CHECK-SAME: : $@convention(method) (OptionsStruct, @thick OptionsConsumerObjC.Type) -> @owned OptionsConsumerObjC
 
-// COM: FIXME: should it be @in_guaranteed OptionStruct? https://github.com/apple/swift/issues/60601
-// CHECK: [[FN2:%[0-9]+]] = objc_method {{%[0-9]+}} : $OptionsConsumerObjC, #OptionsConsumerObjC.doOtherThing!foreign : (OptionsConsumerObjC) -> (OptionsStruct) -> Float, $@convention(objc_method) (@in OptionsStruct, OptionsConsumerObjC) -> Float
+// CHECK: [[FN2:%[0-9]+]] = objc_method {{%[0-9]+}} : $OptionsConsumerObjC, #OptionsConsumerObjC.doOtherThing!foreign : (OptionsConsumerObjC) -> (OptionsStruct) -> Float, $@convention(objc_method) (@in_guaranteed OptionsStruct, OptionsConsumerObjC) -> Float
 // CHECK-NEXT: apply [[FN2]]
-// CHECK-SAME: : $@convention(objc_method) (@in OptionsStruct, OptionsConsumerObjC) -> Float
+// CHECK-SAME: : $@convention(objc_method) (@in_guaranteed OptionsStruct, OptionsConsumerObjC) -> Float
 
-// COM: FIXME: should it be @in_guaranteed OptionStruct? https://github.com/apple/swift/issues/60601
-// CHECK: [[FN3:%[0-9]+]] = objc_method {{%[0-9]+}} : $@objc_metatype OptionsConsumerObjC.Type, #OptionsConsumerObjC.consumer!foreign : (OptionsConsumerObjC.Type) -> (OptionsStruct) -> @dynamic_self OptionsConsumerObjC, $@convention(objc_method) (@in OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> @autoreleased OptionsConsumerObjC
+// CHECK: [[FN3:%[0-9]+]] = objc_method {{%[0-9]+}} : $@objc_metatype OptionsConsumerObjC.Type, #OptionsConsumerObjC.consumer!foreign : (OptionsConsumerObjC.Type) -> (OptionsStruct) -> @dynamic_self OptionsConsumerObjC, $@convention(objc_method) (@in_guaranteed OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> @autoreleased OptionsConsumerObjC
 // CHECK-NEXT: apply [[FN3]]
-// CHECK-SAME: : $@convention(objc_method) (@in OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> @autoreleased OptionsConsumerObjC
+// CHECK-SAME: : $@convention(objc_method) (@in_guaranteed OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> @autoreleased OptionsConsumerObjC
 
-// COM: FIXME: should it be @in_guaranteed OptionStruct? https://github.com/apple/swift/issues/60601
-// CHECK: [[FN4:%[0-9]+]] = objc_method {{%[0-9]+}} : $@objc_metatype OptionsConsumerObjC.Type, #OptionsConsumerObjC.doThing!foreign : (OptionsConsumerObjC.Type) -> (OptionsStruct) -> Int32, $@convention(objc_method) (@in OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> Int32
+// CHECK: [[FN4:%[0-9]+]] = objc_method {{%[0-9]+}} : $@objc_metatype OptionsConsumerObjC.Type, #OptionsConsumerObjC.doThing!foreign : (OptionsConsumerObjC.Type) -> (OptionsStruct) -> Int32, $@convention(objc_method) (@in_guaranteed OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> Int32
 // CHECK-NEXT: apply [[FN4]]
-// CHECK-SAME: : $@convention(objc_method) (@in OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> Int32
+// CHECK-SAME: : $@convention(objc_method) (@in_guaranteed OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> Int32
 
-// COM: FIXME: should it be @in_guaranteed OptionStruct? https://github.com/apple/swift/issues/60601
 // CHECK: [[FN5:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxxC1ERK13OptionsStruct : $@convention(c) (OptionsStruct) -> @out OptionsConsumerCxx
 // CHECK-NEXT: apply [[FN5]]
 // CHECK-SAME: : $@convention(c) (OptionsStruct) -> @out OptionsConsumerCxx
 
-// COM: FIXME: should it be @in_guaranteed OptionStruct? https://github.com/apple/swift/issues/60601
-// CHECK: [[FN6:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxx12doOtherThingERK13OptionsStruct : $@convention(cxx_method) (@in OptionsStruct, @inout OptionsConsumerCxx) -> Float
+// CHECK: [[FN6:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxx12doOtherThingERK13OptionsStruct : $@convention(cxx_method) (@in_guaranteed OptionsStruct, @inout OptionsConsumerCxx) -> Float
 // CHECK-NEXT: apply [[FN6]]
-// CHECK-SAME: : $@convention(cxx_method) (@in OptionsStruct, @inout OptionsConsumerCxx) -> Float
+// CHECK-SAME: : $@convention(cxx_method) (@in_guaranteed OptionsStruct, @inout OptionsConsumerCxx) -> Float
 
-// COM: FIXME: should it be @in_guaranteed OptionStruct? https://github.com/apple/swift/issues/60601
-// CHECK: [[FN6:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxx5buildERK13OptionsStruct : $@convention(c) (@in OptionsStruct) -> OptionsConsumerCxx
+// CHECK: [[FN6:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxx5buildERK13OptionsStruct : $@convention(c) (@in_guaranteed OptionsStruct) -> OptionsConsumerCxx
 // CHECK-NEXT: apply [[FN6]]
-// CHECK-SAME: : $@convention(c) (@in OptionsStruct) -> OptionsConsumerCxx
+// CHECK-SAME: : $@convention(c) (@in_guaranteed OptionsStruct) -> OptionsConsumerCxx
 
-// COM: FIXME: should it be @in_guaranteed OptionStruct? https://github.com/apple/swift/issues/60601
-// CHECK: [[FN7:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxx7doThingERK13OptionsStruct : $@convention(c) (@in OptionsStruct) -> Int32
+// CHECK: [[FN7:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxx7doThingERK13OptionsStruct : $@convention(c) (@in_guaranteed OptionsStruct) -> Int32
 // CHECK-NEXT: apply [[FN7]]
-// CHECK-SAME: : $@convention(c) (@in OptionsStruct) -> Int32
+// CHECK-SAME: : $@convention(c) (@in_guaranteed OptionsStruct) -> Int32
diff --git a/test/Interop/Cxx/reference/reference-silgen-cxx-objc-ctors+init.swift b/test/Interop/Cxx/reference/reference-silgen-cxx-objc-ctors+init.swift
@@ -0,0 +1,21 @@
+// REQUIRES: objc_interop
+// RUN: %target-swift-emit-sil -I %S/Inputs -enable-experimental-cxx-interop -Xcc -std=c++17 -Ounchecked %s | %FileCheck %s
+
+import ConstRefCxxObjCCtorInitParameter
+
+var primativeWrapperInst = PrimitiveWrapperT(value: 42)
+var cxxEmeddedInst = CxxEmbeddedT(primativeWrapperInst)
+var objcSwiftBridgeInst = ObjCSwiftBridgeT(embedded: cxxEmeddedInst)
+
+// FIXME: the const-ref C++ Consructor here is not getting an @in_guaranteed or even an @in convention here.
+// CHECK: {{%[0-9]+}} = function_ref @_ZN12CxxEmbeddedTC1ERK17PrimitiveWrapperT : $@convention(c) (PrimitiveWrapperT) -> @out CxxEmbeddedT
+// CHECK: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(c) (PrimitiveWrapperT) -> @out CxxEmbeddedT
+// CHECK: alloc_global @$s4main19objcSwiftBridgeInstSo09ObjCSwiftD1TCSgvp
+// CHECK: {{%[0-9]+}} = global_addr @$s4main19objcSwiftBridgeInstSo09ObjCSwiftD1TCSgvp : $*Optional<ObjCSwiftBridgeT>
+// CHECK: {{%[0-9]+}} = load {{%[0-9]+}} : $*CxxEmbeddedT
+// CHECK: {{%[0-9]+}} = alloc_ref [objc] $ObjCSwiftBridgeT
+// CHECK: {{%[0-9]+}} = alloc_stack $CxxEmbeddedT
+// CHECK: store {{%[0-9]+}} to {{%[0-9]+}} : $*CxxEmbeddedT
+// CHECK: {{%[0-9]+}} = objc_method {{%[0-9]+}} : $ObjCSwiftBridgeT, #ObjCSwiftBridgeT.init!initializer.foreign : (ObjCSwiftBridgeT.Type) -> (CxxEmbeddedT) -> ObjCSwiftBridgeT?, $@convention(objc_method) (@in_guaranteed CxxEmbeddedT, @owned ObjCSwiftBridgeT) -> @owned Optional<ObjCSwiftBridgeT>
+// CHECK: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(objc_method) (@in_guaranteed CxxEmbeddedT, @owned ObjCSwiftBridgeT) -> @owned Optional<ObjCSwiftBridgeT>
+// CHECK: dealloc_stack {{%[0-9]+}} : $*CxxEmbeddedT
diff --git a/test/Interop/Cxx/reference/reference-silgen.swift b/test/Interop/Cxx/reference/reference-silgen.swift
@@ -49,8 +49,8 @@ func setCxxConstRef() {
 }
 
 // CHECK: sil hidden @$s4main14setCxxConstRefyyF : $@convention(thin) () -> ()
-// CHECK: [[REF:%.*]] = function_ref @{{_Z20setConstStaticIntRefRKi|\?setConstStaticIntRef@@YAXAEBH@Z}} : $@convention(c) (@in Int32) -> ()
-// CHECK: apply [[REF]](%{{[0-9]+}}) : $@convention(c) (@in Int32) -> ()
+// CHECK: [[REF:%.*]] = function_ref @{{_Z20setConstStaticIntRefRKi|\?setConstStaticIntRef@@YAXAEBH@Z}} : $@convention(c) (@in_guaranteed Int32) -> ()
+// CHECK: apply [[REF]](%{{[0-9]+}}) : $@convention(c) (@in_guaranteed Int32) -> ()
 
 func setCxxRvalueRef() {
   var val: CInt = 21
@@ -67,5 +67,5 @@ func setCxxConstRvalueRef() {
 }
 
 // CHECK: sil hidden @$s4main20setCxxConstRvalueRefyyF : $@convention(thin) () -> ()
-// CHECK: [[REF:%.*]] = function_ref @{{_Z26setConstStaticIntRvalueRefOKi|\?setConstStaticIntRvalueRef@@YAX\$\$QEBH@Z}} : $@convention(c) (@in Int32) -> ()
-// CHECK: apply [[REF]](%{{[0-9]+}}) : $@convention(c) (@in Int32) -> ()
+// CHECK: [[REF:%.*]] = function_ref @{{_Z26setConstStaticIntRvalueRefOKi|\?setConstStaticIntRvalueRef@@YAX\$\$QEBH@Z}} : $@convention(c) (@in_guaranteed Int32) -> ()
+// CHECK: apply [[REF]](%{{[0-9]+}}) : $@convention(c) (@in_guaranteed Int32) -> ()
diff --git a/test/Interop/Cxx/templates/member-templates-silgen.swift b/test/Interop/Cxx/templates/member-templates-silgen.swift
@@ -17,8 +17,8 @@ import MemberTemplates
 // CHECK: [[ADD_ALL:%.*]] = function_ref @_ZN18HasMemberTemplates6addAllIiiEEiiT_T0_ : $@convention(cxx_method) (Int32, Int32, Int32, @inout HasMemberTemplates) -> Int32
 // CHECK: apply [[ADD_ALL]]({{.*}}) : $@convention(cxx_method) (Int32, Int32, Int32, @inout HasMemberTemplates) -> Int32
 
-// CHECK: [[DO_NOTHING:%.*]] = function_ref @_ZN18HasMemberTemplates17doNothingConstRefIiEEvRKT_ : $@convention(cxx_method) (@in Int32, @inout HasMemberTemplates) -> ()
-// CHECK: apply [[DO_NOTHING]]({{.*}}) : $@convention(cxx_method) (@in Int32, @inout HasMemberTemplates) -> ()
+// CHECK: [[DO_NOTHING:%.*]] = function_ref @_ZN18HasMemberTemplates17doNothingConstRefIiEEvRKT_ : $@convention(cxx_method) (@in_guaranteed Int32, @inout HasMemberTemplates) -> ()
+// CHECK: apply [[DO_NOTHING]]({{.*}}) : $@convention(cxx_method) (@in_guaranteed Int32, @inout HasMemberTemplates) -> ()
 
 // CHECK-LABEL: end sil function '$s4main9basicTestyyF'
 func basicTest() {
@@ -36,7 +36,7 @@ func basicTest() {
 
 // CHECK-LABEL: sil [clang HasMemberTemplates.addAll] @_ZN18HasMemberTemplates6addAllIiiEEiiT_T0_ : $@convention(cxx_method) (Int32, Int32, Int32, @inout HasMemberTemplates) -> Int32
 
-// CHECK-LABEL: sil [clang HasMemberTemplates.doNothingConstRef] @_ZN18HasMemberTemplates17doNothingConstRefIiEEvRKT_ : $@convention(cxx_method) (@in Int32, @inout HasMemberTemplates) -> ()
+// CHECK-LABEL: sil [clang HasMemberTemplates.doNothingConstRef] @_ZN18HasMemberTemplates17doNothingConstRefIiEEvRKT_ : $@convention(cxx_method) (@in_guaranteed Int32, @inout HasMemberTemplates) -> ()
 
 // CHECK-LABEL: sil hidden @$s4main12testSetValueyyF : $@convention(thin) () -> ()
 

Original file line number	Diff line number	Diff line change
`@@ -2702,6 +2702,8 @@ static bool isCFTypedef(const TypeLowering &tl, clang::QualType type) {`
`2702`	`2702`	`static ParameterConvention getIndirectCParameterConvention(clang::QualType type) {`
`2703`	`2703`	`// Non-trivial C++ types would be Indirect_Inout (at least in Itanium).`
`2704`	`2704`	`// A trivial const * parameter in C should be considered @in.`
	`2705`	`+ if (type->isReferenceType() && type->getPointeeType().isConstQualified())`
	`2706`	`+ return ParameterConvention::Indirect_In_Guaranteed;`
`2705`	`2707`	`return ParameterConvention::Indirect_In;`
`2706`	`2708`	`}`
`2707`	`2709`