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

Author: susmonteiro

lib/ClangImporter/ClangImporter.cpp

@@ -7851,6 +7851,14 @@ static bool isSufficientlyTrivial(const clang::CXXRecordDecl *decl) {
   return true;
 }
 
+static bool hasDefaultArgs(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) {
@@ -7867,6 +7875,7 @@ static bool hasCopyTypeOperations(const clang::CXXRecordDecl *decl) {
   // struct.
   return llvm::any_of(decl->ctors(), [](clang::CXXConstructorDecl *ctor) {
     return ctor->isCopyConstructor() && !ctor->isDeleted() &&
+           !hasDefaultArgs(ctor) &&
            ctor->getAccess() == clang::AccessSpecifier::AS_public;
   });
 }

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,23 @@ 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(0)
+  let copy = original
+  expectEqual(original.numCopies, 0)
+  expectEqual(copy.numCopies, 1)
+}
+
 runAllTests()