From b459fb5fc20eee7081c107f71f8ffc50d9d5ba08 Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Mon, 12 Jun 2023 16:22:29 +0100 Subject: [PATCH] [cxx-interop] Avoid linker errors when calling a defaulted constructor When a default constructor is declared, but does not have a body because it is defaulted (`= default;`), Swift did not emit the IR for it. This was causing linker error for types such as `std::map` in libstdc++ when someone tried to initialize such types from Swift. rdar://110638499 / resolves https://github.com/apple/swift/issues/61412 --- lib/ClangImporter/ImportDecl.cpp | 46 ++++++++++++------- test/Interop/Cxx/class/Inputs/constructors.h | 5 ++ .../Cxx/class/constructors-executable.swift | 6 +++ .../class/constructors-module-interface.swift | 5 ++ test/Interop/Cxx/stdlib/use-std-map.swift | 2 - 5 files changed, 45 insertions(+), 19 deletions(-) diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 88d2a9a93dd3e..48ef2808303fd 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -2635,30 +2635,38 @@ namespace { } clang::CXXConstructorDecl *copyCtor = nullptr; clang::CXXConstructorDecl *moveCtor = nullptr; + clang::CXXConstructorDecl *defaultCtor = nullptr; if (decl->needsImplicitCopyConstructor()) { copyCtor = clangSema.DeclareImplicitCopyConstructor( const_cast(decl)); - } else if (decl->needsImplicitMoveConstructor()) { + } + if (decl->needsImplicitMoveConstructor()) { moveCtor = clangSema.DeclareImplicitMoveConstructor( const_cast(decl)); - } else { - // We may have a defaulted copy constructor that needs to be defined. - // Try to find it. - for (auto methods : decl->methods()) { - if (auto declCtor = dyn_cast(methods)) { - if (declCtor->isDefaulted() && - declCtor->getAccess() == clang::AS_public && - !declCtor->isDeleted() && - // Note: we use "doesThisDeclarationHaveABody" here because - // that's what "DefineImplicitCopyConstructor" checks. - !declCtor->doesThisDeclarationHaveABody()) { - if (declCtor->isCopyConstructor()) { + } + if (decl->needsImplicitDefaultConstructor()) { + defaultCtor = clangSema.DeclareImplicitDefaultConstructor( + const_cast(decl)); + } + // We may have a defaulted copy/move/default constructor that needs to + // be defined. Try to find it. + for (auto methods : decl->methods()) { + if (auto declCtor = dyn_cast(methods)) { + if (declCtor->isDefaulted() && + declCtor->getAccess() == clang::AS_public && + !declCtor->isDeleted() && + // Note: we use "doesThisDeclarationHaveABody" here because + // that's what "DefineImplicitCopyConstructor" checks. + !declCtor->doesThisDeclarationHaveABody()) { + if (declCtor->isCopyConstructor()) { + if (!copyCtor) copyCtor = declCtor; - break; - } else if (declCtor->isMoveConstructor()) { + } else if (declCtor->isMoveConstructor()) { + if (!moveCtor) moveCtor = declCtor; - break; - } + } else if (declCtor->isDefaultConstructor()) { + if (!defaultCtor) + defaultCtor = declCtor; } } } @@ -2671,6 +2679,10 @@ namespace { clangSema.DefineImplicitMoveConstructor(clang::SourceLocation(), moveCtor); } + if (defaultCtor) { + clangSema.DefineImplicitDefaultConstructor(clang::SourceLocation(), + defaultCtor); + } if (decl->needsImplicitDestructor()) { auto dtor = clangSema.DeclareImplicitDestructor( diff --git a/test/Interop/Cxx/class/Inputs/constructors.h b/test/Interop/Cxx/class/Inputs/constructors.h index 4421391fae127..7493fc91cf785 100644 --- a/test/Interop/Cxx/class/Inputs/constructors.h +++ b/test/Interop/Cxx/class/Inputs/constructors.h @@ -10,6 +10,11 @@ struct ImplicitDefaultConstructor { int x = 42; }; +struct DefaultedDefaultConstructor { + int x = 42; + DefaultedDefaultConstructor() = default; +}; + struct MemberOfClassType { ImplicitDefaultConstructor member; }; diff --git a/test/Interop/Cxx/class/constructors-executable.swift b/test/Interop/Cxx/class/constructors-executable.swift index 9a8eb852d46d2..1efbb2556b6cc 100644 --- a/test/Interop/Cxx/class/constructors-executable.swift +++ b/test/Interop/Cxx/class/constructors-executable.swift @@ -19,6 +19,12 @@ CxxConstructorTestSuite.test("ImplicitDefaultConstructor") { expectEqual(42, instance.x) } +CxxConstructorTestSuite.test("DefaultedDefaultConstructor") { + let instance = DefaultedDefaultConstructor() + + expectEqual(42, instance.x) +} + CxxConstructorTestSuite.test("MemberOfClassType") { let instance = MemberOfClassType() diff --git a/test/Interop/Cxx/class/constructors-module-interface.swift b/test/Interop/Cxx/class/constructors-module-interface.swift index fab9918de6771..5f39ea6e8c1aa 100644 --- a/test/Interop/Cxx/class/constructors-module-interface.swift +++ b/test/Interop/Cxx/class/constructors-module-interface.swift @@ -9,6 +9,11 @@ // CHECK-NEXT: init(x: Int32) // CHECK-NEXT: var x: Int32 // CHECK-NEXT: } +// CHECK-NEXT: struct DefaultedDefaultConstructor { +// CHECK-NEXT: init() +// CHECK-NEXT: init(x: Int32) +// CHECK-NEXT: var x: Int32 +// CHECK-NEXT: } // CHECK-NEXT: struct MemberOfClassType { // CHECK-NEXT: init() // CHECK-NEXT: init(member: ImplicitDefaultConstructor) diff --git a/test/Interop/Cxx/stdlib/use-std-map.swift b/test/Interop/Cxx/stdlib/use-std-map.swift index 189ca92c2692d..d3480bbe22db7 100644 --- a/test/Interop/Cxx/stdlib/use-std-map.swift +++ b/test/Interop/Cxx/stdlib/use-std-map.swift @@ -11,13 +11,11 @@ import Cxx var StdMapTestSuite = TestSuite("StdMap") -#if !os(Linux) // https://github.com/apple/swift/issues/61412 StdMapTestSuite.test("init") { let m = Map() expectEqual(m.size(), 0) expectTrue(m.empty()) } -#endif StdMapTestSuite.test("Map.subscript") { // This relies on the `std::map` conformance to `CxxDictionary` protocol.