Reapply [cxx-interop] Avoid copies when accessing pointee

After swiftlang#83289 and swiftlang#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

Author: Gabor Horvath

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<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)
@@ -4125,10 +4169,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);
@@ -4151,10 +4204,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);
@@ -4233,7 +4283,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.
@@ -4272,6 +4322,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()