Merge pull request #82480 from swiftlang/gaborh/addressable_params_copy

[cxx-interop] Avoid copies when accessing pointee

Author: Xazax-hun

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<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));
+        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<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);
@@ -4189,10 +4227,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);
@@ -4271,7 +4306,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.

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

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

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