Skip to content

[cxx-interop] enable support for move-only types #69790

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 24 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e738bdf
[cxx-interop] Import move only types even when `-enable-experimental-…
zoecarver May 2, 2023
4ba62e1
[cxx-interop] Add tests for `std::unique_ptr`.
zoecarver May 2, 2023
70a353b
[tests][cxx-interop] Fix straggling test.
zoecarver May 2, 2023
c79eace
no merge: add some debugging for linux.
zoecarver May 3, 2023
a78a7ec
[cxx-interop] Cleanup: remove debug printing, enable more tests.
zoecarver Jul 8, 2023
5d8db5c
[cxx-interop] add a testcase for setting the pointee on unique_ptr
hyp Nov 10, 2023
3cdb4b1
[cxx-interop] add some initial move only tests
hyp Nov 10, 2023
86e5556
[cxx-interop] add support for 'address' pointee for non-copyable C++ …
hyp Nov 16, 2023
5d2637a
[cxx-interop] add support for 'mutableAddress' pointee for non-copyab…
hyp Nov 27, 2023
a913c4e
[cxx-interop] fix support for value-only non-copyable type dereference
hyp Nov 27, 2023
fd6a5a2
[cxx-interop] test accessing fields of a move-only C++ type
hyp Nov 28, 2023
51d0e84
[cxx-interop] add a test to cover accessing non-copyable field
hyp Nov 28, 2023
bacc58e
[cxx-interop] provide correct referential access to non-copyable base…
hyp Nov 29, 2023
6a47011
[cxx-interop] add IRGen test for field base accessors for non-copyabl…
hyp Nov 29, 2023
16a8ae4
[cxx-interop] fix the use of '.pointee' with address accessors for de…
hyp Nov 29, 2023
0296448
[cxx-interop] fix the use of '.pointee' with getter accessor for deri…
hyp Nov 29, 2023
b7158ea
[cxx-interop] add SWIFT_NONCOPYABLE annotation
hyp Nov 30, 2023
580ecd0
[cxx-interop] fix Egor's review comments for move-only patch
hyp Nov 30, 2023
99e6d33
[cxx-interop] attempt to fix windows unique ptr test
hyp Nov 30, 2023
c029ed6
[cxx-interop] try to workaround the linux libstdc++ move-only failure
hyp Dec 1, 2023
633e7f1
fix test
hyp Dec 3, 2023
3a340c7
[cxx-interop] move-only: do not test consume with unsafeAddress acces…
hyp Dec 4, 2023
ec931c8
[cxx-interop] non-copyable test windows workaround and no SIL verify …
hyp Dec 5, 2023
623d3d2
[cxx-interop] review fixes for non-copyable patch, ensure we only ena…
hyp Dec 5, 2023
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
208 changes: 166 additions & 42 deletions lib/ClangImporter/ClangImporter.cpp

Large diffs are not rendered by default.

36 changes: 26 additions & 10 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,22 @@ void ClangImporter::Implementation::makeComputed(AbstractStorageDecl *storage,
AccessorDecl *getter,
AccessorDecl *setter) {
assert(getter);
// The synthesized computed property can either use a `get` or an
// `unsafeAddress` accessor.
auto isAddress = getter->getAccessorKind() == AccessorKind::Address;
storage->getASTContext().evaluator.cacheOutput(HasStorageRequest{storage}, false);
if (setter) {
storage->setImplInfo(StorageImplInfo::getMutableComputed());
if (isAddress)
assert(setter->getAccessorKind() == AccessorKind::MutableAddress);
storage->setImplInfo(
isAddress ? StorageImplInfo(ReadImplKind::Address,
WriteImplKind::MutableAddress,
ReadWriteImplKind::MutableAddress)
: StorageImplInfo::getMutableComputed());
storage->setAccessors(SourceLoc(), {getter, setter}, SourceLoc());
} else {
storage->setImplInfo(StorageImplInfo::getImmutableComputed());
storage->setImplInfo(isAddress ? StorageImplInfo(ReadImplKind::Address)
: StorageImplInfo::getImmutableComputed());
storage->setAccessors(SourceLoc(), {getter}, SourceLoc());
}
}
Expand Down Expand Up @@ -2183,17 +2193,23 @@ namespace {
Impl.ImportedDecls[{decl->getCanonicalDecl(), getVersion()}] = result;

if (recordHasMoveOnlySemantics(decl)) {
if (!Impl.SwiftContext.LangOpts.hasFeature(Feature::MoveOnly)) {
if (Impl.isCxxInteropCompatVersionAtLeast(
version::getUpcomingCxxInteropCompatVersion())) {
if (decl->isInStdNamespace() && decl->getName() == "promise") {
// Do not import std::promise.
return nullptr;
}
result->getAttrs().add(new (Impl.SwiftContext)
MoveOnlyAttr(/*Implicit=*/true));
} else {
Impl.addImportDiagnostic(
decl, Diagnostic(
diag::move_only_requires_move_only,
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())),
decl,
Diagnostic(
diag::move_only_requires_move_only,
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())),
decl->getLocation());
return nullptr;
}

result->getAttrs().add(new (Impl.SwiftContext)
MoveOnlyAttr(/*Implicit=*/true));
}

// FIXME: Figure out what to do with superclasses in C++. One possible
Expand Down Expand Up @@ -3931,7 +3947,7 @@ namespace {
auto loc = Impl.importSourceLoc(decl->getLocation());
auto dc = Impl.importDeclContextOf(
decl, importedName.getEffectiveContext());

SmallVector<GenericTypeParamDecl *, 4> genericParams;
for (auto &param : *decl->getTemplateParameters()) {
auto genericParamDecl =
Expand Down
111 changes: 96 additions & 15 deletions lib/ClangImporter/SwiftDeclSynthesizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1496,12 +1496,36 @@ AccessorDecl *SwiftDeclSynthesizer::buildSubscriptSetterDecl(

// MARK: C++ subscripts

Expr *SwiftDeclSynthesizer::synthesizeReturnReinterpretCast(ASTContext &ctx,
Type givenType,
Type exprType,
Expr *baseExpr) {
auto reinterpretCast = cast<FuncDecl>(
getBuiltinValueDecl(ctx, ctx.getIdentifier("reinterpretCast")));
SubstitutionMap subMap = SubstitutionMap::get(
reinterpretCast->getGenericSignature(), {givenType, exprType}, {});
ConcreteDeclRef concreteDeclRef(reinterpretCast, subMap);
auto reinterpretCastRef =
new (ctx) DeclRefExpr(concreteDeclRef, DeclNameLoc(), /*implicit*/ true);
FunctionType::ExtInfo info;
reinterpretCastRef->setType(
FunctionType::get({FunctionType::Param(givenType)}, exprType, info));

auto *argList = ArgumentList::forImplicitUnlabeled(ctx, {baseExpr});
auto reinterpreted =
CallExpr::createImplicit(ctx, reinterpretCastRef, argList);
reinterpreted->setType(exprType);
reinterpreted->setThrows(nullptr);
return reinterpreted;
}

/// Synthesizer callback for a subscript getter or a getter for a
/// dereference property (`var pointee`). If the getter's implementation returns
/// an UnsafePointer or UnsafeMutablePointer, it unwraps the pointer and returns
/// the underlying value.
static std::pair<BraceStmt *, bool>
synthesizeUnwrappingGetterBody(AbstractFunctionDecl *afd, void *context) {
synthesizeUnwrappingGetterOrAddressGetterBody(AbstractFunctionDecl *afd,
void *context, bool isAddress) {
auto getterDecl = cast<AccessorDecl>(afd);
auto getterImpl = static_cast<FuncDecl *>(context);

Expand All @@ -1525,7 +1549,8 @@ synthesizeUnwrappingGetterBody(AbstractFunctionDecl *afd, void *context) {
// reference type. This check actually checks to see if the type is a pointer
// type, but this does not apply to C pointers because they are Optional types
// when imported. TODO: Use a more obvious check here.
if (getterImpl->getResultInterfaceType()->getAnyPointerElementType(ptrKind)) {
if (!isAddress &&
getterImpl->getResultInterfaceType()->getAnyPointerElementType(ptrKind)) {
// `getterImpl` can return either UnsafePointer or UnsafeMutablePointer.
// Retrieve the corresponding `.pointee` declaration.
VarDecl *pointeePropertyDecl = ctx.getPointerPointeePropertyDecl(ptrKind);
Expand All @@ -1540,6 +1565,11 @@ synthesizeUnwrappingGetterBody(AbstractFunctionDecl *afd, void *context) {
pointeePropertyRefExpr->setType(elementTy);
propertyExpr = pointeePropertyRefExpr;
}
// Cast an 'address' result from a mutable pointer if needed.
if (isAddress &&
getterImpl->getResultInterfaceType()->isUnsafeMutablePointer())
propertyExpr = SwiftDeclSynthesizer::synthesizeReturnReinterpretCast(
ctx, getterImpl->getResultInterfaceType(), elementTy, propertyExpr);

auto returnStmt = new (ctx) ReturnStmt(SourceLoc(), propertyExpr,
/*implicit*/ true);
Expand All @@ -1549,6 +1579,19 @@ synthesizeUnwrappingGetterBody(AbstractFunctionDecl *afd, void *context) {
return {body, /*isTypeChecked*/ true};
}

static std::pair<BraceStmt *, bool>
synthesizeUnwrappingGetterBody(AbstractFunctionDecl *afd, void *context) {
return synthesizeUnwrappingGetterOrAddressGetterBody(afd, context,
/*isAddress=*/false);
}

static std::pair<BraceStmt *, bool>
synthesizeUnwrappingAddressGetterBody(AbstractFunctionDecl *afd,
void *context) {
return synthesizeUnwrappingGetterOrAddressGetterBody(afd, context,
/*isAddress=*/true);
}

/// Synthesizer callback for a subscript setter or a setter for a dereference
/// property (`var pointee`).
static std::pair<BraceStmt *, bool>
Expand Down Expand Up @@ -1596,6 +1639,26 @@ synthesizeUnwrappingSetterBody(AbstractFunctionDecl *afd, void *context) {
return {body, /*isTypeChecked*/ true};
}

static std::pair<BraceStmt *, bool>
synthesizeUnwrappingAddressSetterBody(AbstractFunctionDecl *afd,
void *context) {
auto setterDecl = cast<AccessorDecl>(afd);
auto setterImpl = static_cast<FuncDecl *>(context);

ASTContext &ctx = setterDecl->getASTContext();

auto selfArg = createSelfArg(setterDecl);
auto *setterImplCallExpr =
createAccessorImplCallExpr(setterImpl, selfArg, nullptr);

auto returnStmt = new (ctx) ReturnStmt(SourceLoc(), setterImplCallExpr,
/*implicit*/ true);

auto body = BraceStmt::create(ctx, SourceLoc(), {returnStmt}, SourceLoc(),
/*implicit*/ true);
return {body, /*isTypeChecked*/ true};
}

SubscriptDecl *SwiftDeclSynthesizer::makeSubscript(FuncDecl *getter,
FuncDecl *setter) {
assert((getter || setter) &&
Expand Down Expand Up @@ -1685,7 +1748,6 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter,
FuncDecl *setter) {
assert((getter || setter) &&
"getter or setter required to generate a pointee property");

auto &ctx = ImporterImpl.SwiftContext;
FuncDecl *getterImpl = getter ? getter : setter;
FuncDecl *setterImpl = setter;
Expand All @@ -1697,6 +1759,11 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter,
const auto elementTy = rawElementTy->getAnyPointerElementType()
? rawElementTy->getAnyPointerElementType()
: rawElementTy;
// Use 'address' or 'mutableAddress' accessors for non-copyable
// types that are returned indirectly.
bool isImplicit = !elementTy->isNoncopyable(dc);
bool useAddress =
rawElementTy->getAnyPointerElementType() && elementTy->isNoncopyable(dc);

auto result = new (ctx)
VarDecl(/*isStatic*/ false, VarDecl::Introducer::Var,
Expand All @@ -1705,16 +1772,21 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter,
result->setAccess(AccessLevel::Public);

AccessorDecl *getterDecl = AccessorDecl::create(
ctx, getterImpl->getLoc(), getterImpl->getLoc(), AccessorKind::Get,
result, SourceLoc(), StaticSpellingKind::None,
ctx, getterImpl->getLoc(), getterImpl->getLoc(),
useAddress ? AccessorKind::Address : AccessorKind::Get, result,
SourceLoc(), StaticSpellingKind::None,
/*async*/ false, SourceLoc(),
/*throws*/ false, SourceLoc(), /*ThrownType=*/TypeLoc(),
ParameterList::createEmpty(ctx), elementTy, dc);
ParameterList::createEmpty(ctx),
useAddress ? elementTy->wrapInPointer(PTK_UnsafePointer) : elementTy, dc);
getterDecl->setAccess(AccessLevel::Public);
getterDecl->setImplicit();
if (isImplicit)
getterDecl->setImplicit();
getterDecl->setIsDynamic(false);
getterDecl->setIsTransparent(true);
getterDecl->setBodySynthesizer(synthesizeUnwrappingGetterBody,
getterDecl->setBodySynthesizer(useAddress
? synthesizeUnwrappingAddressGetterBody
: synthesizeUnwrappingGetterBody,
getterImpl);

if (getterImpl->isMutating()) {
Expand All @@ -1733,20 +1805,29 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter,
paramVarDecl->setSpecifier(ParamSpecifier::Default);
paramVarDecl->setInterfaceType(elementTy);

auto setterParamList =
ParameterList::create(ctx, {paramVarDecl});
auto setterParamList = useAddress
? ParameterList::create(ctx, {})
: ParameterList::create(ctx, {paramVarDecl});

setterDecl = AccessorDecl::create(
ctx, setterImpl->getLoc(), setterImpl->getLoc(), AccessorKind::Set,
result, SourceLoc(), StaticSpellingKind::None,
ctx, setterImpl->getLoc(), setterImpl->getLoc(),
useAddress ? AccessorKind::MutableAddress : AccessorKind::Set, result,
SourceLoc(), StaticSpellingKind::None,
/*async*/ false, SourceLoc(),
/*throws*/ false, SourceLoc(), /*ThrownType=*/TypeLoc(),
setterParamList, TupleType::getEmpty(ctx), dc);
setterParamList,
useAddress ? elementTy->wrapInPointer(PTK_UnsafeMutablePointer)
: TupleType::getEmpty(ctx),
dc);
setterDecl->setAccess(AccessLevel::Public);
setterDecl->setImplicit();
if (isImplicit)
setterDecl->setImplicit();
setterDecl->setIsDynamic(false);
setterDecl->setIsTransparent(true);
setterDecl->setBodySynthesizer(synthesizeUnwrappingSetterBody, setterImpl);
setterDecl->setBodySynthesizer(useAddress
? synthesizeUnwrappingAddressSetterBody
: synthesizeUnwrappingSetterBody,
setterImpl);

if (setterImpl->isMutating()) {
setterDecl->setSelfAccessKind(SelfAccessKind::Mutating);
Expand Down
4 changes: 4 additions & 0 deletions lib/ClangImporter/SwiftDeclSynthesizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ class SwiftDeclSynthesizer {
VarDecl::Introducer introducer, bool isImplicit,
AccessLevel access, AccessLevel setterAccess);

/// Create a reinterpretCast from the `exprType`, to the `givenType`.
static Expr *synthesizeReturnReinterpretCast(ASTContext &ctx, Type givenType,
Type exprType, Expr *baseExpr);

/// Create a new named constant with the given value.
///
/// \param name The name of the constant.
Expand Down
6 changes: 6 additions & 0 deletions lib/ClangImporter/bridging
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@
#define SWIFT_UNCHECKED_SENDABLE \
__attribute__((swift_attr("@Sendable")))

/// Specifies that a specific c++ type such class or struct should be imported
/// as a non-copyable Swift value type.
#define SWIFT_NONCOPYABLE \
__attribute__((swift_attr("~Copyable")))

#else // #if _CXX_INTEROP_HAS_ATTRIBUTE(swift_attr)

// Empty defines for compilers that don't support `attribute(swift_attr)`.
Expand All @@ -166,6 +171,7 @@
#define SWIFT_COMPUTED_PROPERTY
#define SWIFT_MUTATING
#define SWIFT_UNCHECKED_SENDABLE
#define SWIFT_NONCOPYABLE

#endif // #if _CXX_INTEROP_HAS_ATTRIBUTE(swift_attr)

Expand Down
11 changes: 11 additions & 0 deletions test/Interop/Cxx/class/Inputs/type-classification.h
Original file line number Diff line number Diff line change
Expand Up @@ -251,4 +251,15 @@ struct HasMethodThatReturnsIteratorBox {
IteratorBox getIteratorBox() const;
};

struct __attribute__((swift_attr("~Copyable"))) StructCopyableMovableAnnotatedNonCopyable {
inline StructCopyableMovableAnnotatedNonCopyable() {}
StructCopyableMovableAnnotatedNonCopyable(const StructCopyableMovableAnnotatedNonCopyable &) = default;
StructCopyableMovableAnnotatedNonCopyable(StructCopyableMovableAnnotatedNonCopyable &&) = default;
StructCopyableMovableAnnotatedNonCopyable &
operator=(const StructCopyableMovableAnnotatedNonCopyable &) = default;
StructCopyableMovableAnnotatedNonCopyable &
operator=(StructCopyableMovableAnnotatedNonCopyable &&) = default;
~StructCopyableMovableAnnotatedNonCopyable() = default;
};

#endif // TEST_INTEROP_CXX_CLASS_INPUTS_TYPE_CLASSIFICATION_H
4 changes: 4 additions & 0 deletions test/Interop/Cxx/class/move-only/Inputs/module.modulemap
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module MoveOnlyCxxValueType {
header "move-only-cxx-value-type.h"
requires cplusplus
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#ifndef TEST_INTEROP_CXX_CLASS_MOVE_ONLY_VT_H
#define TEST_INTEROP_CXX_CLASS_MOVE_ONLY_VT_H

#include <memory>

struct Copyable {
int x;
};

struct NonCopyable {
inline NonCopyable(int x) : x(x) {}
inline NonCopyable(const NonCopyable &) = delete;
inline NonCopyable(NonCopyable &&other) : x(other.x) { other.x = -123; }

inline int method(int y) const { return x * y; }
inline int mutMethod(int y) {
x = y;
return y;
}

int x;
};

struct NonCopyableDerived: public NonCopyable {
NonCopyableDerived(int x) : NonCopyable(x) {}
};

struct NonCopyableDerivedDerived: public NonCopyableDerived {
NonCopyableDerivedDerived(int x) : NonCopyableDerived(x) {}
};

struct NonCopyableHolder {
inline NonCopyableHolder(int x) : x(x) {}
inline NonCopyableHolder(const NonCopyableHolder &) = delete;
inline NonCopyableHolder(NonCopyableHolder &&other) : x(std::move(other.x)) {}

inline NonCopyable &returnMutNonCopyableRef() { return x; }

inline const NonCopyable &returnNonCopyableRef() const { return x; }

NonCopyable x;
};

struct NonCopyableHolderDerived: NonCopyableHolder {
inline NonCopyableHolderDerived(int x) : NonCopyableHolder(x) {}
};

struct NonCopyableHolderDerivedDerived: NonCopyableHolderDerived {
inline NonCopyableHolderDerivedDerived(int x) : NonCopyableHolderDerived(x) {}

inline int getActualX() const {
return x.x;
}
};

#endif // TEST_INTEROP_CXX_CLASS_MOVE_ONLY_VT_H
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// RUN: %target-swift-emit-irgen -I %S/Inputs -cxx-interoperability-mode=upcoming-swift %s -validate-tbd-against-ir=none -Xcc -fignore-exceptions | %FileCheck %s

import MoveOnlyCxxValueType

func testGetX() -> CInt {
let derived = NonCopyableHolderDerivedDerived(42)
return derived.x.x
}

let _ = testGetX()

func testSetX(_ x: CInt) {
var derived = NonCopyableHolderDerivedDerived(42)
derived.x.x = 2
}

testSetX(2)

// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}__synthesizedBaseCall___synthesizedBaseGetterAccessor{{.*}}

// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}__synthesizedBaseCall___synthesizedBaseSetterAccessor{{.*}}

// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}__synthesizedBaseGetterAccessor{{.*}}
// CHECK: %[[VPTR:.*]] = getelementptr inbounds %struct.NonCopyableHolder
// CHECK: ret ptr %[[VPTR]]

// CHECK: define {{.*}}linkonce_odr{{.*}} ptr @{{.*}}__synthesizedBaseSetterAccessor{{.*}}
// CHECK: %[[VPTRS:.*]] = getelementptr inbounds %struct.NonCopyableHolder
// CHECK: ret ptr %[[VPTRS]]
Loading