Skip to content

Commit 33778fa

Browse files
author
Gabor Horvath
committed
[cxx-interop] Fix calling convention for rvalue reference params
In C++, we always expected to invoke the dtor for moved-from objects. This is not the case for swift. Fortunately, @incxx calling convention is already expressing that the caller supposed to destroy the object. This fixes the missing dtor calls when calling C++ functions taking rvalue references. Fixes #77894. rdar://140786022
1 parent 09003d6 commit 33778fa

File tree

5 files changed

+74
-1
lines changed

5 files changed

+74
-1
lines changed

lib/SIL/IR/SILFunctionType.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,6 +1256,9 @@ class Conventions {
12561256
case ValueOwnership::Shared:
12571257
return ParameterConvention::Indirect_In_Guaranteed;
12581258
case ValueOwnership::Owned:
1259+
if (kind == ConventionsKind::CFunction ||
1260+
kind == ConventionsKind::CFunctionType)
1261+
return getIndirectParameter(index, type, substTL);
12591262
return ParameterConvention::Indirect_In;
12601263
}
12611264
llvm_unreachable("unhandled ownership");
@@ -3392,6 +3395,8 @@ static ParameterConvention getIndirectCParameterConvention(clang::QualType type)
33923395
// A trivial const * parameter in C should be considered @in.
33933396
if (importer::isCxxConstReferenceType(type.getTypePtr()))
33943397
return ParameterConvention::Indirect_In_Guaranteed;
3398+
if (type->isRValueReferenceType())
3399+
return ParameterConvention::Indirect_In_CXX;
33953400
if (auto *decl = type->getAsRecordDecl()) {
33963401
if (!decl->isParamDestroyedInCallee())
33973402
return ParameterConvention::Indirect_In_CXX;

lib/SILGen/SILGenApply.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4655,7 +4655,8 @@ static void emitConsumedLValueRecursive(SILGenFunction &SGF, SILLocation loc,
46554655

46564656
// Load if necessary.
46574657
if (value.getType().isAddress()) {
4658-
if (!param.isIndirectIn() || !SGF.silConv.useLoweredAddresses()) {
4658+
if ((!param.isIndirectIn() && !param.isIndirectInCXX()) ||
4659+
!SGF.silConv.useLoweredAddresses()) {
46594660
value = SGF.B.createFormalAccessLoadTake(loc, value);
46604661

46614662
// If our value is a moveonlywrapped type, unwrap it using owned so that

test/Interop/Cxx/reference/Inputs/module.modulemap

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ module Reference {
33
requires cplusplus
44
}
55

6+
module SpecialMembers {
7+
header "print-special-members.h"
8+
requires cplusplus
9+
}
10+
611
module Closures {
712
header "closures.h"
813
requires cplusplus
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#pragma once
2+
#include <iostream>
3+
4+
struct MoveOnly {
5+
int id;
6+
MoveOnly() : id(0) { std::cout << "MoveOnly " << id << " created\n"; }
7+
MoveOnly(const MoveOnly&) = delete;
8+
MoveOnly(MoveOnly&& other) : id(other.id + 1) { std::cout << "MoveOnly " << id << " move-created\n"; }
9+
~MoveOnly() { std::cout << "MoveOnly " << id << " destroyed\n"; }
10+
};
11+
12+
struct Copyable {
13+
int id;
14+
Copyable() : id(0) { std::cout << "Copyable " << id << " created\n"; }
15+
Copyable(const Copyable& other) : id(other.id + 1) { std::cout << "Copyable " << id << " copy-created\n"; }
16+
Copyable(Copyable&& other) : id(other.id + 1) { std::cout << "Copyable " << id << " move-created\n"; }
17+
~Copyable() { std::cout << "Copyable " << id << " destroyed\n"; }
18+
};
19+
20+
inline void byRValueRef(MoveOnly&& x) {}
21+
inline void byRValueRef(Copyable&& x) {}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop) | %FileCheck %s
2+
//
3+
// REQUIRES: executable_test
4+
5+
import SpecialMembers
6+
7+
func consume(_ x: consuming MoveOnly) {}
8+
func consume(_ x: consuming Copyable) {}
9+
10+
func main() {
11+
let x = MoveOnly()
12+
// CHECK: MoveOnly 0 created
13+
byRValueRef(consuming: x)
14+
// CHECK-NEXT: MoveOnly 1 move-created
15+
// CHECK-NEXT: MoveOnly 0 destroyed
16+
// CHECK-NEXT: MoveOnly 1 destroyed
17+
let x2 = MoveOnly()
18+
// CHECK-NEXT: MoveOnly 0 created
19+
consume(x2)
20+
// CHECK-NEXT: MoveOnly 1 move-created
21+
// CHECK-NEXT: MoveOnly 0 destroyed
22+
// CHECK-NEXT: MoveOnly 2 move-created
23+
// CHECK-NEXT: MoveOnly 1 destroyed
24+
// CHECK-NEXT: MoveOnly 2 destroyed
25+
let x3 = Copyable()
26+
// CHECK-NEXT: Copyable 0 created
27+
byRValueRef(consuming: x3)
28+
// CHECK-NEXT: Copyable 1 copy-created
29+
// CHECK-NEXT: Copyable 1 destroyed
30+
let x4 = Copyable()
31+
// CHECK-NEXT: Copyable 0 created
32+
consume(x4)
33+
// CHECK-NEXT: Copyable 1 copy-created
34+
// CHECK-NEXT: Copyable 2 move-created
35+
// CHECK-NEXT: Copyable 1 destroyed
36+
// CHECK-NEXT: Copyable 2 destroyed
37+
// CHECK-NEXT: Copyable 0 destroyed
38+
// CHECK-NEXT: Copyable 0 destroyed
39+
}
40+
41+
main()

0 commit comments

Comments
 (0)