Merge pull request swiftlang#83370 from Xazax-hun/addressable-param-copy-reapply

Reapply [cxx-interop] Avoid copies when accessing pointee

Author: Xazax-hun

lib/ClangImporter/ImportDecl.cpp

@@ -4117,6 +4117,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<clang::CXXMethodDecl>(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<LifetimeDependenceInfo, 1> 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<LifetimeDependenceInfo> &lifetimeDeps) {
+      if (isa<ConstructorDecl>(f) || !f->getImportAsMemberStatus().isInstance())
+        return false;
+      for (auto dependence : lifetimeDeps) {
+        auto returnIdx = f->getParameters()->size() + !isa<ConstructorDecl>(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)
@@ -4135,10 +4179,19 @@ namespace {
                    CxxEscapability::Unknown) != CxxEscapability::NonEscapable;
       };
 
+      auto swiftParams = result->getParameters();
+      bool hasSelf =
+          result->hasImplicitSelfDecl() && !isa<ConstructorDecl>(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<LifetimeDependenceInfo, 1> lifetimeDependencies;
       LifetimeDependenceInfo immortalLifetime(nullptr, nullptr, 0,
                                               /*isImmortal*/ true);
@@ -4161,10 +4214,7 @@ namespace {
         }
       };
 
-      auto swiftParams = result->getParameters();
-      bool hasSelf =
-          result->hasImplicitSelfDecl() && !isa<ConstructorDecl>(result);
-      const auto dependencyVecSize = swiftParams->size() + hasSelf;
+      const auto dependencyVecSize = returnIdx;
       SmallBitVector inheritLifetimeParamIndicesForReturn(dependencyVecSize);
       SmallBitVector scopedLifetimeParamIndicesForReturn(dependencyVecSize);
       SmallBitVector paramHasAnnotation(dependencyVecSize);
@@ -4243,7 +4293,7 @@ namespace {
                 ? IndexSubset::get(Impl.SwiftContext,
                                    scopedLifetimeParamIndicesForReturn)
                 : nullptr,
-            swiftParams->size() + hasSelf,
+            returnIdx,
             /*isImmortal*/ false));
       else if (auto *ctordecl = dyn_cast<clang::CXXConstructorDecl>(decl)) {
         // Assume default constructed view types have no dependencies.
@@ -4282,6 +4332,10 @@ namespace {
       }
 
       Impl.diagnoseTargetDirectly(decl);
+
+      if (isReturnDependsOnSelf(result, lifetimeDependencies)) {
+        Impl.returnsSelfDependentValue.insert(result);
+      }
     }
 
     void finishFuncDecl(const clang::FunctionDecl *decl,

lib/ClangImporter/ImporterImpl.h

@@ -732,6 +732,12 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   /// properties.
   llvm::DenseMap<const clang::FunctionDecl *, VarDecl *> 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<const AbstractFunctionDecl *> returnsSelfDependentValue;
+
   importer::EnumInfo getEnumInfo(const clang::EnumDecl *decl) {
     return getNameImporter().getEnumInfo(decl);
   }

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<T>`.
   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<T>`.
   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,

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)

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()}; }

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 *
+}

test/Interop/Cxx/stdlib/Inputs/std-unique-ptr.h

@@ -66,4 +66,19 @@ std::shared_ptr<std::string> makeStringShared() {
   return std::make_unique<std::string>("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<CountCopies> getCopyCountedUniquePtr() { return std::make_unique<CountCopies>(); }
+
 #endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_UNIQUE_PTR_H

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()