Skip to content

[cxx-interop] Use "c function conventions" for static methods and ctors (not "cxx method conventions"). #63506

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 2 commits into from
Feb 12, 2023
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
29 changes: 20 additions & 9 deletions lib/SIL/IR/SILFunctionType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3005,6 +3005,13 @@ class CFunctionConventions : public CFunctionTypeConventions {
}

ResultConvention getResult(const TypeLowering &tl) const override {
// C++ constructors return indirectly.
// TODO: this may be different depending on the ABI, so we may have to
// check with clang here.
if (isa<clang::CXXConstructorDecl>(TheDecl)) {
return ResultConvention::Indirect;
}

if (isCFTypedef(tl, TheDecl->getReturnType())) {
// The CF attributes aren't represented in the type, so we need
// to check them here.
Expand Down Expand Up @@ -3109,15 +3116,19 @@ static CanSILFunctionType getSILFunctionTypeForClangDecl(
}

if (auto method = dyn_cast<clang::CXXMethodDecl>(clangDecl)) {
AbstractionPattern origPattern = AbstractionPattern::getCXXMethod(origType, method,
foreignInfo.self);
bool isMutating =
TC.Context.getClangModuleLoader()->isCXXMethodMutating(method);
auto conventions = CXXMethodConventions(method, isMutating);
return getSILFunctionType(TC, TypeExpansionContext::minimal(), origPattern,
substInterfaceType, extInfoBuilder, conventions,
foreignInfo, constant, constant, None,
ProtocolConformanceRef());
// Static methods and ctors should be lowered like plane functions
// (case below).
if (!isa<clang::CXXConstructorDecl>(method) || method->isStatic()) {
AbstractionPattern origPattern = AbstractionPattern::getCXXMethod(origType, method,
foreignInfo.self);
bool isMutating =
TC.Context.getClangModuleLoader()->isCXXMethodMutating(method);
auto conventions = CXXMethodConventions(method, isMutating);
return getSILFunctionType(TC, TypeExpansionContext::minimal(), origPattern,
substInterfaceType, extInfoBuilder, conventions,
foreignInfo, constant, constant, None,
ProtocolConformanceRef());
}
}

if (auto func = dyn_cast<clang::FunctionDecl>(clangDecl)) {
Expand Down
9 changes: 9 additions & 0 deletions test/Interop/Cxx/class/method/Inputs/methods.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,13 @@ struct HasMethods {
NonTrivialInWrapper constPassThroughAsWrapper(int a) const { return {a}; }
};

struct ReferenceParams {
int a;
int b;
ReferenceParams(const int &a, const int &b) : a(a), b(b) { }
static void staticMethod(const int &a, const int &b) {
ReferenceParams t{a, b};
}
};

#endif // TEST_INTEROP_CXX_CLASS_METHOD_METHODS_H
16 changes: 16 additions & 0 deletions test/Interop/Cxx/class/method/methods-silgen.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// RUN: %target-swift-emit-silgen -I %S/Inputs -enable-experimental-cxx-interop %s | %FileCheck %s

import Methods

// clang name: ReferenceParams::ReferenceParams
// CHECK: sil [clang ReferenceParams.init] @{{_ZN15ReferenceParamsC1ERKiS1_|\?\?0ReferenceParams@@QEAA@AEBH0@Z}} : $@convention(c) (@in_guaranteed Int32, @in_guaranteed Int32) -> @out ReferenceParams

// clang name: ReferenceParams::staticMethod
// CHECK: sil [clang ReferenceParams.staticMethod] @{{_ZN15ReferenceParams12staticMethodERKiS1_|\?staticMethod@ReferenceParams@@SAXAEBH0@Z}} : $@convention(c) (@in_guaranteed Int32, @in_guaranteed Int32) -> ()

public func use() {
let a = CInt(42)
let b = CInt(42)
_ = ReferenceParams(a, b)
ReferenceParams.staticMethod(a, b)
}
16 changes: 16 additions & 0 deletions test/Interop/Cxx/class/method/methods.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,20 @@ CxxMethodTestSuite.test("(Int) -> NonTrivialInWrapper") {
expectEqual(42, instance.constPassThroughAsWrapper(42).value)
}

CxxMethodTestSuite.test("Constructor with ref params") {
let a = CInt(42)
let b = CInt(11)
var instance = ReferenceParams(a, b)

expectEqual(42, instance.a)
expectEqual(11, instance.b)
}

// Just make sure we don't crash
CxxMethodTestSuite.test("Static method with ref params") {
let a = CInt(42)
let b = CInt(11)
ReferenceParams.staticMethod(a, b)
}

runAllTests()
4 changes: 2 additions & 2 deletions test/Interop/Cxx/reference/const-ref-parameter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ func testFunction() {
// CHECK-NEXT: apply [[FN4]]
// CHECK-SAME: : $@convention(objc_method) (@in_guaranteed OptionsStruct, @objc_metatype OptionsConsumerObjC.Type) -> Int32

// CHECK: [[FN5:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxxC1ERK13OptionsStruct : $@convention(c) (OptionsStruct) -> @out OptionsConsumerCxx
// CHECK: [[FN5:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxxC1ERK13OptionsStruct : $@convention(c) (@in_guaranteed OptionsStruct) -> @out OptionsConsumerCxx
// CHECK-NEXT: apply [[FN5]]
// CHECK-SAME: : $@convention(c) (OptionsStruct) -> @out OptionsConsumerCxx
// CHECK-SAME: : $@convention(c) (@in_guaranteed OptionsStruct) -> @out OptionsConsumerCxx

// CHECK: [[FN6:%[0-9]+]] = function_ref @_ZN18OptionsConsumerCxx12doOtherThingERK13OptionsStruct : $@convention(cxx_method) (@in_guaranteed OptionsStruct, @inout OptionsConsumerCxx) -> Float
// CHECK-NEXT: apply [[FN6]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ 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: {{%[0-9]+}} = function_ref @_ZN10IntWrapperC1ERKi : $@convention(c) (@in_guaranteed Int32) -> @out IntWrapper
// CHECK: {{%[0-9]+}} = apply {{%[0-9]+}}({{%[0-9]+}}, {{%[0-9]+}}) : $@convention(c) (@in_guaranteed Int32) -> @out IntWrapper
// CHECK: alloc_global @$s4main1cSo15ObjCSwiftBridgeCSgvp
// CHECK: {{%[0-9]+}} = global_addr @$s4main1cSo15ObjCSwiftBridgeCSgvp : $*Optional<ObjCSwiftBridge>
// CHECK: {{%[0-9]+}} = load {{%[0-9]+}} : $*IntWrapper
Expand Down