Merge pull request #79325 from swiftlang/susmonteiro/copy-constructor-default-args

[cxx-interop] Prevent usage in Swift of C++ copy constructor with default args

Author: susmonteiro

lib/ClangImporter/ClangImporter.cpp

@@ -8006,6 +8006,14 @@ static bool isSufficientlyTrivial(const clang::CXXRecordDecl *decl) {
   return true;
 }
 
+static bool hasNonFirstDefaultArg(const clang::CXXConstructorDecl *ctor) {
+  if (ctor->getNumParams() < 2)
+    return false;
+
+  auto lastParam = ctor->parameters().back();
+  return lastParam->hasDefaultArg();
+}
+
 /// Checks if a record provides the required value type lifetime operations
 /// (copy and destroy).
 static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
@@ -8022,6 +8030,7 @@ static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
   // struct.
   return llvm::any_of(decl->ctors(), [](clang::CXXConstructorDecl *ctor) {
     return ctor->isCopyConstructor() && !ctor->isDeleted() &&
+           !hasNonFirstDefaultArg(ctor) &&
            ctor->getAccess() == clang::AccessSpecifier::AS_public;
   });
 }

lib/IRGen/GenStruct.cpp

@@ -541,12 +541,20 @@ namespace {
                                   FixedTypeInfo, ClangFieldInfo> {
     const clang::RecordDecl *ClangDecl;
 
+    bool hasNonFirstDefaultArg(const clang::CXXConstructorDecl *ctor) const {
+      if (ctor->getNumParams() < 2)
+        return false;
+
+      auto lastParam = ctor->parameters().back();
+      return lastParam->hasDefaultArg();
+    }
+
     const clang::CXXConstructorDecl *findCopyConstructor() const {
       const auto *cxxRecordDecl = dyn_cast<clang::CXXRecordDecl>(ClangDecl);
       if (!cxxRecordDecl)
         return nullptr;
       for (auto ctor : cxxRecordDecl->ctors()) {
-        if (ctor->isCopyConstructor() &&
+        if (ctor->isCopyConstructor() && !hasNonFirstDefaultArg(ctor) &&
             ctor->getAccess() == clang::AS_public && !ctor->isDeleted())
           return ctor;
       }

lib/SILOptimizer/Transforms/TempRValueElimination.cpp

@@ -640,8 +640,6 @@ void TempRValueOptPass::tryOptimizeCopyIntoTemp(CopyAddrInst *copyInst) {
         // This copy_addr [take] will perform the final deinitialization.
         return false;
       }
-      assert(!tempObj->getType().isMoveOnly() &&
-             "introducing copy of move-only value!?");
       return true;
     }
     if (auto *li = dyn_cast<LoadInst>(lastLoadInst)) {
@@ -650,8 +648,6 @@ void TempRValueOptPass::tryOptimizeCopyIntoTemp(CopyAddrInst *copyInst) {
         // This load [take] will perform the final deinitialization.
         return false;
       }
-      assert(!tempObj->getType().isMoveOnly() &&
-             "introducing copy of move-only value!?");
       return true;
     }
     return true;

test/Interop/Cxx/class/Inputs/type-classification.h

@@ -262,4 +262,16 @@ struct __attribute__((swift_attr("~Copyable"))) StructCopyableMovableAnnotatedNo
   ~StructCopyableMovableAnnotatedNonCopyable() = default;
 };
 
+struct HasCopyConstructorWithDefaultArgs {
+  int value;
+  HasCopyConstructorWithDefaultArgs(int value) : value(value) {}
+
+  HasCopyConstructorWithDefaultArgs(
+      const HasCopyConstructorWithDefaultArgs &other, int value = 1)
+      : value(other.value + value) {}
+
+  HasCopyConstructorWithDefaultArgs(HasCopyConstructorWithDefaultArgs &&) =
+      default;
+};
+
 #endif // TEST_INTEROP_CXX_CLASS_INPUTS_TYPE_CLASSIFICATION_H

test/Interop/Cxx/class/type-classification-typechecker.swift

@@ -22,6 +22,13 @@ func testAnnotated() {
   _ = v
 }
 
+func testNonCopyable() {
+  let x = HasCopyConstructorWithDefaultArgs(5)
+  let v = copy x // expected-error {{'copy' cannot be applied to noncopyable types}}
+  _ = v
+}
+
 test()
 testField()
 testAnnotated()
+testNonCopyable()

test/Interop/Cxx/value-witness-table/Inputs/copy-constructors.h

@@ -23,6 +23,30 @@ struct HasNonTrivialDefaultCopyConstructor {
       const HasNonTrivialDefaultCopyConstructor &) = default;
 };
 
+struct HasCopyConstructorWithDefaultArgs {
+  int value;
+  HasCopyConstructorWithDefaultArgs(int value) : value(value) {}
+
+  HasCopyConstructorWithDefaultArgs(
+      const HasCopyConstructorWithDefaultArgs &other, int value = 1)
+      : value(other.value + value) {}
+
+  HasCopyConstructorWithDefaultArgs(HasCopyConstructorWithDefaultArgs &&) =
+      default;
+};
+
+struct HasCopyConstructorWithOneParameterWithDefaultArg {
+  int numCopies;
+
+  HasCopyConstructorWithOneParameterWithDefaultArg(int numCopies)
+      : numCopies(numCopies) {}
+
+  HasCopyConstructorWithOneParameterWithDefaultArg(
+      const HasCopyConstructorWithOneParameterWithDefaultArg &other =
+          HasCopyConstructorWithOneParameterWithDefaultArg{1})
+      : numCopies(other.numCopies + 1) {}
+};
+
 // Make sure that we don't crash on struct templates with copy-constructors.
 template <typename T> struct S {
   S(S const &) {}

test/Interop/Cxx/value-witness-table/copy-constructors-execution.swift

@@ -48,4 +48,22 @@ CXXCopyConstructorTestSuite.test("Default copy constructor, member with user-def
   expectTrue(result.0.box.numCopies + result.1.box.numCopies > 0)
 }
 
+CXXCopyConstructorTestSuite.test("Copy constructor with default arguments") {
+  // When in the presence of a C++ copy constructor with default args, we make the type non-copyable
+  let originalObj = HasCopyConstructorWithDefaultArgs(5)
+  expectEqual(originalObj.value, 5)
+
+  // move originalObj
+  let newObj = originalObj
+  expectEqual(newObj.value, 5)
+}
+
+CXXCopyConstructorTestSuite.test("Copy constructor with one parameter that has a default argument") {
+  // If the C++ copy constructor has exactly one param and it has a default argument, ignore the default argument and import the type as copyable to Swift
+
+  let original = HasCopyConstructorWithOneParameterWithDefaultArg(1)
+  let copy = original
+  expectTrue(original.numCopies + copy.numCopies > 1)
+}
+
 runAllTests()