From 6a9942a7c83b3547bd7039e7c59b4df3e276b006 Mon Sep 17 00:00:00 2001 From: susmonteiro Date: Thu, 18 Dec 2025 16:14:14 +0000 Subject: [PATCH] [cxx-interop] Add support for subscripts of non-copyable types --- lib/ClangImporter/ClangImporter.cpp | 21 ++---- lib/ClangImporter/SwiftDeclSynthesizer.cpp | 53 +++++++++---- lib/IRGen/GenStruct.cpp | 2 +- .../Interop/Cxx/class/noncopyable-irgen.swift | 24 +++++- .../Cxx/class/noncopyable-typechecker.swift | 14 ++++ test/Interop/Cxx/class/noncopyable.swift | 74 +++++++++++++++++++ test/Interop/Cxx/stdlib/Inputs/std-vector.h | 10 +++ test/Interop/Cxx/stdlib/use-std-vector.swift | 20 +++++ 8 files changed, 187 insertions(+), 31 deletions(-) create mode 100644 test/Interop/Cxx/class/noncopyable.swift diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 716d0c9f870fe..1fd4371828f7f 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -6257,9 +6257,9 @@ makeBaseClassMemberAccessors(DeclContext *declContext, // Use 'address' or 'mutableAddress' accessors for non-copyable // types, unless the base accessor returns it by value. - bool useAddress = contextTy->isNoncopyable() && - (baseClassVar->getReadImpl() == ReadImplKind::Stored || - baseClassVar->getAccessor(AccessorKind::Address)); + bool useAddress = baseClassVar->getAccessor(AccessorKind::Address) || + (contextTy->isNoncopyable() && + baseClassVar->getReadImpl() == ReadImplKind::Stored); ParameterList *bodyParams = nullptr; if (auto subscript = dyn_cast(baseClassVar)) { @@ -6407,12 +6407,6 @@ static ValueDecl *cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext, } if (auto subscript = dyn_cast(decl)) { - auto contextTy = - newContext->mapTypeIntoEnvironment(subscript->getElementInterfaceType()); - // Subscripts that return non-copyable types are not yet supported. - // See: https://github.com/apple/swift/issues/70047. - if (contextTy->isNoncopyable()) - return nullptr; auto out = SubscriptDecl::create( subscript->getASTContext(), subscript->getName(), subscript->getStaticLoc(), subscript->getStaticSpelling(), subscript->getSubscriptLoc(), @@ -8349,7 +8343,7 @@ const clang::CXXConstructorDecl * importer::findCopyConstructor(const clang::CXXRecordDecl *decl) { for (auto ctor : decl->ctors()) { if (ctor->isCopyConstructor() && - // FIXME: Support default arguments (rdar://142414553) + // FIXME: Support default arguments ctor->getNumParams() == 1 && ctor->getAccess() == clang::AS_public && !ctor->isDeleted() && !ctor->isIneligibleOrNotSelected()) return ctor; @@ -8364,9 +8358,8 @@ static bool hasMoveTypeOperations(const clang::CXXRecordDecl *decl) { return llvm::any_of(decl->ctors(), [](clang::CXXConstructorDecl *ctor) { return ctor->isMoveConstructor() && !ctor->isDeleted() && !ctor->isIneligibleOrNotSelected() && - // FIXME: Support default arguments (rdar://142414553) - ctor->getNumParams() == 1 && - ctor->getAccess() == clang::AS_public; + // FIXME: Support default arguments + ctor->getNumParams() == 1 && ctor->getAccess() == clang::AS_public; }); } @@ -8393,7 +8386,7 @@ static bool hasConstructorWithUnsupportedDefaultArgs(const clang::CXXRecordDecl *decl) { return llvm::any_of(decl->ctors(), [](clang::CXXConstructorDecl *ctor) { return (ctor->isCopyConstructor() || ctor->isMoveConstructor()) && - // FIXME: Support default arguments (rdar://142414553) + // FIXME: Support default arguments ctor->getNumParams() != 1; }); } diff --git a/lib/ClangImporter/SwiftDeclSynthesizer.cpp b/lib/ClangImporter/SwiftDeclSynthesizer.cpp index 1a204f9fb5561..37c880f0ca7c4 100644 --- a/lib/ClangImporter/SwiftDeclSynthesizer.cpp +++ b/lib/ClangImporter/SwiftDeclSynthesizer.cpp @@ -1688,8 +1688,13 @@ synthesizeUnwrappingAddressSetterBody(AbstractFunctionDecl *afd, ASTContext &ctx = setterDecl->getASTContext(); auto selfArg = createSelfArg(setterDecl); + SmallVector arguments; + for (size_t idx = 0, end = setterDecl->getParameters()->size(); idx < end; + ++idx) + arguments.push_back(createParamRefExpr(setterDecl, idx)); + auto *setterImplCallExpr = - createAccessorImplCallExpr(setterImpl, selfArg, {}); + createAccessorImplCallExpr(setterImpl, selfArg, arguments); auto *returnStmt = ReturnStmt::createImplicit(ctx, setterImplCallExpr); @@ -1708,7 +1713,6 @@ SubscriptDecl *SwiftDeclSynthesizer::makeSubscript(FuncDecl *getter, FuncDecl *getterImpl = getter ? getter : setter; FuncDecl *setterImpl = setter; - // FIXME: support unsafeAddress accessors. // Get the return type wrapped in `Unsafe(Mutable)Pointer`. const auto rawElementTy = getterImpl->getResultInterfaceType(); // Unwrap `T`. Use rawElementTy for return by value. @@ -1737,17 +1741,27 @@ SubscriptDecl *SwiftDeclSynthesizer::makeSubscript(FuncDecl *getter, getterImpl->getClangNode()); subscript->copyFormalAccessFrom(getterImpl); - AccessorDecl *getterDecl = - AccessorDecl::create(ctx, getterImpl->getLoc(), getterImpl->getLoc(), - AccessorKind::Get, subscript, - /*async*/ false, SourceLoc(), - /*throws*/ false, SourceLoc(), - /*ThrownType=*/TypeLoc(), bodyParams, elementTy, dc); + // Use 'address' or 'mutableAddress' accessors for non-copyable + // types that are returned indirectly. + bool useAddress = + rawElementTy->getAnyPointerElementType() && elementTy->isNoncopyable(); + + AccessorDecl *getterDecl = AccessorDecl::create( + ctx, getterImpl->getLoc(), getterImpl->getLoc(), + useAddress ? AccessorKind::Address : AccessorKind::Get, subscript, + /*async*/ false, SourceLoc(), + /*throws*/ false, SourceLoc(), + /*ThrownType=*/TypeLoc(), bodyParams, + useAddress ? elementTy->wrapInPointer(PTK_UnsafePointer) : elementTy, dc); getterDecl->copyFormalAccessFrom(subscript); - getterDecl->setImplicit(); + if (!useAddress) + getterDecl->setImplicit(); getterDecl->setIsDynamic(false); getterDecl->setIsTransparent(true); - getterDecl->setBodySynthesizer(synthesizeUnwrappingGetterBody, getterImpl); + getterDecl->setBodySynthesizer(useAddress + ? synthesizeUnwrappingAddressGetterBody + : synthesizeUnwrappingGetterBody, + getterImpl); if (getterImpl->isMutating()) { getterDecl->setSelfAccessKind(SelfAccessKind::Mutating); @@ -1763,22 +1777,31 @@ SubscriptDecl *SwiftDeclSynthesizer::makeSubscript(FuncDecl *getter, paramVarDecl->setInterfaceType(elementTy); SmallVector setterParams; - setterParams.push_back(paramVarDecl); + if (!useAddress) + setterParams.push_back(paramVarDecl); setterParams.append(bodyParams->begin(), bodyParams->end()); auto setterParamList = ParameterList::create(ctx, setterParams); setterDecl = AccessorDecl::create( - ctx, setterImpl->getLoc(), setterImpl->getLoc(), AccessorKind::Set, + ctx, setterImpl->getLoc(), setterImpl->getLoc(), + useAddress ? AccessorKind::MutableAddress : AccessorKind::Set, subscript, /*async*/ false, SourceLoc(), /*throws*/ false, SourceLoc(), /*ThrownType=*/TypeLoc(), - setterParamList, TupleType::getEmpty(ctx), dc); + setterParamList, + useAddress ? elementTy->wrapInPointer(PTK_UnsafeMutablePointer) + : TupleType::getEmpty(ctx), + dc); setterDecl->copyFormalAccessFrom(subscript); - setterDecl->setImplicit(); + if (!useAddress) + 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); diff --git a/lib/IRGen/GenStruct.cpp b/lib/IRGen/GenStruct.cpp index 2c789392fe8a0..2841840217850 100644 --- a/lib/IRGen/GenStruct.cpp +++ b/lib/IRGen/GenStruct.cpp @@ -562,7 +562,7 @@ namespace { return nullptr; for (auto ctor : cxxRecordDecl->ctors()) { if (ctor->isMoveConstructor() && - // FIXME: Support default arguments (rdar://142414553) + // FIXME: Support default arguments ctor->getNumParams() == 1 && ctor->getAccess() == clang::AS_public && !ctor->isDeleted() && !ctor->isIneligibleOrNotSelected()) diff --git a/test/Interop/Cxx/class/noncopyable-irgen.swift b/test/Interop/Cxx/class/noncopyable-irgen.swift index a4928a93b2ae3..9e4e7d9c66998 100644 --- a/test/Interop/Cxx/class/noncopyable-irgen.swift +++ b/test/Interop/Cxx/class/noncopyable-irgen.swift @@ -4,6 +4,7 @@ // RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-additional-prefix TEST2- -D TEST2 // RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-additional-prefix TEST3- -D TEST3 // RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-additional-prefix TEST4- -D TEST4 -Xcc -std=c++20 +// RUN: %target-swift-frontend -cxx-interoperability-mode=default -emit-ir -I %swift_src_root/lib/ClangImporter/SwiftBridging -I %t%{fs-sep}Inputs %t%{fs-sep}test.swift -Xcc -fignore-exceptions -verify -verify-additional-file %t%{fs-sep}Inputs%{fs-sep}noncopyable.h -verify-additional-prefix TEST5- -D TEST5 //--- Inputs/module.modulemap module Test { @@ -17,8 +18,15 @@ module Test { struct NonCopyable { NonCopyable() = default; - NonCopyable(const NonCopyable& other) = delete; // expected-note {{'NonCopyable' has been explicitly marked deleted here}} + NonCopyable(int x) : number(x) {} + NonCopyable(const NonCopyable& other) = delete; + // expected-TEST1-note@-1 {{'NonCopyable' has been explicitly marked deleted here}} + // expected-TEST2-note@-2 {{'NonCopyable' has been explicitly marked deleted here}} + // expected-TEST3-note@-3 {{'NonCopyable' has been explicitly marked deleted here}} + // expected-TEST4-note@-4 {{'NonCopyable' has been explicitly marked deleted here}} NonCopyable(NonCopyable&& other) = default; + + int number = 0; }; template @@ -73,6 +81,11 @@ template struct Requires { using RequiresOwnsNonCopyable = Requires>; #endif +struct HasSubscript { + NonCopyable &operator[](int idx) { return nc; } + NonCopyable nc; +}; + //--- test.swift import Test import CxxStdlib @@ -109,4 +122,13 @@ func requires() { takeCopyable(s) } +#elseif TEST5 +func useSubscript() { + var obj = HasSubscript(nc: NonCopyable(5)) + let _ = obj[42] // expected-TEST5-error {{'obj.subscript' is borrowed and cannot be consumed}} + // expected-TEST5-note@-1 {{consumed here}} + + func borrow(_ x: borrowing NonCopyable) -> Int32 { return x.number; } + let _ = borrow(obj[42]) +} #endif diff --git a/test/Interop/Cxx/class/noncopyable-typechecker.swift b/test/Interop/Cxx/class/noncopyable-typechecker.swift index 7572e2757a246..85466693a71e7 100644 --- a/test/Interop/Cxx/class/noncopyable-typechecker.swift +++ b/test/Interop/Cxx/class/noncopyable-typechecker.swift @@ -16,8 +16,11 @@ module Test { struct NonCopyable { NonCopyable() = default; + NonCopyable(int x) : number(x) {} NonCopyable(const NonCopyable& other) = delete; // expected-note {{'NonCopyable' has been explicitly marked deleted here}} NonCopyable(NonCopyable&& other) = default; + + int number = 0; }; template @@ -137,6 +140,11 @@ struct FieldDependsOnMe { // used to trigger a request cycle OneField> field; }; +struct HasConstSubscript { + const NonCopyable &operator[](int idx) { return nc; } + NonCopyable nc; +}; + //--- test.swift import Test import CxxStdlib @@ -218,3 +226,9 @@ func couldCreateCycleOfCxxValueSemanticsRequests() { let d3 = FieldDependsOnMe() takeCopyable(d3) // expected-error {{global function 'takeCopyable' requires that 'FieldDependsOnMe' conform to 'Copyable'}} } + +func useConstSubscript() { + var obj = HasConstSubscript(nc: NonCopyable(5)) + _ = obj[42] + obj[42] = NonCopyable(7) // expected-error {{cannot assign through subscript: subscript is get-only}} +} diff --git a/test/Interop/Cxx/class/noncopyable.swift b/test/Interop/Cxx/class/noncopyable.swift new file mode 100644 index 0000000000000..1c334bcd7ea8f --- /dev/null +++ b/test/Interop/Cxx/class/noncopyable.swift @@ -0,0 +1,74 @@ +// RUN: %empty-directory(%t) +// RUN: split-file %s %t +// RUN: %target-build-swift %t%{fs-sep}test.swift -I %t%{fs-sep}Inputs -o %t%{fs-sep}out -cxx-interoperability-mode=default +// RUN: %target-codesign %t%{fs-sep}out +// RUN: %target-run %t%{fs-sep}out +// +// REQUIRES: executable_test + +//--- Inputs/module.modulemap +module Test { + header "noncopyable.h" + requires cplusplus +} + +//--- Inputs/noncopyable.h +#include + +struct NonCopyable { + NonCopyable() = default; + NonCopyable(int x) : number(x) {} + NonCopyable(const NonCopyable& other) = delete; + NonCopyable(NonCopyable&& other) = default; + + int number = 0; +}; + +template +struct HasSubscript { + T &operator[](int idx) { return element; } + T element; +}; + +using HasSubscriptInt = HasSubscript; +using HasSubscriptNonCopyable = HasSubscript; + +struct InheritsFromHasSubscript : public HasSubscript {}; + +//--- test.swift +import StdlibUnittest +import Test + +var NonCopyableTestSuite = TestSuite("NonCopyable") + +func borrow(_ x: borrowing NonCopyable) -> Int32 { return x.number; } + +NonCopyableTestSuite.test("Use subscript") { + var o1 = HasSubscriptInt(element: 7) + expectEqual(o1[42], 7) + + var o2 = HasSubscriptNonCopyable(element: NonCopyable(5)) + expectEqual(borrow(o2[42]), 5) + + var inherited = InheritsFromHasSubscript() + expectEqual(borrow(inherited[42]), 0) +} + +NonCopyableTestSuite.test("Mutate subscript") { + var o1 = HasSubscriptInt(element: 7) + expectEqual(o1[42], 7) + o1[42] = 8 + expectEqual(o1[42], 8) + + var o2 = HasSubscriptNonCopyable(element: NonCopyable(5)) + expectEqual(borrow(o2[42]), 5) + o2[42] = NonCopyable(16) + expectEqual(borrow(o2[42]), 16) + + var inherited = InheritsFromHasSubscript() + expectEqual(borrow(inherited[42]), 0) + inherited[42] = NonCopyable(3) + expectEqual(borrow(inherited[42]), 3) +} + +runAllTests() diff --git a/test/Interop/Cxx/stdlib/Inputs/std-vector.h b/test/Interop/Cxx/stdlib/Inputs/std-vector.h index 403d883f86b82..848decbcc998b 100644 --- a/test/Interop/Cxx/stdlib/Inputs/std-vector.h +++ b/test/Interop/Cxx/stdlib/Inputs/std-vector.h @@ -32,14 +32,24 @@ using VectorOfImmortalRefPtr = std::vector; struct NonCopyable { NonCopyable() = default; + NonCopyable(int x) : number(x) {} NonCopyable(const NonCopyable &other) = delete; NonCopyable(NonCopyable &&other) = default; ~NonCopyable() {} + + int number = 0; }; using VectorOfNonCopyable = std::vector; using VectorOfPointer = std::vector; +inline VectorOfNonCopyable makeVectorOfNonCopyable() { + VectorOfNonCopyable vec; + vec.emplace_back(1); + vec.emplace_back(2); + return vec; +} + struct HasVector { std::vector field; }; diff --git a/test/Interop/Cxx/stdlib/use-std-vector.swift b/test/Interop/Cxx/stdlib/use-std-vector.swift index 5e2a1f0d9c6dc..7062014d2853d 100644 --- a/test/Interop/Cxx/stdlib/use-std-vector.swift +++ b/test/Interop/Cxx/stdlib/use-std-vector.swift @@ -211,4 +211,24 @@ StdVectorTestSuite.test("VectorOfImmortalRefPtr").require(.stdlib_5_8).code { expectEqual(v[0]?.value, 123) } +StdVectorTestSuite.test("Subscript of VectorOfNonCopyable") { + var v = makeVectorOfNonCopyable() + expectEqual(v.size(), 2) + expectFalse(v.empty()) + + func getNumber(_ x: borrowing NonCopyable) -> Int32 { + return x.number + } + + expectEqual(getNumber(v[0]), 1) + expectEqual(getNumber(v[1]), 2) + + v[0] = NonCopyable(3) + v[1] = NonCopyable(4) + + expectEqual(getNumber(v[0]), 3) + expectEqual(getNumber(v[1]), 4) +} + + runAllTests()