Skip to content

5.10: [interop][SwiftToCxx] consuming parameters should be passed using an additional copy #69522

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 5 commits into from
Oct 31, 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
11 changes: 9 additions & 2 deletions include/swift/IRGen/IRABIDetailsProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,16 @@ class LoweredFunctionSignature {

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

inline ParameterConvention getConvention() const { return convention; }

private:
DirectParameter(IRABIDetailsProviderImpl &owner,
const irgen::TypeInfo &typeDetails,
const ParamDecl &paramDecl);
const ParamDecl &paramDecl, ParameterConvention convention);
IRABIDetailsProviderImpl &owner;
const irgen::TypeInfo &typeDetails;
const ParamDecl &paramDecl;
ParameterConvention convention;
friend class LoweredFunctionSignature;
};

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

inline ParameterConvention getConvention() const { return convention; }

private:
IndirectParameter(const ParamDecl &paramDecl);
IndirectParameter(const ParamDecl &paramDecl,
ParameterConvention convention);
const ParamDecl &paramDecl;
ParameterConvention convention;
friend class LoweredFunctionSignature;
};

Expand Down
21 changes: 13 additions & 8 deletions lib/IRGen/IRABIDetailsProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,13 @@ bool LoweredFunctionSignature::DirectResultType::enumerateRecordMembers(

LoweredFunctionSignature::DirectParameter::DirectParameter(
IRABIDetailsProviderImpl &owner, const irgen::TypeInfo &typeDetails,
const ParamDecl &paramDecl)
: owner(owner), typeDetails(typeDetails), paramDecl(paramDecl) {}
const ParamDecl &paramDecl, ParameterConvention convention)
: owner(owner), typeDetails(typeDetails), paramDecl(paramDecl),
convention(convention) {}

LoweredFunctionSignature::IndirectParameter::IndirectParameter(
const ParamDecl &paramDecl)
: paramDecl(paramDecl) {}
const ParamDecl &paramDecl, ParameterConvention convention)
: paramDecl(paramDecl), convention(convention) {}

bool LoweredFunctionSignature::DirectParameter::enumerateRecordMembers(
llvm::function_ref<void(clang::CharUnits, clang::CharUnits, Type)> callback)
Expand Down Expand Up @@ -401,10 +402,11 @@ void LoweredFunctionSignature::visitParameterList(
: silParamMapping[currentSilParam];
++currentSilParam;
if (!isIndirect) {
DirectParameter param(owner, abiParam.typeInfo, *paramDecl);
DirectParameter param(owner, abiParam.typeInfo, *paramDecl,
abiParam.convention);
directParamVisitor(param);
} else {
IndirectParameter param(*paramDecl);
IndirectParameter param(*paramDecl, abiParam.convention);
indirectParamVisitor(param);
}
}
Expand Down Expand Up @@ -432,8 +434,11 @@ void LoweredFunctionSignature::visitParameterList(

if (abiDetails.hasTrailingSelfParam) {
assert(!abiDetails.hasContextParam);
assert(FD->hasImplicitSelfDecl());
indirectParamVisitor(IndirectParameter(*FD->getImplicitSelfDecl()));
indirectParamVisitor(IndirectParameter(
*FD->getImplicitSelfDecl(),
FD->getImplicitSelfDecl()->getValueOwnership() == ValueOwnership::Owned
? ParameterConvention::Direct_Owned
: ParameterConvention::Direct_Guaranteed));
} else if (abiDetails.hasContextParam) {
contextParamVisitor(ContextParameter());
}
Expand Down
119 changes: 106 additions & 13 deletions lib/PrintAsClang/PrintClangFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "swift/ClangImporter/ClangImporter.h"
#include "swift/IRGen/IRABIDetailsProvider.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
#include "clang/AST/DeclObjC.h"
#include "llvm/ADT/STLExtras.h"

Expand Down Expand Up @@ -281,6 +282,25 @@ class CFunctionSignatureTypePrinter
os << " __strong";
printInoutTypeModifier();
}
if (isa<clang::CXXRecordDecl>(cd->getClangDecl())) {
if (std::find_if(
cd->getClangDecl()->getAttrs().begin(),
cd->getClangDecl()->getAttrs().end(), [](clang::Attr *attr) {
if (auto *sa = dyn_cast<clang::SwiftAttrAttr>(attr)) {
llvm::StringRef value = sa->getAttribute();
if ((value.startswith("retain:") ||
value.startswith("release:")) &&
!value.endswith(":immortal"))
return true;
}
return false;
}) != cd->getClangDecl()->getAttrs().end()) {
// This is a shared FRT. Do not bridge it back to
// C++ as its ownership is not managed automatically
// in C++ yet.
return ClangRepresentation::unsupported;
}
}
// FIXME: Mark that this is only ObjC representable.
return ClangRepresentation::representable;
}
Expand Down Expand Up @@ -966,7 +986,7 @@ void DeclAndTypeClangFunctionPrinter::printTypeImplTypeSpecifier(

void DeclAndTypeClangFunctionPrinter::printCxxToCFunctionParameterUse(
Type type, StringRef name, const ModuleDecl *moduleContext, bool isInOut,
bool isIndirect, std::string directTypeEncoding, bool isSelf) {
bool isIndirect, std::string directTypeEncoding, bool forceSelf) {
auto namePrinter = [&]() { ClangSyntaxPrinter(os).printIdentifier(name); };
if (!isKnownCxxType(type, typeMapping) &&
!hasKnownOptionalNullableCxxMapping(type)) {
Expand Down Expand Up @@ -1007,9 +1027,9 @@ void DeclAndTypeClangFunctionPrinter::printCxxToCFunctionParameterUse(
} else {
ClangValueTypePrinter(os, cPrologueOS, interopContext)
.printParameterCxxToCUseScaffold(
moduleContext,
[&]() { printTypeImplTypeSpecifier(type, moduleContext); },
namePrinter, isSelf);
moduleContext,
[&]() { printTypeImplTypeSpecifier(type, moduleContext); },
namePrinter, forceSelf);
}
if (!directTypeEncoding.empty())
os << ')';
Expand Down Expand Up @@ -1153,6 +1173,82 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
break;
}
}

auto getParamName = [&](const ParamDecl &param, size_t paramIndex,
bool isConsumed) {
std::string paramName;
if (isConsumed)
paramName = "consumedParamCopy_";
if (param.isSelfParameter()) {
if (isConsumed)
paramName += "this";
else
paramName = "*this";
} else if (param.getName().empty()) {
llvm::raw_string_ostream paramOS(paramName);
if (!isConsumed)
paramOS << "_";
paramOS << paramIndex;
} else {
StringRef nameStr = param.getName().str();
if (isConsumed)
paramName += nameStr.str();
else
paramName = nameStr;
renameCxxParameterIfNeeded(FD, paramName);
}
return paramName;
};

// Check if we need to copy any parameters that are consumed by Swift,
// to ensure that Swift does not destroy the value that's owned by C++.
// FIXME: Support non-copyable types here as well between C++ -> Swift.
// FIXME: class types can be optimized down to an additional retain right
// here.
size_t paramIndex = 1;
auto emitParamCopyForConsume = [&](const ParamDecl &param) {
auto name = getParamName(param, paramIndex, /*isConsumed=*/false);
auto consumedName = getParamName(param, paramIndex, /*isConsumed=*/true);
std::string paramType;

llvm::raw_string_ostream typeOS(paramType);

CFunctionSignatureTypePrinter typePrinter(
typeOS, cPrologueOS, typeMapping, OutputLanguageMode::Cxx,
interopContext, CFunctionSignatureTypePrinterModifierDelegate(),
moduleContext, declPrinter, FunctionSignatureTypeUse::TypeReference);
auto result = typePrinter.visit(param.getInterfaceType(), OTK_None,
/*isInOutParam=*/false);
assert(!result.isUnsupported());
typeOS.flush();

os << " alignas(alignof(" << paramType << ")) char copyBuffer_"
<< consumedName << "[sizeof(" << paramType << ")];\n";
os << " auto &" << consumedName << " = *(new(copyBuffer_" << consumedName
<< ") " << paramType << "(" << name << "));\n";
os << " swift::" << cxx_synthesis::getCxxImplNamespaceName()
<< "::ConsumedValueStorageDestroyer<" << paramType << "> storageGuard_"
<< consumedName << "(" << consumedName << ");\n";
};
signature.visitParameterList(
[&](const LoweredFunctionSignature::IndirectResultValue &) {},
[&](const LoweredFunctionSignature::DirectParameter &param) {
if (isConsumedParameter(param.getConvention()))
emitParamCopyForConsume(param.getParamDecl());
++paramIndex;
},
[&](const LoweredFunctionSignature::IndirectParameter &param) {
if (isConsumedParameter(param.getConvention()))
emitParamCopyForConsume(param.getParamDecl());
++paramIndex;
},
[&](const LoweredFunctionSignature::GenericRequirementParameter
&genericRequirementParam) {},
[&](const LoweredFunctionSignature::MetadataSourceParameter
&metadataSrcParam) {},
[&](const LoweredFunctionSignature::ContextParameter &) {},
[&](const LoweredFunctionSignature::ErrorResultValue &) {});

auto printCallToCFunc = [&](llvm::Optional<StringRef> additionalParam) {
if (indirectFunctionVar)
os << "(* " << *indirectFunctionVar << ')';
Expand All @@ -1168,13 +1264,14 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
needsComma = true;
};
auto printParamUse = [&](const ParamDecl &param, bool isIndirect,
bool isConsumed,

std::string directTypeEncoding) {
emitNewParam();
std::string paramName;
if (param.isSelfParameter()) {
bool needsStaticSelf = isa<ConstructorDecl>(FD) || isStaticMethod;
if (needsStaticSelf) {
// Static self value is just the type's metadata value.
os << "swift::TypeMetadataTrait<";
CFunctionSignatureTypePrinter typePrinter(
os, cPrologueOS, typeMapping, OutputLanguageMode::Cxx,
Expand All @@ -1187,19 +1284,13 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
os << ">::getTypeMetadata()";
return;
}
paramName = "*this";
} else if (param.getName().empty()) {
llvm::raw_string_ostream paramOS(paramName);
paramOS << "_" << paramIndex;
} else {
paramName = param.getName().str().str();
renameCxxParameterIfNeeded(FD, paramName);
}
auto paramName = getParamName(param, paramIndex, isConsumed);
++paramIndex;
printCxxToCFunctionParameterUse(param.getInterfaceType(), paramName,
param.getModuleContext(), param.isInOut(),
isIndirect, directTypeEncoding,
param.isSelfParameter());
!isConsumed && param.isSelfParameter());
};

signature.visitParameterList(
Expand All @@ -1211,10 +1302,12 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody(
},
[&](const LoweredFunctionSignature::DirectParameter &param) {
printParamUse(param.getParamDecl(), /*isIndirect=*/false,
isConsumedParameter(param.getConvention()),
encodeTypeInfo(param, moduleContext, typeMapping));
},
[&](const LoweredFunctionSignature::IndirectParameter &param) {
printParamUse(param.getParamDecl(), /*isIndirect=*/true,
isConsumedParameter(param.getConvention()),
/*directTypeEncoding=*/"");
},
[&](const LoweredFunctionSignature::GenericRequirementParameter
Expand Down
29 changes: 16 additions & 13 deletions lib/PrintAsClang/PrintClangValueType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -502,16 +502,16 @@ void ClangValueTypePrinter::printValueTypeDecl(
if (!isOpaqueLayout)
printCValueTypeStorageStruct(cPrologueOS, typeDecl, *typeSizeAlign);

printTypeGenericTraits(os, typeDecl, typeMetadataFuncName,
typeMetadataFuncGenericParams,
typeDecl->getModuleContext(), declAndTypePrinter);
printTypeGenericTraits(
os, typeDecl, typeMetadataFuncName, typeMetadataFuncGenericParams,
typeDecl->getModuleContext(), declAndTypePrinter, isOpaqueLayout);
}

void ClangValueTypePrinter::printParameterCxxToCUseScaffold(
const ModuleDecl *moduleContext, llvm::function_ref<void()> typePrinter,
llvm::function_ref<void()> cxxParamPrinter, bool isSelf) {
llvm::function_ref<void()> cxxParamPrinter, bool forceSelf) {
// A Swift value type is passed to its underlying Swift function
if (isSelf) {
if (forceSelf) {
os << "_getOpaquePointer()";
} else {
// FIXME: can we propagate the _impl request here?
Expand Down Expand Up @@ -598,7 +598,8 @@ void ClangValueTypePrinter::printTypePrecedingGenericTraits(
void ClangValueTypePrinter::printTypeGenericTraits(
raw_ostream &os, const TypeDecl *typeDecl, StringRef typeMetadataFuncName,
ArrayRef<GenericRequirement> typeMetadataFuncRequirements,
const ModuleDecl *moduleContext, DeclAndTypePrinter &declAndTypePrinter) {
const ModuleDecl *moduleContext, DeclAndTypePrinter &declAndTypePrinter,
bool isOpaqueLayout) {
auto *NTD = dyn_cast<NominalTypeDecl>(typeDecl);
ClangSyntaxPrinter printer(os);
// FIXME: avoid popping out of the module's namespace here.
Expand Down Expand Up @@ -668,14 +669,16 @@ void ClangValueTypePrinter::printTypeGenericTraits(
os << "::";
printer.printBaseName(typeDecl);
os << "> = true;\n";
if (NTD && NTD->isResilient()) {
}
if (isOpaqueLayout) {
assert(NTD && "not a nominal type?");
assert(!isa<ClassDecl>(typeDecl) && !typeDecl->hasClangNode());
if (printer.printNominalTypeOutsideMemberDeclTemplateSpecifiers(NTD))
os << "template<>\n";
os << "static inline const constexpr bool isOpaqueLayout<";
printer.printBaseName(typeDecl->getModuleContext());
os << "::";
printer.printBaseName(typeDecl);
os << "> = true;\n";
}
os << "static inline const constexpr bool isOpaqueLayout<";
printer.printNominalTypeReference(NTD,
/*moduleContext=*/nullptr);
os << "> = true;\n";
}

// FIXME: generic support.
Expand Down
3 changes: 2 additions & 1 deletion lib/PrintAsClang/PrintClangValueType.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ class ClangValueTypePrinter {
static void printTypeGenericTraits(
raw_ostream &os, const TypeDecl *typeDecl, StringRef typeMetadataFuncName,
ArrayRef<GenericRequirement> typeMetadataFuncRequirements,
const ModuleDecl *moduleContext, DeclAndTypePrinter &declAndTypePrinter);
const ModuleDecl *moduleContext, DeclAndTypePrinter &declAndTypePrinter,
bool isOpaqueLayout = false);

static void printTypePrecedingGenericTraits(raw_ostream &os,
const NominalTypeDecl *typeDecl,
Expand Down
30 changes: 30 additions & 0 deletions lib/PrintAsClang/_SwiftCxxInteroperability.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ extern "C" void _fatalError_Cxx_move_of_Swift_value_type_not_supported_yet();

SWIFT_INLINE_THUNK void *_Nonnull opaqueAlloc(size_t size,
size_t align) noexcept {
#if defined(SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_alloc) && \
defined(SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_free)
// Allow the user to provide custom allocator for heap-allocated Swift
// value types.
return SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_alloc(size, align);
#else
#if defined(_WIN32)
void *r = _aligned_malloc(size, align);
#else
Expand All @@ -120,14 +126,22 @@ SWIFT_INLINE_THUNK void *_Nonnull opaqueAlloc(size_t size,
(void)res;
#endif
return r;
#endif
}

SWIFT_INLINE_THUNK void opaqueFree(void *_Nonnull p) noexcept {
#if defined(SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_alloc) && \
defined(SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_free)
// Allow the user to provide custom allocator for heap-allocated Swift
// value types.
SWIFT_CXX_INTEROPERABILITY_OVERRIDE_OPAQUE_STORAGE_free(p);
#else
#if defined(_WIN32)
_aligned_free(p);
#else
free(p);
#endif
#endif
}

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

/// Helper struct that destroys any additional storage allocated (e.g. for
/// resilient value types) for a Swift value owned by C++ code after the Swift
/// value was consumed and thus the original C++ destructor is not ran.
template <class T> class ConsumedValueStorageDestroyer {
public:
SWIFT_INLINE_THUNK ConsumedValueStorageDestroyer(T &val) noexcept
: value(val) {}
SWIFT_INLINE_THUNK ~ConsumedValueStorageDestroyer() noexcept {
if constexpr (isOpaqueLayout<T>)
reinterpret_cast<OpaqueStorage &>(value).~OpaqueStorage();
}

private:
T &value;
};

} // namespace _impl

#pragma clang diagnostic pop
Expand Down
Loading