[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

Author: Gabor Horvath

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