Skip to content

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

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 1 commit into from
Sep 14, 2022
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
2 changes: 2 additions & 0 deletions lib/SIL/IR/SILFunctionType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
4 changes: 2 additions & 2 deletions lib/SILGen/SILGenBridging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions test/Interop/Cxx/reference/Inputs/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,9 @@ module ConstRefParameter {
requires objc
requires cplusplus
}

module ConstRefCxxObjCCtorInitParameter {
header "reference-silgen-cxx-objc-ctors+init.h"
requires objc
requires cplusplus
}
Original file line number Diff line number Diff line change
@@ -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
32 changes: 12 additions & 20 deletions test/Interop/Cxx/reference/const-ref-parameter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is the one that I was confused. IIUC $sSo19OptionsConsumerObjCC7optionsABSo0A6StructV_tcfC demangles to __C.OptionsConsumerObjC.__allocating_init(options: __C.OptionsStruct) -> __C.OptionsConsumerObjC which seems to be an Obj-C initializer, and it does not have @in nor @in_guaranteed. I don't know if this is required, or in this case it is fine to pass a copy.

But this one is different than in your new test, which shows a non-mangled name. I wonder if the designated initializer bit changes something.

// 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
Original file line number Diff line number Diff line change
@@ -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
8 changes: 4 additions & 4 deletions test/Interop/Cxx/reference/reference-silgen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) -> ()
6 changes: 3 additions & 3 deletions test/Interop/Cxx/templates/member-templates-silgen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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) () -> ()

Expand Down