From 66d15a18135457fddc4e3c8d8f06df2a3792a0f3 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 28 Jul 2025 10:20:22 +0100 Subject: [PATCH] Reapply [cxx-interop] Avoid copies when accessing pointee After #83289 and #82879 landed we should no longer get deserialization failures and this feature is no longer behind a flag. This patch also changes how we query if a function's return value depends on self. Previously, we queried the lifetime dependencies from the Swift declaration. Unfortunately, this is problematic as we might have not finished fully importing the types in the function signature just yet and the compiler might end up populating the conformance tables prematurely. To work this around, I store functions with self-dependent return values where lifetimes are computed in the importer for later use. The PR also adds a test to make sure the addressable dependency feature will not result in deserialization errors. rdar://155319311&154213694&112690482&128293252 --- lib/ClangImporter/ImportDecl.cpp | 64 +++++++++++++++++-- lib/ClangImporter/ImporterImpl.h | 6 ++ lib/ClangImporter/SwiftDeclSynthesizer.cpp | 7 +- .../methods-addressable-dependency.swift | 29 +++++++++ .../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 | 40 ++++++++++++ 8 files changed, 182 insertions(+), 7 deletions(-) create mode 100644 test/Interop/Cxx/class/method/methods-addressable-dependency.swift 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 5052c09d8b61f..5cfd0b37ff617 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -4107,6 +4107,50 @@ 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)); + Impl.returnsSelfDependentValue.insert(result); + return true; + } + return false; + } + + static bool isReturnDependsOnSelf( + AbstractFunctionDecl *f, + const ArrayRef &lifetimeDeps) { + if (isa(f) || !f->getImportAsMemberStatus().isInstance()) + return false; + for (auto dependence : lifetimeDeps) { + auto returnIdx = f->getParameters()->size() + !isa(f); + if (!dependence.hasInheritLifetimeParamIndices() && + dependence.hasScopeLifetimeParamIndices() && + dependence.getTargetIndex() == returnIdx) + return dependence.getScopeIndices()->contains(f->getSelfIndex()); + } + return false; + } + void addLifetimeDependencies(const clang::FunctionDecl *decl, AbstractFunctionDecl *result) { if (decl->getTemplatedKind() == clang::FunctionDecl::TK_FunctionTemplate) @@ -4125,10 +4169,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); @@ -4151,10 +4204,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); @@ -4233,7 +4283,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. @@ -4272,6 +4322,10 @@ namespace { } Impl.diagnoseTargetDirectly(decl); + + if (isReturnDependsOnSelf(result, lifetimeDependencies)) { + Impl.returnsSelfDependentValue.insert(result); + } } void finishFuncDecl(const clang::FunctionDecl *decl, diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index 8d3507d40f220..e7c7fd3094a0e 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -732,6 +732,12 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation /// properties. llvm::DenseMap FunctionsAsProperties; + /// Calling AbstractFunctionDecl::getLifetimeDependencies before we added + /// the conformances we want to all the imported types is problematic because + /// it will populate the conformance too early. To avoid the need for that we + /// store functions with interesting lifetimes. + llvm::DenseSet returnsSelfDependentValue; + importer::EnumInfo getEnumInfo(const clang::EnumDecl *decl) { return getNameImporter().getEnumInfo(decl); } diff --git a/lib/ClangImporter/SwiftDeclSynthesizer.cpp b/lib/ClangImporter/SwiftDeclSynthesizer.cpp index 7ae73a79977a2..2544844f334de 100644 --- a/lib/ClangImporter/SwiftDeclSynthesizer.cpp +++ b/lib/ClangImporter/SwiftDeclSynthesizer.cpp @@ -1691,6 +1691,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. @@ -1788,6 +1789,8 @@ SwiftDeclSynthesizer::makeDereferencedPointeeProperty(FuncDecl *getter, FuncDecl *getterImpl = getter ? getter : setter; FuncDecl *setterImpl = setter; auto dc = getterImpl->getDeclContext(); + bool resultDependsOnSelf = + ImporterImpl.returnsSelfDependentValue.contains(getterImpl); // Get the return type wrapped in `Unsafe(Mutable)Pointer`. const auto rawElementTy = getterImpl->getResultInterfaceType(); @@ -1798,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-dependency.swift b/test/Interop/Cxx/class/method/methods-addressable-dependency.swift new file mode 100644 index 0000000000000..fd8c6f634ad36 --- /dev/null +++ b/test/Interop/Cxx/class/method/methods-addressable-dependency.swift @@ -0,0 +1,29 @@ +// This ensures that a Swift module built without AddressableParameters can be +// consumed by a dependency Swift module that enables AddressableParameters. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %target-build-swift %t/library.swift -emit-module -emit-library -enable-library-evolution -cxx-interoperability-mode=upcoming-swift -module-name MyLibrary -emit-module-path %t/artifacts/MyLibrary.swiftmodule -emit-module-interface-path %t/artifacts/MyLibrary.swiftinterface -I %S/Inputs -swift-version 6 -enable-experimental-feature AssumeResilientCxxTypes +// RUN: rm %t/artifacts/MyLibrary.swiftmodule +// RUN: %target-build-swift %t/executable.swift -emit-irgen -cxx-interoperability-mode=default -module-name ImportsMyLibrary -I %t/artifacts -I %S/Inputs -swift-version 6 -enable-experimental-feature AssumeResilientCxxTypes -enable-experimental-feature AddressableParameters + +// REQUIRES: swift_feature_AddressableParameters +// REQUIRES: swift_feature_AssumeResilientCxxTypes + +//--- library.swift +import Methods + +@inlinable // emit the body of the function into the textual interface +public func addressableTest(_ x: borrowing NonTrivialInWrapper) { + let m = HasMethods() + m.nonTrivialTakesConstRef(x) +} + +//--- executable.swift +import MyLibrary +import Methods + +let x = NonTrivialInWrapper(123) +let m = HasMethods() +m.nonTrivialTakesConstRef(x) 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..6ee02fc37931b --- /dev/null +++ b/test/Interop/Cxx/stdlib/avoid-redundant-copies.swift @@ -0,0 +1,40 @@ +// RUN: %target-run-simple-swift(-I %S/Inputs -cxx-interoperability-mode=upcoming-swift) + +// REQUIRES: executable_test + +// 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()