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

plotfi · plotfi · commit 84a69c414366 · 2022-09-12T16:27:27.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
@@ -2224,8 +2224,8 @@ void SILGenFunction::emitForeignToNativeThunk(SILDeclRef thunk) {
         auto bridged = emitNativeToBridgedValue(fd, param, nativeFormalType,
                                                 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,15 @@
+
+
+@interface NSObject
+@end
+
+struct IntWrapper {
+  int value;
+  IntWrapper() = delete;
+  IntWrapper(const int &value): value(value) {};
+};
+
+@interface ObjCSwiftBridge : NSObject
+- (instancetype)init  __attribute__((unavailable));
+- (instancetype)initWithEmbedded:(const IntWrapper &)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 a: Int32 = 32
+var b = IntWrapper(a)
+var c = ObjCSwiftBridge(embedded: b)
+
+// FIXME: the const-ref C++ Consructor here is not getting an @in_guaranteed or even an @in convention here.
+// CHECK: {{%[0-9]+}} = function_ref @_ZN10IntWrapperC1ERKi : $@convention(c) (Int32) -> @out IntWrapper
+// CHECK: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(c) (Int32) -> @out IntWrapper
+// CHECK: alloc_global @$s4main1cSo15ObjCSwiftBridgeCSgvp
+// CHECK: {{%[0-9]+}} = global_addr @$s4main1cSo15ObjCSwiftBridgeCSgvp : $*Optional<ObjCSwiftBridge>
+// CHECK: {{%[0-9]+}} = load {{%[0-9]+}} : $*IntWrapper
+// CHECK: {{%[0-9]+}} = alloc_ref [objc] $ObjCSwiftBridge
+// CHECK: {{%[0-9]+}} = alloc_stack $IntWrapper
+// CHECK: store {{%[0-9]+}} to {{%[0-9]+}} : $*IntWrapper
+// CHECK: {{%[0-9]+}} = objc_method {{%[0-9]+}} : $ObjCSwiftBridge, #ObjCSwiftBridge.init!initializer.foreign : (ObjCSwiftBridge.Type) -> (IntWrapper) -> ObjCSwiftBridge?, $@convention(objc_method) (@in_guaranteed IntWrapper, @owned ObjCSwiftBridge) -> @owned Optional<ObjCSwiftBridge>
+// CHECK: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(objc_method) (@in_guaranteed IntWrapper, @owned ObjCSwiftBridge) -> @owned Optional<ObjCSwiftBridge>
+// CHECK: dealloc_stack {{%[0-9]+}} : $*IntWrapper
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`