Merge pull request #83231 from Xazax-hun/addressable-params-copy-revert

Revert [cxx-interop] Avoid copies when accessing pointee

lib/ClangImporter/ImportDecl.cpp

@@ -4098,8 +4098,7 @@ namespace {
             func->setSelfIndex(selfIdx.value());
             if (Impl.SwiftContext.LangOpts.hasFeature(
                     Feature::AddressableParameters))
-              func->getAttrs().add(new (Impl.SwiftContext)
-                                       AddressableSelfAttr(true));
+              func->getImplicitSelfDecl()->setAddressable();
           } else {
             func->setStatic();
             func->setImportAsStaticMember();
@@ -4147,34 +4146,6 @@ 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)
@@ -4193,19 +4164,10 @@ 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);
@@ -4228,7 +4190,10 @@ namespace {
         }
       };
 
-      const auto dependencyVecSize = returnIdx;
+      auto swiftParams = result->getParameters();
+      bool hasSelf =
+          result->hasImplicitSelfDecl() && !isa<ConstructorDecl>(result);
+      const auto dependencyVecSize = swiftParams->size() + hasSelf;
       SmallBitVector inheritLifetimeParamIndicesForReturn(dependencyVecSize);
       SmallBitVector scopedLifetimeParamIndicesForReturn(dependencyVecSize);
       SmallBitVector paramHasAnnotation(dependencyVecSize);
@@ -4307,7 +4272,7 @@ namespace {
                 ? IndexSubset::get(Impl.SwiftContext,
                                    scopedLifetimeParamIndicesForReturn)
                 : nullptr,
-            returnIdx,
+            swiftParams->size() + hasSelf,
             /*isImmortal*/ false));
       else if (auto *ctordecl = dyn_cast<clang::CXXConstructorDecl>(decl)) {
         // Assume default constructed view types have no dependencies.

lib/ClangImporter/SwiftDeclSynthesizer.cpp

@@ -1681,7 +1681,6 @@ 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.
@@ -1764,23 +1763,6 @@ 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 *
@@ -1792,7 +1774,6 @@ 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();
@@ -1803,9 +1784,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 || resultDependsOnSelf);
+  bool isImplicit = !isNoncopyable;
   bool useAddress =
-      rawElementTy->getAnyPointerElementType() && (isNoncopyable || resultDependsOnSelf);
+      rawElementTy->getAnyPointerElementType() && isNoncopyable;
 
   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 %{{[0-9]+}}([[ACCESS]], %{{[0-9]+}}) : $@convention(cxx_method) (@inout NonTrivialInWrapper, @in_guaranteed HasMethods) -> ()
+  // CHECK: %{{[0-9]+}} = apply %32([[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

@@ -1,22 +0,0 @@
-#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,9 +81,3 @@ 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,19 +66,4 @@ 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

@@ -1,41 +0,0 @@
-// 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()