From 00fa738209710eb614d19a2c821ef88c8fa650c0 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 24 Feb 2025 15:50:07 +0000 Subject: [PATCH] [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 --- lib/SIL/IR/SILFunctionType.cpp | 5 ++ lib/SILGen/SILGenApply.cpp | 3 +- .../Transforms/CopyForwarding.cpp | 2 + .../Cxx/reference/Inputs/module.modulemap | 5 ++ .../reference/Inputs/print-special-members.h | 21 ++++++ .../Cxx/reference/rvalue-reference.swift | 70 +++++++++++++++++++ 6 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 test/Interop/Cxx/reference/Inputs/print-special-members.h create mode 100644 test/Interop/Cxx/reference/rvalue-reference.swift diff --git a/lib/SIL/IR/SILFunctionType.cpp b/lib/SIL/IR/SILFunctionType.cpp index 582c21903cfa2..ac7d7f6cb2c18 100644 --- a/lib/SIL/IR/SILFunctionType.cpp +++ b/lib/SIL/IR/SILFunctionType.cpp @@ -1256,6 +1256,9 @@ class Conventions { case ValueOwnership::Shared: return ParameterConvention::Indirect_In_Guaranteed; case ValueOwnership::Owned: + if (kind == ConventionsKind::CFunction || + kind == ConventionsKind::CFunctionType) + return getIndirectParameter(index, type, substTL); return ParameterConvention::Indirect_In; } llvm_unreachable("unhandled ownership"); @@ -3392,6 +3395,8 @@ static ParameterConvention getIndirectCParameterConvention(clang::QualType type) // A trivial const * parameter in C should be considered @in. if (importer::isCxxConstReferenceType(type.getTypePtr())) return ParameterConvention::Indirect_In_Guaranteed; + if (type->isRValueReferenceType()) + return ParameterConvention::Indirect_In_CXX; if (auto *decl = type->getAsRecordDecl()) { if (!decl->isParamDestroyedInCallee()) return ParameterConvention::Indirect_In_CXX; diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 9dc44624610fa..eae867b11a62f 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -4655,7 +4655,8 @@ static void emitConsumedLValueRecursive(SILGenFunction &SGF, SILLocation loc, // Load if necessary. if (value.getType().isAddress()) { - if (!param.isIndirectIn() || !SGF.silConv.useLoweredAddresses()) { + if ((!param.isIndirectIn() && !param.isIndirectInCXX()) || + !SGF.silConv.useLoweredAddresses()) { value = SGF.B.createFormalAccessLoadTake(loc, value); // If our value is a moveonlywrapped type, unwrap it using owned so that diff --git a/lib/SILOptimizer/Transforms/CopyForwarding.cpp b/lib/SILOptimizer/Transforms/CopyForwarding.cpp index 0b279a6cb6d29..9cf693a5b7913 100644 --- a/lib/SILOptimizer/Transforms/CopyForwarding.cpp +++ b/lib/SILOptimizer/Transforms/CopyForwarding.cpp @@ -341,6 +341,7 @@ class AnalyzeForwardUse case SILArgumentConvention::Indirect_In: return true; case SILArgumentConvention::Indirect_In_Guaranteed: + case SILArgumentConvention::Indirect_In_CXX: case SILArgumentConvention::Indirect_Inout: case SILArgumentConvention::Indirect_InoutAliasable: return false; @@ -445,6 +446,7 @@ class AnalyzeBackwardUse case SILArgumentConvention::Indirect_InoutAliasable: case SILArgumentConvention::Indirect_In_Guaranteed: return false; + case SILArgumentConvention::Indirect_In_CXX: case SILArgumentConvention::Indirect_In: llvm_unreachable("copy_addr src destroyed without reinitialization"); default: diff --git a/test/Interop/Cxx/reference/Inputs/module.modulemap b/test/Interop/Cxx/reference/Inputs/module.modulemap index bf2dacdab027f..e4a36c5218f35 100644 --- a/test/Interop/Cxx/reference/Inputs/module.modulemap +++ b/test/Interop/Cxx/reference/Inputs/module.modulemap @@ -3,6 +3,11 @@ module Reference { requires cplusplus } +module SpecialMembers { + header "print-special-members.h" + requires cplusplus +} + module Closures { header "closures.h" requires cplusplus diff --git a/test/Interop/Cxx/reference/Inputs/print-special-members.h b/test/Interop/Cxx/reference/Inputs/print-special-members.h new file mode 100644 index 0000000000000..d2c810bd530b7 --- /dev/null +++ b/test/Interop/Cxx/reference/Inputs/print-special-members.h @@ -0,0 +1,21 @@ +#pragma once +#include + +struct MoveOnly { + int id; + MoveOnly() : id(0) { printf("MoveOnly %d created\n", id); } + MoveOnly(const MoveOnly&) = delete; + MoveOnly(MoveOnly&& other) : id(other.id + 1) { printf("MoveOnly %d move-created\n", id); } + ~MoveOnly() { printf("MoveOnly %d destroyed\n", id); } +}; + +struct Copyable { + int id; + Copyable() : id(0) { printf("Copyable %d created\n", id); } + Copyable(const Copyable& other) : id(other.id + 1) { printf("Copyable %d copy-created\n", id); } + Copyable(Copyable&& other) : id(other.id + 1) { printf("Copyable %d move-created\n", id); } + ~Copyable() { printf("Copyable %d destroyed\n", id); } +}; + +inline void byRValueRef(MoveOnly&& x) {} +inline void byRValueRef(Copyable&& x) {} diff --git a/test/Interop/Cxx/reference/rvalue-reference.swift b/test/Interop/Cxx/reference/rvalue-reference.swift new file mode 100644 index 0000000000000..e62c320fc8b28 --- /dev/null +++ b/test/Interop/Cxx/reference/rvalue-reference.swift @@ -0,0 +1,70 @@ +// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop -Onone) | %FileCheck -check-prefix=CHECK-DASH-ONONE %s +// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop -O) | %FileCheck -check-prefix=CHECK-DASH-O %s +// +// REQUIRES: executable_test + +import SpecialMembers + +func consume(_ x: consuming MoveOnly) {} +func consume(_ x: consuming Copyable) {} + +func moveOnly1() { + let x = MoveOnly() +// CHECK-DASH-ONONE: MoveOnly 0 created +// CHECK-DASH-O: MoveOnly 0 created + byRValueRef(consuming: x) +// CHECK-DASH-ONONE-NEXT: MoveOnly 1 move-created +// CHECK-DASH-O-NEXT: MoveOnly 1 move-created +// CHECK-DASH-ONONE-NEXT: MoveOnly 0 destroyed +// CHECK-DASH-O-NEXT: MoveOnly 0 destroyed +// CHECK-DASH-ONONE-NEXT: MoveOnly 1 destroyed +// CHECK-DASH-O-NEXT: MoveOnly 1 destroyed +} + +func moveOnly2() { + let x = MoveOnly() +// CHECK-DASH-ONONE-NEXT: MoveOnly 0 created +// CHECK-DASH-O-NEXT: MoveOnly 0 created + consume(x) +// CHECK-DASH-ONONE-NEXT: MoveOnly 1 move-created +// CHECK-DASH-ONONE-NEXT: MoveOnly 0 destroyed +// CHECK-DASH-O-NEXT: MoveOnly 0 destroyed +// CHECK-DASH-ONONE-NEXT: MoveOnly 2 move-created +// CHECK-DASH-ONONE-NEXT: MoveOnly 1 destroyed +// CHECK-DASH-ONONE-NEXT: MoveOnly 2 destroyed +} + +func copyable1() { + let x = Copyable() +// CHECK-DASH-ONONE-NEXT: Copyable 0 created +// CHECK-DASH-O-NEXT: Copyable 0 created + byRValueRef(consuming: x) +// CHECK-DASH-ONONE-NEXT: Copyable 1 copy-created +// CHECK-DASH-O-NEXT: Copyable 1 copy-created +// CHECK-DASH-ONONE-NEXT: Copyable 1 destroyed +// CHECK-DASH-O-NEXT: Copyable 1 destroyed +// CHECK-DASH-ONONE-NEXT: Copyable 0 destroyed +// CHECK-DASH-O-NEXT: Copyable 0 destroyed +} + +func copyable2() { + let x4 = Copyable() +// CHECK-DASH-ONONE-NEXT: Copyable 0 created +// CHECK-DASH-O-NEXT: Copyable 0 created + consume(x4) +// CHECK-DASH-ONONE-NEXT: Copyable 1 copy-created +// CHECK-DASH-ONONE-NEXT: Copyable 2 move-created +// CHECK-DASH-ONONE-NEXT: Copyable 1 destroyed +// CHECK-DASH-ONONE-NEXT: Copyable 2 destroyed +// CHECK-DASH-ONONE-NEXT: Copyable 0 destroyed +// CHECK-DASH-O-NEXT: Copyable 0 destroyed +} + +func main() { + moveOnly1() + moveOnly2() + copyable1() + copyable2() +} + +main()