From 7474a516918c9e8ab7e825efa9656042de3c450f Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Thu, 10 Apr 2025 13:29:49 +0100 Subject: [PATCH] [cxx-interop] Avoid copies when accessing pointee Previously, we would get two copies, one accessing the pointee and one when we pass the pointee as a method as the implicit self argument. These copies are unsafe as they might introduce slicing. When addressable paramaters features are enabled, we no longer make these copies for the standard STL types. Custom smart pointers can replicate this by making the lifetime dependency between the implicit object parameter and the returned reference of operator* explicit via a lifetime annotation. rdar://154213694&128293252&112690482 --- lib/ClangImporter/ImportDecl.cpp | 47 ++++++++++++++++--- lib/ClangImporter/SwiftDeclSynthesizer.cpp | 23 ++++++++- .../method/methods-addressable-silgen.swift | 2 +- .../Cxx/stdlib/Inputs/custom-smart-ptr.h | 22 +++++++++ .../Cxx/stdlib/Inputs/module.modulemap | 6 +++ .../Cxx/stdlib/Inputs/std-unique-ptr.h | 15 ++++++ .../Cxx/stdlib/avoid-redundant-copies.swift | 41 ++++++++++++++++ 7 files changed, 147 insertions(+), 9 deletions(-) create mode 100644 test/Interop/Cxx/stdlib/Inputs/custom-smart-ptr.h create mode 100644 test/Interop/Cxx/stdlib/avoid-redundant-copies.swift diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 8d9227cc0f9ab..524a4756a1294 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -4097,7 +4097,8 @@ namespace { func->setSelfIndex(selfIdx.value()); if (Impl.SwiftContext.LangOpts.hasFeature( Feature::AddressableParameters)) - func->getImplicitSelfDecl()->setAddressable(); + func->getAttrs().add(new (Impl.SwiftContext) + AddressableSelfAttr(true)); } else { func->setStatic(); func->setImportAsStaticMember(); @@ -4145,6 +4146,34 @@ namespace { return false; } + // Inject lifetime annotations selectively for some STL types so we can use + // unsafeAddress to avoid copies. + bool inferSelfDependence(const clang::FunctionDecl *decl, + AbstractFunctionDecl *result, size_t returnIdx) { + const auto *method = dyn_cast(decl); + if (!method) + return false; + const auto *enclosing = method->getParent(); + if (enclosing->isInStdNamespace() && + (enclosing->getName() == "unique_ptr" || + enclosing->getName() == "shared_ptr") && + method->isOverloadedOperator() && + method->getOverloadedOperator() == clang::OO_Star) { + SmallVector lifetimeDependencies; + SmallBitVector dependenciesOfRet(returnIdx); + dependenciesOfRet[result->getSelfIndex()] = true; + lifetimeDependencies.push_back(LifetimeDependenceInfo( + nullptr, IndexSubset::get(Impl.SwiftContext, dependenciesOfRet), + returnIdx, + /*isImmortal*/ false)); + Impl.SwiftContext.evaluator.cacheOutput( + LifetimeDependenceInfoRequest{result}, + Impl.SwiftContext.AllocateCopy(lifetimeDependencies)); + return true; + } + return false; + } + void addLifetimeDependencies(const clang::FunctionDecl *decl, AbstractFunctionDecl *result) { if (decl->getTemplatedKind() == clang::FunctionDecl::TK_FunctionTemplate) @@ -4163,10 +4192,19 @@ namespace { CxxEscapability::Unknown) != CxxEscapability::NonEscapable; }; + auto swiftParams = result->getParameters(); + bool hasSelf = + result->hasImplicitSelfDecl() && !isa(result); + auto returnIdx = swiftParams->size() + hasSelf; + + if (inferSelfDependence(decl, result, returnIdx)) + return; + // FIXME: this uses '0' as the result index. That only works for // standalone functions with no parameters. // See markReturnsUnsafeNonescapable() for a general approach. auto &ASTContext = result->getASTContext(); + SmallVector lifetimeDependencies; LifetimeDependenceInfo immortalLifetime(nullptr, nullptr, 0, /*isImmortal*/ true); @@ -4189,10 +4227,7 @@ namespace { } }; - auto swiftParams = result->getParameters(); - bool hasSelf = - result->hasImplicitSelfDecl() && !isa(result); - const auto dependencyVecSize = swiftParams->size() + hasSelf; + const auto dependencyVecSize = returnIdx; SmallBitVector inheritLifetimeParamIndicesForReturn(dependencyVecSize); SmallBitVector scopedLifetimeParamIndicesForReturn(dependencyVecSize); SmallBitVector paramHasAnnotation(dependencyVecSize); @@ -4271,7 +4306,7 @@ namespace { ? IndexSubset::get(Impl.SwiftContext, scopedLifetimeParamIndicesForReturn) : nullptr, - swiftParams->size() + hasSelf, + returnIdx, /*isImmortal*/ false)); else if (auto *ctordecl = dyn_cast(decl)) { // Assume default constructed view types have no dependencies. diff --git a/lib/ClangImporter/SwiftDeclSynthesizer.cpp b/lib/ClangImporter/SwiftDeclSynthesizer.cpp index 24e1a8d94e565..6937f58808bef 100644 --- a/lib/ClangImporter/SwiftDeclSynthesizer.cpp +++ b/lib/ClangImporter/SwiftDeclSynthesizer.cpp @@ -1679,6 +1679,7 @@ 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. @@ -1761,6 +1762,23 @@ SubscriptDecl *SwiftDeclSynthesizer::makeSubscript(FuncDecl *getter, return subscript; } +static bool doesReturnDependsOnSelf(FuncDecl *f) { + if (!f->getASTContext().LangOpts.hasFeature(Feature::AddressableParameters)) + return false; + if (!f->hasImplicitSelfDecl()) + return false; + if (auto deps = f->getLifetimeDependencies()) { + for (auto dependence : *deps) { + auto returnIdx = f->getParameters()->size() + !isa(f); + if (!dependence.hasInheritLifetimeParamIndices() && + dependence.hasScopeLifetimeParamIndices() && + dependence.getTargetIndex() == returnIdx) + return dependence.getScopeIndices()->contains(f->getSelfIndex()); + } + } + return false; +} + // MARK: C++ dereference operator VarDecl * @@ -1772,6 +1790,7 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter, FuncDecl *getterImpl = getter ? getter : setter; FuncDecl *setterImpl = setter; auto dc = getterImpl->getDeclContext(); + bool resultDependsOnSelf = doesReturnDependsOnSelf(getterImpl); // Get the return type wrapped in `Unsafe(Mutable)Pointer`. const auto rawElementTy = getterImpl->getResultInterfaceType(); @@ -1782,9 +1801,9 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter, // Use 'address' or 'mutableAddress' accessors for non-copyable // types that are returned indirectly. bool isNoncopyable = dc->mapTypeIntoContext(elementTy)->isNoncopyable(); - bool isImplicit = !isNoncopyable; + bool isImplicit = !(isNoncopyable || resultDependsOnSelf); bool useAddress = - rawElementTy->getAnyPointerElementType() && isNoncopyable; + rawElementTy->getAnyPointerElementType() && (isNoncopyable || resultDependsOnSelf); auto result = new (ctx) VarDecl(/*isStatic*/ false, VarDecl::Introducer::Var, diff --git a/test/Interop/Cxx/class/method/methods-addressable-silgen.swift b/test/Interop/Cxx/class/method/methods-addressable-silgen.swift index 485851edae2fe..4578263e46df8 100644 --- a/test/Interop/Cxx/class/method/methods-addressable-silgen.swift +++ b/test/Interop/Cxx/class/method/methods-addressable-silgen.swift @@ -17,7 +17,7 @@ public func addressableTest(x: borrowing @_addressable NonTrivialInWrapper, y: i // CHECK: %{{[0-9]+}} = apply %{{[0-9]+}}([[UNWRAPPED]], %{{[0-9]+}}) : $@convention(cxx_method) (@in_guaranteed NonTrivialInWrapper, @in_guaranteed HasMethods) -> () var m2 = HasMethods() // CHECK: [[ACCESS:%[0-9]+]] = begin_access [modify] [unknown] [[INPUT2]] - // CHECK: %{{[0-9]+}} = apply %32([[ACCESS]], %{{[0-9]+}}) : $@convention(cxx_method) (@inout NonTrivialInWrapper, @in_guaranteed HasMethods) -> () + // CHECK: %{{[0-9]+}} = apply %{{[0-9]+}}([[ACCESS]], %{{[0-9]+}}) : $@convention(cxx_method) (@inout NonTrivialInWrapper, @in_guaranteed HasMethods) -> () // CHECK-NEXT: end_access [[ACCESS]] m2.nonTrivialTakesRef(&y) } diff --git a/test/Interop/Cxx/stdlib/Inputs/custom-smart-ptr.h b/test/Interop/Cxx/stdlib/Inputs/custom-smart-ptr.h new file mode 100644 index 0000000000000..617b728fb79d7 --- /dev/null +++ b/test/Interop/Cxx/stdlib/Inputs/custom-smart-ptr.h @@ -0,0 +1,22 @@ +#pragma once + +static int copies2 = 0; + +struct CountCopies2 { + CountCopies2() = default; + CountCopies2(const CountCopies2& other) { ++copies2; } + ~CountCopies2() {} + + int getCopies() const { return copies2; } + void method() {} + void constMethod() const {} + int field = 42; +}; + +struct MySmartPtr { + CountCopies2& operator*() const [[clang::lifetimebound]] { return *ptr; } + + CountCopies2* ptr; +}; + +inline MySmartPtr getPtr() { return MySmartPtr{new CountCopies2()}; } diff --git a/test/Interop/Cxx/stdlib/Inputs/module.modulemap b/test/Interop/Cxx/stdlib/Inputs/module.modulemap index 57f7eae3bb670..0ca81ec0670af 100644 --- a/test/Interop/Cxx/stdlib/Inputs/module.modulemap +++ b/test/Interop/Cxx/stdlib/Inputs/module.modulemap @@ -81,3 +81,9 @@ module NoCXXStdlib { requires cplusplus export * } + +module CustomSmartPtr { + header "custom-smart-ptr.h" + requires cplusplus + export * +} diff --git a/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h b/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h index abda254b64f7d..f805ddb955cc3 100644 --- a/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h +++ b/test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h @@ -66,4 +66,19 @@ std::shared_ptr makeStringShared() { return std::make_unique("Shared string"); } +static int copies = 0; + +struct CountCopies { + CountCopies() = default; + CountCopies(const CountCopies& other) { ++copies; } + ~CountCopies() {} + + int getCopies() const { return copies; } + void method() {} + void constMethod() const {} + int field = 42; +}; + +inline std::unique_ptr getCopyCountedUniquePtr() { return std::make_unique(); } + #endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_UNIQUE_PTR_H diff --git a/test/Interop/Cxx/stdlib/avoid-redundant-copies.swift b/test/Interop/Cxx/stdlib/avoid-redundant-copies.swift new file mode 100644 index 0000000000000..d80b3d8dba5ed --- /dev/null +++ b/test/Interop/Cxx/stdlib/avoid-redundant-copies.swift @@ -0,0 +1,41 @@ +// RUN: %target-run-simple-swift(-I %S/Inputs -cxx-interoperability-mode=upcoming-swift -enable-experimental-feature AddressableParameters) + +// REQUIRES: executable_test +// REQUIRES: swift_feature_AddressableParameters + +// https://github.com/apple/swift/issues/70226 +// UNSUPPORTED: OS=windows-msvc + +import StdlibUnittest +import StdUniquePtr +import CustomSmartPtr +import CxxStdlib + +var AvoidCopiesSuite = TestSuite("AvoidRedundantCopies") + +AvoidCopiesSuite.test("The pointee does not copy when passed as self") { + let up = getNonCopyableUniquePtr() + expectEqual(up.pointee.method(1), 42) + expectEqual(up.pointee.method(1), 42) + let cup = getCopyCountedUniquePtr(); + expectEqual(cup.pointee.getCopies(), 0) + cup.pointee.method() + cup.pointee.constMethod() + let _ = cup.pointee.field + expectEqual(cup.pointee.getCopies(), 0) + let copy = cup.pointee + expectEqual(copy.getCopies(), 1) +} + +AvoidCopiesSuite.test("The custom smart pointer pointee does not copy when passed as self") { + let myptr = getPtr(); + expectEqual(myptr.pointee.getCopies(), 0) + myptr.pointee.method() + myptr.pointee.constMethod() + let _ = myptr.pointee.field + expectEqual(myptr.pointee.getCopies(), 0) + let copy = myptr.pointee + expectEqual(copy.getCopies(), 1) +} + +runAllTests()