Skip to content

Commit f93de71

Browse files
hyphyp
authored
Merge pull request #69522 from hyp/eng/5.10/consuming-parameter
5.10: [interop][SwiftToCxx] consuming parameters should be passed using an additional copy
2 parents fa3d735 + 3e89932 commit f93de71

15 files changed

+639
-43
lines changed

‎include/swift/IRGen/IRABIDetailsProvider.h‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,16 @@ class LoweredFunctionSignature {
9696

9797
inline const ParamDecl &getParamDecl() const { return paramDecl; }
9898

99+
inline ParameterConvention getConvention() const { return convention; }
100+
99101
private:
100102
DirectParameter(IRABIDetailsProviderImpl &owner,
101103
const irgen::TypeInfo &typeDetails,
102-
const ParamDecl &paramDecl);
104+
const ParamDecl &paramDecl, ParameterConvention convention);
103105
IRABIDetailsProviderImpl &owner;
104106
const irgen::TypeInfo &typeDetails;
105107
const ParamDecl &paramDecl;
108+
ParameterConvention convention;
106109
friend class LoweredFunctionSignature;
107110
};
108111

@@ -111,9 +114,13 @@ class LoweredFunctionSignature {
111114
public:
112115
inline const ParamDecl &getParamDecl() const { return paramDecl; }
113116

117+
inline ParameterConvention getConvention() const { return convention; }
118+
114119
private:
115-
IndirectParameter(const ParamDecl &paramDecl);
120+
IndirectParameter(const ParamDecl &paramDecl,
121+
ParameterConvention convention);
116122
const ParamDecl &paramDecl;
123+
ParameterConvention convention;
117124
friend class LoweredFunctionSignature;
118125
};
119126

‎lib/IRGen/IRABIDetailsProvider.cpp‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -313,12 +313,13 @@ bool LoweredFunctionSignature::DirectResultType::enumerateRecordMembers(
313313

314314
LoweredFunctionSignature::DirectParameter::DirectParameter(
315315
IRABIDetailsProviderImpl &owner, const irgen::TypeInfo &typeDetails,
316-
const ParamDecl &paramDecl)
317-
: owner(owner), typeDetails(typeDetails), paramDecl(paramDecl) {}
316+
const ParamDecl &paramDecl, ParameterConvention convention)
317+
: owner(owner), typeDetails(typeDetails), paramDecl(paramDecl),
318+
convention(convention) {}
318319

319320
LoweredFunctionSignature::IndirectParameter::IndirectParameter(
320-
const ParamDecl &paramDecl)
321-
: paramDecl(paramDecl) {}
321+
const ParamDecl &paramDecl, ParameterConvention convention)
322+
: paramDecl(paramDecl), convention(convention) {}
322323

323324
bool LoweredFunctionSignature::DirectParameter::enumerateRecordMembers(
324325
llvm::function_ref<void(clang::CharUnits, clang::CharUnits, Type)> callback)
@@ -401,10 +402,11 @@ void LoweredFunctionSignature::visitParameterList(
401402
: silParamMapping[currentSilParam];
402403
++currentSilParam;
403404
if (!isIndirect) {
404-
DirectParameter param(owner, abiParam.typeInfo, *paramDecl);
405+
DirectParameter param(owner, abiParam.typeInfo, *paramDecl,
406+
abiParam.convention);
405407
directParamVisitor(param);
406408
} else {
407-
IndirectParameter param(*paramDecl);
409+
IndirectParameter param(*paramDecl, abiParam.convention);
408410
indirectParamVisitor(param);
409411
}
410412
}
@@ -432,8 +434,11 @@ void LoweredFunctionSignature::visitParameterList(
432434

433435
if (abiDetails.hasTrailingSelfParam) {
434436
assert(!abiDetails.hasContextParam);
435-
assert(FD->hasImplicitSelfDecl());
436-
indirectParamVisitor(IndirectParameter(*FD->getImplicitSelfDecl()));
437+
indirectParamVisitor(IndirectParameter(
438+
*FD->getImplicitSelfDecl(),
439+
FD->getImplicitSelfDecl()->getValueOwnership() == ValueOwnership::Owned
440+
? ParameterConvention::Direct_Owned
441+
: ParameterConvention::Direct_Guaranteed));
437442
} else if (abiDetails.hasContextParam) {
438443
contextParamVisitor(ContextParameter());
439444
}

‎lib/PrintAsClang/PrintClangFunction.cpp‎

Lines changed: 106 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "swift/ClangImporter/ClangImporter.h"
3131
#include "swift/IRGen/IRABIDetailsProvider.h"
3232
#include "clang/AST/ASTContext.h"
33+
#include "clang/AST/Attr.h"
3334
#include "clang/AST/DeclObjC.h"
3435
#include "llvm/ADT/STLExtras.h"
3536

@@ -281,6 +282,25 @@ class CFunctionSignatureTypePrinter
281282
os << " __strong";
282283
printInoutTypeModifier();
283284
}
285+
if (isa<clang::CXXRecordDecl>(cd->getClangDecl())) {
286+
if (std::find_if(
287+
cd->getClangDecl()->getAttrs().begin(),
288+
cd->getClangDecl()->getAttrs().end(), [](clang::Attr *attr) {
289+
if (auto *sa = dyn_cast<clang::SwiftAttrAttr>(attr)) {
290+
llvm::StringRef value = sa->getAttribute();
291+
if ((value.startswith("retain:") ||
292+
value.startswith("release:")) &&
293+
!value.endswith(":immortal"))
294+
return true;
295+
}
296+
return false;
297+
}) != cd->getClangDecl()->getAttrs().end()) {
298+
// This is a shared FRT. Do not bridge it back to
299+
// C++ as its ownership is not managed automatically
300+
// in C++ yet.
301+
return ClangRepresentation::unsupported;
302+
}
303+
}
284304
// FIXME: Mark that this is only ObjC representable.
285305
return ClangRepresentation::representable;
286306
}
@@ -966,7 +986,7 @@ void DeclAndTypeClangFunctionPrinter::printTypeImplTypeSpecifier(
966986

967987
void DeclAndTypeClangFunctionPrinter::printCxxToCFunctionParameterUse(
968988
Type type, StringRef name, const ModuleDecl *moduleContext, bool isInOut,
969-
bool isIndirect, std::string directTypeEncoding, bool isSelf) {
989+
bool isIndirect, std::string directTypeEncoding, bool forceSelf) {
970990
auto namePrinter = [&]() { ClangSyntaxPrinter(os).printIdentifier(name); };
971991
if (!isKnownCxxType(type, typeMapping) &&
972992
!hasKnownOptionalNullableCxxMapping(type)) {
@@ -1007,9 +1027,9 @@ void DeclAndTypeClangFunctionPrinter::printCxxToCFunctionParameterUse(
10071027
} else {
10081028
ClangValueTypePrinter(os, cPrologueOS, interopContext)
10091029
.printParameterCxxToCUseScaffold(
1010-
moduleContext,
1011-
[&]() { printTypeImplTypeSpecifier(type, moduleContext); },
1012-
namePrinter, isSelf);
1030+
moduleContext,
1031+
[&]() { printTypeImplTypeSpecifier(type, moduleContext); },
1032+
namePrinter, forceSelf);
10131033
}
10141034
if (!directTypeEncoding.empty())
10151035
os << ')';
@@ -1153,6 +1173,82 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
11531173
break;
11541174
}
11551175
}
1176+
1177+
auto getParamName = [&](const ParamDecl &param, size_t paramIndex,
1178+
bool isConsumed) {
1179+
std::string paramName;
1180+
if (isConsumed)
1181+
paramName = "consumedParamCopy_";
1182+
if (param.isSelfParameter()) {
1183+
if (isConsumed)
1184+
paramName += "this";
1185+
else
1186+
paramName = "*this";
1187+
} else if (param.getName().empty()) {
1188+
llvm::raw_string_ostream paramOS(paramName);
1189+
if (!isConsumed)
1190+
paramOS << "_";
1191+
paramOS << paramIndex;
1192+
} else {
1193+
StringRef nameStr = param.getName().str();
1194+
if (isConsumed)
1195+
paramName += nameStr.str();
1196+
else
1197+
paramName = nameStr;
1198+
renameCxxParameterIfNeeded(FD, paramName);
1199+
}
1200+
return paramName;
1201+
};
1202+
1203+
// Check if we need to copy any parameters that are consumed by Swift,
1204+
// to ensure that Swift does not destroy the value that's owned by C++.
1205+
// FIXME: Support non-copyable types here as well between C++ -> Swift.
1206+
// FIXME: class types can be optimized down to an additional retain right
1207+
// here.
1208+
size_t paramIndex = 1;
1209+
auto emitParamCopyForConsume = [&](const ParamDecl &param) {
1210+
auto name = getParamName(param, paramIndex, /*isConsumed=*/false);
1211+
auto consumedName = getParamName(param, paramIndex, /*isConsumed=*/true);
1212+
std::string paramType;
1213+
1214+
llvm::raw_string_ostream typeOS(paramType);
1215+
1216+
CFunctionSignatureTypePrinter typePrinter(
1217+
typeOS, cPrologueOS, typeMapping, OutputLanguageMode::Cxx,
1218+
interopContext, CFunctionSignatureTypePrinterModifierDelegate(),
1219+
moduleContext, declPrinter, FunctionSignatureTypeUse::TypeReference);
1220+
auto result = typePrinter.visit(param.getInterfaceType(), OTK_None,
1221+
/*isInOutParam=*/false);
1222+
assert(!result.isUnsupported());
1223+
typeOS.flush();
1224+
1225+
os << " alignas(alignof(" << paramType << ")) char copyBuffer_"
1226+
<< consumedName << "[sizeof(" << paramType << ")];\n";
1227+
os << " auto &" << consumedName << " = *(new(copyBuffer_" << consumedName
1228+
<< ") " << paramType << "(" << name << "));\n";
1229+
os << " swift::" << cxx_synthesis::getCxxImplNamespaceName()
1230+
<< "::ConsumedValueStorageDestroyer<" << paramType << "> storageGuard_"
1231+
<< consumedName << "(" << consumedName << ");\n";
1232+
};
1233+
signature.visitParameterList(
1234+
[&](const LoweredFunctionSignature::IndirectResultValue &) {},
1235+
[&](const LoweredFunctionSignature::DirectParameter &param) {
1236+
if (isConsumedParameter(param.getConvention()))
1237+
emitParamCopyForConsume(param.getParamDecl());
1238+
++paramIndex;
1239+
},
1240+
[&](const LoweredFunctionSignature::IndirectParameter &param) {
1241+
if (isConsumedParameter(param.getConvention()))
1242+
emitParamCopyForConsume(param.getParamDecl());
1243+
++paramIndex;
1244+
},
1245+
[&](const LoweredFunctionSignature::GenericRequirementParameter
1246+
&genericRequirementParam) {},
1247+
[&](const LoweredFunctionSignature::MetadataSourceParameter
1248+
&metadataSrcParam) {},
1249+
[&](const LoweredFunctionSignature::ContextParameter &) {},
1250+
[&](const LoweredFunctionSignature::ErrorResultValue &) {});
1251+
11561252
auto printCallToCFunc = [&](llvm::Optional<StringRef> additionalParam) {
11571253
if (indirectFunctionVar)
11581254
os << "(* " << *indirectFunctionVar << ')';
@@ -1168,13 +1264,14 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
11681264
needsComma = true;
11691265
};
11701266
auto printParamUse = [&](const ParamDecl &param, bool isIndirect,
1267+
bool isConsumed,
11711268

11721269
std::string directTypeEncoding) {
11731270
emitNewParam();
1174-
std::string paramName;
11751271
if (param.isSelfParameter()) {
11761272
bool needsStaticSelf = isa<ConstructorDecl>(FD) || isStaticMethod;
11771273
if (needsStaticSelf) {
1274+
// Static self value is just the type's metadata value.
11781275
os << "swift::TypeMetadataTrait<";
11791276
CFunctionSignatureTypePrinter typePrinter(
11801277
os, cPrologueOS, typeMapping, OutputLanguageMode::Cxx,
@@ -1187,19 +1284,13 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
11871284
os << ">::getTypeMetadata()";
11881285
return;
11891286
}
1190-
paramName = "*this";
1191-
} else if (param.getName().empty()) {
1192-
llvm::raw_string_ostream paramOS(paramName);
1193-
paramOS << "_" << paramIndex;
1194-
} else {
1195-
paramName = param.getName().str().str();
1196-
renameCxxParameterIfNeeded(FD, paramName);
11971287
}
1288+
auto paramName = getParamName(param, paramIndex, isConsumed);
11981289
++paramIndex;
11991290
printCxxToCFunctionParameterUse(param.getInterfaceType(), paramName,
12001291
param.getModuleContext(), param.isInOut(),
12011292
isIndirect, directTypeEncoding,
1202-
param.isSelfParameter());
1293+
!isConsumed && param.isSelfParameter());
12031294
};
12041295

12051296
signature.visitParameterList(
@@ -1211,10 +1302,12 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
12111302
},
12121303
[&](const LoweredFunctionSignature::DirectParameter &param) {
12131304
printParamUse(param.getParamDecl(), /*isIndirect=*/false,
1305+
isConsumedParameter(param.getConvention()),
12141306
encodeTypeInfo(param, moduleContext, typeMapping));
12151307
},
12161308
[&](const LoweredFunctionSignature::IndirectParameter &param) {
12171309
printParamUse(param.getParamDecl(), /*isIndirect=*/true,
1310+
isConsumedParameter(param.getConvention()),
12181311
/*directTypeEncoding=*/"");
12191312
},
12201313
[&](const LoweredFunctionSignature::GenericRequirementParameter

‎lib/PrintAsClang/PrintClangValueType.cpp‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -502,16 +502,16 @@ void ClangValueTypePrinter::printValueTypeDecl(
502502
if (!isOpaqueLayout)
503503
printCValueTypeStorageStruct(cPrologueOS, typeDecl, *typeSizeAlign);
504504

505-
printTypeGenericTraits(os, typeDecl, typeMetadataFuncName,
506-
typeMetadataFuncGenericParams,
507-
typeDecl->getModuleContext(), declAndTypePrinter);
505+
printTypeGenericTraits(
506+
os, typeDecl, typeMetadataFuncName, typeMetadataFuncGenericParams,
507+
typeDecl->getModuleContext(), declAndTypePrinter, isOpaqueLayout);
508508
}
509509

510510
void ClangValueTypePrinter::printParameterCxxToCUseScaffold(
511511
const ModuleDecl *moduleContext, llvm::function_ref<void()> typePrinter,
512-
llvm::function_ref<void()> cxxParamPrinter, bool isSelf) {
512+
llvm::function_ref<void()> cxxParamPrinter, bool forceSelf) {
513513
// A Swift value type is passed to its underlying Swift function
514-
if (isSelf) {
514+
if (forceSelf) {
515515
os << "_getOpaquePointer()";
516516
} else {
517517
// FIXME: can we propagate the _impl request here?
@@ -598,7 +598,8 @@ void ClangValueTypePrinter::printTypePrecedingGenericTraits(
598598
void ClangValueTypePrinter::printTypeGenericTraits(
599599
raw_ostream &os, const TypeDecl *typeDecl, StringRef typeMetadataFuncName,
600600
ArrayRef<GenericRequirement> typeMetadataFuncRequirements,
601-
const ModuleDecl *moduleContext, DeclAndTypePrinter &declAndTypePrinter) {
601+
const ModuleDecl *moduleContext, DeclAndTypePrinter &declAndTypePrinter,
602+
bool isOpaqueLayout) {
602603
auto *NTD = dyn_cast<NominalTypeDecl>(typeDecl);
603604
ClangSyntaxPrinter printer(os);
604605
// FIXME: avoid popping out of the module's namespace here.
@@ -668,14 +669,16 @@ void ClangValueTypePrinter::printTypeGenericTraits(
668669
os << "::";
669670
printer.printBaseName(typeDecl);
670671
os << "> = true;\n";
671-
if (NTD && NTD->isResilient()) {
672+
}
673+
if (isOpaqueLayout) {
674+
assert(NTD && "not a nominal type?");
675+
assert(!isa<ClassDecl>(typeDecl) && !typeDecl->hasClangNode());
676+
if (printer.printNominalTypeOutsideMemberDeclTemplateSpecifiers(NTD))
672677
os << "template<>\n";
673-
os << "static inline const constexpr bool isOpaqueLayout<";
674-
printer.printBaseName(typeDecl->getModuleContext());
675-
os << "::";
676-
printer.printBaseName(typeDecl);
677-
os << "> = true;\n";
678-
}
678+
os << "static inline const constexpr bool isOpaqueLayout<";
679+
printer.printNominalTypeReference(NTD,
680+
/*moduleContext=*/nullptr);
681+
os << "> = true;\n";
679682
}
680683

681684
// FIXME: generic support.

‎lib/PrintAsClang/PrintClangValueType.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ class ClangValueTypePrinter {
9696
static void printTypeGenericTraits(
9797
raw_ostream &os, const TypeDecl *typeDecl, StringRef typeMetadataFuncName,
9898
ArrayRef<GenericRequirement> typeMetadataFuncRequirements,
99-
const ModuleDecl *moduleContext, DeclAndTypePrinter &declAndTypePrinter);
99+
const ModuleDecl *moduleContext, DeclAndTypePrinter &declAndTypePrinter,
100+
bool isOpaqueLayout = false);
100101

101102
static void printTypePrecedingGenericTraits(raw_ostream &os,
102103
const NominalTypeDecl *typeDecl,

‎lib/PrintAsClang/_SwiftCxxInteroperability.h‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@ extern "C" void _fatalError_Cxx_move_of_Swift_value_type_not_supported_yet();
110110

111111
SWIFT_INLINE_THUNK void *_Nonnull opaqueAlloc(size_t size,
112112
size_t align) noexcept {
113+
#if defined(SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_alloc) && \
114+
defined(SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_free)
115+
// Allow the user to provide custom allocator for heap-allocated Swift
116+
// value types.
117+
return SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_alloc(size, align);
118+
#else
113119
#if defined(_WIN32)
114120
void *r = _aligned_malloc(size, align);
115121
#else
@@ -120,14 +126,22 @@ SWIFT_INLINE_THUNK void *_Nonnull opaqueAlloc(size_t size,
120126
(void)res;
121127
#endif
122128
return r;
129+
#endif
123130
}
124131

125132
SWIFT_INLINE_THUNK void opaqueFree(void *_Nonnull p) noexcept {
133+
#if defined(SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_alloc) && \
134+
defined(SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_free)
135+
// Allow the user to provide custom allocator for heap-allocated Swift
136+
// value types.
137+
SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_free(p);
138+
#else
126139
#if defined(_WIN32)
127140
_aligned_free(p);
128141
#else
129142
free(p);
130143
#endif
144+
#endif
131145
}
132146

133147
/// Base class for a container for an opaque Swift value, like resilient struct.
@@ -279,6 +293,22 @@ SWIFT_INLINE_THUNK void *_Nonnull getOpaquePointer(T &value) {
279293
return reinterpret_cast<void *>(&value);
280294
}
281295

296+
/// Helper struct that destroys any additional storage allocated (e.g. for
297+
/// resilient value types) for a Swift value owned by C++ code after the Swift
298+
/// value was consumed and thus the original C++ destructor is not ran.
299+
template <class T> class ConsumedValueStorageDestroyer {
300+
public:
301+
SWIFT_INLINE_THUNK ConsumedValueStorageDestroyer(T &val) noexcept
302+
: value(val) {}
303+
SWIFT_INLINE_THUNK ~ConsumedValueStorageDestroyer() noexcept {
304+
if constexpr (isOpaqueLayout<T>)
305+
reinterpret_cast<OpaqueStorage &>(value).~OpaqueStorage();
306+
}
307+
308+
private:
309+
T &value;
310+
};
311+
282312
} // namespace _impl
283313

284314
#pragma clang diagnostic pop

0 commit comments

Comments
 (0)