From 330fc0b07a3cd32f994bb26f033407fcddd42d80 Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Tue, 9 Aug 2022 05:20:58 -0700 Subject: [PATCH] [interop][SwiftToCxx] generic functions should return value types correctly --- lib/PrintAsClang/PrintClangFunction.cpp | 10 +++++++- lib/PrintAsClang/PrintClangValueType.cpp | 24 ++++++++++++------- .../SwiftShims/_SwiftCxxInteroperability.h | 7 ++++-- .../SwiftToCxx/class/swift-class-in-cxx.swift | 2 +- .../generics/generic-function-execution.cpp | 20 +++++++++++++--- .../generics/generic-function-in-cxx.swift | 13 ++++++++++ .../structs/swift-struct-in-cxx.swift | 8 ++++++- 7 files changed, 68 insertions(+), 16 deletions(-) diff --git a/lib/PrintAsClang/PrintClangFunction.cpp b/lib/PrintAsClang/PrintClangFunction.cpp index 52e8916aa31..8578fd5a876 100644 --- a/lib/PrintAsClang/PrintClangFunction.cpp +++ b/lib/PrintAsClang/PrintClangFunction.cpp @@ -576,10 +576,10 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody( if (!isKnownCxxType(resultTy, typeMapping) && !hasKnownOptionalNullableCxxMapping(resultTy)) { if (isGenericType(resultTy)) { - // FIXME: Support returning value types. std::string returnAddress; llvm::raw_string_ostream ros(returnAddress); ros << "reinterpret_cast(&returnValue)"; + StringRef resultTyName = "T"; // FIXME os << " if constexpr (std::is_base_of<::swift::" << cxx_synthesis::getCxxImplNamespaceName() @@ -589,6 +589,14 @@ void DeclAndTypeClangFunctionPrinter::printCxxThunkBody( os << ";\n"; os << " return ::swift::" << cxx_synthesis::getCxxImplNamespaceName() << "::implClassFor::type::makeRetained(returnValue);\n"; + os << " } else if constexpr (::swift::" + << cxx_synthesis::getCxxImplNamespaceName() << "::isValueType<" + << resultTyName << ">) {\n"; + os << " return ::swift::" << cxx_synthesis::getCxxImplNamespaceName() + << "::implClassFor<" << resultTyName + << ">::type::returnNewValue([&](void * _Nonnull returnValue) {\n"; + printCallToCFunc(/*additionalParam=*/StringRef("returnValue")); + os << ";\n });\n"; os << " } else {\n"; os << " T returnValue;\n"; printCallToCFunc(/*additionalParam=*/StringRef(ros.str())); diff --git a/lib/PrintAsClang/PrintClangValueType.cpp b/lib/PrintAsClang/PrintClangValueType.cpp index 8969ad5766a..03a810347b1 100644 --- a/lib/PrintAsClang/PrintClangValueType.cpp +++ b/lib/PrintAsClang/PrintClangValueType.cpp @@ -475,8 +475,6 @@ void ClangValueTypePrinter::printTypeGenericTraits( os << "::"; printer.printBaseName(typeDecl); os << "> = true;\n"; - os << "#pragma clang diagnostic pop\n"; - os << "template<>\n"; os << "inline void * _Nonnull getTypeMetadata<"; printer.printBaseName(typeDecl->getModuleContext()); @@ -488,8 +486,18 @@ void ClangValueTypePrinter::printTypeGenericTraits( os << "::" << cxx_synthesis::getCxxImplNamespaceName() << "::" << typeMetadataFuncName << "(0)._0;\n"; os << "}\n"; - if (isa(typeDecl)) { + os << "namespace " << cxx_synthesis::getCxxImplNamespaceName() << "{\n"; + + if (!isa(typeDecl)) { + os << "template<>\n"; + os << "static inline const constexpr bool isValueType<"; + printer.printBaseName(typeDecl->getModuleContext()); + os << "::"; + printer.printBaseName(typeDecl); + os << "> = true;\n"; + } + os << "template<>\n"; os << "struct implClassFor<"; printer.printBaseName(typeDecl->getModuleContext()); @@ -501,9 +509,9 @@ void ClangValueTypePrinter::printTypeGenericTraits( printCxxImplClassName(os, typeDecl); os << "; };\n"; os << "} // namespace\n"; - } - os << "} // namespace swift\n"; - os << "\nnamespace "; - printer.printBaseName(typeDecl->getModuleContext()); - os << " {\n"; + os << "#pragma clang diagnostic pop\n"; + os << "} // namespace swift\n"; + os << "\nnamespace "; + printer.printBaseName(typeDecl->getModuleContext()); + os << " {\n"; } diff --git a/stdlib/public/SwiftShims/_SwiftCxxInteroperability.h b/stdlib/public/SwiftShims/_SwiftCxxInteroperability.h index e731df27c36..41ca4e43814 100644 --- a/stdlib/public/SwiftShims/_SwiftCxxInteroperability.h +++ b/stdlib/public/SwiftShims/_SwiftCxxInteroperability.h @@ -104,8 +104,6 @@ using UInt = size_t; template static inline const constexpr bool isUsableInGenericContext = false; -#pragma clang diagnostic pop - /// Returns the type metadat for the given Swift type T. template inline void *_Nonnull getTypeMetadata(); @@ -117,8 +115,13 @@ template struct implClassFor { // using type = ...; }; +/// True if the given type is a Swift value type. +template static inline const constexpr bool isValueType = false; + } // namespace _impl +#pragma clang diagnostic pop + } // namespace swift #endif diff --git a/test/Interop/SwiftToCxx/class/swift-class-in-cxx.swift b/test/Interop/SwiftToCxx/class/swift-class-in-cxx.swift index b3649bb24b6..a0295fa25df 100644 --- a/test/Interop/SwiftToCxx/class/swift-class-in-cxx.swift +++ b/test/Interop/SwiftToCxx/class/swift-class-in-cxx.swift @@ -65,7 +65,6 @@ public final class ClassWithIntField { // CHECK-NEXT: #pragma clang diagnostic ignored "-Wc++17-extensions" // CHECK-NEXT: template<> // CHECK-NEXT: static inline const constexpr bool isUsableInGenericContext = true; -// CHECK-NEXT: #pragma clang diagnostic pop // CHECK-NEXT: template<> // CHECK-NEXT: inline void * _Nonnull getTypeMetadata() { // CHECK-NEXT: return Class::_impl::$s5Class0A12WithIntFieldCMa(0)._0; @@ -74,6 +73,7 @@ public final class ClassWithIntField { // CHECK-NEXT: template<> // CHECK-NEXT: struct implClassFor { using type = Class::_impl::_impl_ClassWithIntField; }; // CHECK-NEXT: } // namespace +// CHECK-NEXT: #pragma clang diagnostic pop // CHECK-NEXT: } // namespace swift // CHECK-EMPTY: // CHECK-NEXT: namespace Class { diff --git a/test/Interop/SwiftToCxx/generics/generic-function-execution.cpp b/test/Interop/SwiftToCxx/generics/generic-function-execution.cpp index ed39581b3bc..716b9957357 100644 --- a/test/Interop/SwiftToCxx/generics/generic-function-execution.cpp +++ b/test/Interop/SwiftToCxx/generics/generic-function-execution.cpp @@ -171,11 +171,19 @@ int main() { genericSwap(x, y); genericPrintFunction(x); genericPrintFunction(y); + auto xy = genericRet(x); + genericPrintFunction(xy); + xy.mut(); + genericPrintFunction(xy); + genericPrintFunction(x); } // CHECK-NEXT: TestLargeStruct value=TestLargeStruct(x1: 11, x2: 12, x3: 10, x4: 11, x5: 13, x6: 9) // CHECK-NEXT: TestLargeStruct value=TestLargeStruct(x1: -9, x2: -8, x3: -10, x4: -9, x5: -7, x6: -11) // CHECK-NEXT: TestLargeStruct value=TestLargeStruct(x1: -9, x2: -8, x3: -10, x4: -9, x5: -7, x6: -11) - // CHECK-NEXT: TestLargeStruct value=TestLargeStruct(x1: 11, x2: 12, x3: 10, x4: 11, x5: 13, x6: 9) +// CHECK-NEXT: TestLargeStruct value=TestLargeStruct(x1: 11, x2: 12, x3: 10, x4: 11, x5: 13, x6: 9) +// CHECK-NEXT: TestLargeStruct value=TestLargeStruct(x1: -9, x2: -8, x3: -10, x4: -9, x5: -7, x6: -11) +// CHECK-NEXT: TestLargeStruct value=TestLargeStruct(x1: 9, x2: -8, x3: -10, x4: -9, x5: -7, x6: -7) +// CHECK-NEXT: TestLargeStruct value=TestLargeStruct(x1: -9, x2: -8, x3: -10, x4: -9, x5: -7, x6: -11) { auto x = createTestSmallStruct(45); @@ -185,12 +193,18 @@ int main() { genericSwap(y, x); genericPrintFunction(x); genericPrintFunction(y); + auto xy = genericRet(x); + genericPrintFunction(xy); + xy.mut(); + genericPrintFunction(xy); + genericPrintFunction(x); } // CHECK-NEXT: TestSmallStruct value=TestSmallStruct(x1: 45) // CHECK-NEXT: TestSmallStruct value=TestSmallStruct(x1: 65233) // CHECK-NEXT: TestSmallStruct value=TestSmallStruct(x1: 65233) // CHECK-NEXT: TestSmallStruct value=TestSmallStruct(x1: 45) - -// FIXME: return struct. +// CHECK-NEXT: TestSmallStruct value=TestSmallStruct(x1: 65233) +// CHECK-NEXT: TestSmallStruct value=TestSmallStruct(x1: 4294902062) +// CHECK-NEXT: TestSmallStruct value=TestSmallStruct(x1: 65233) return 0; } diff --git a/test/Interop/SwiftToCxx/generics/generic-function-in-cxx.swift b/test/Interop/SwiftToCxx/generics/generic-function-in-cxx.swift index d7399e233e1..04a27920abd 100644 --- a/test/Interop/SwiftToCxx/generics/generic-function-in-cxx.swift +++ b/test/Interop/SwiftToCxx/generics/generic-function-in-cxx.swift @@ -51,6 +51,11 @@ public struct TestLargeStruct { x5 = x+2 x6 = x-2 } + + public mutating func mut() { + x1 = -x1 + x6 = x5 + } } public func createTestLargeStruct(_ x: Int) -> TestLargeStruct { @@ -59,6 +64,10 @@ public func createTestLargeStruct(_ x: Int) -> TestLargeStruct { public struct TestSmallStruct { var x1: UInt32 + + public mutating func mut() { + x1 = ~x1 + } } public func createTestSmallStruct(_ x: UInt32) -> TestSmallStruct { @@ -102,6 +111,10 @@ public func createTestSmallStruct(_ x: UInt32) -> TestSmallStruct { // CHECK-NEXT: void *returnValue; // CHECK-NEXT: _impl::$s9Functions10genericRetyxxlF(reinterpret_cast(&returnValue), reinterpret_cast(&x), swift::getTypeMetadata()); // CHECK-NEXT: return ::swift::_impl::implClassFor::type::makeRetained(returnValue); +// CHECK-NEXT: } else if constexpr (::swift::_impl::isValueType) { +// CHECK-NEXT: return ::swift::_impl::implClassFor::type::returnNewValue([&](void * _Nonnull returnValue) { +// CHECK-NEXT: _impl::$s9Functions10genericRetyxxlF(returnValue, reinterpret_cast(&x), swift::getTypeMetadata()); +// CHECK-NEXT: }); // CHECK-NEXT: } else { // CHECK-NEXT: T returnValue; // CHECK-NEXT: _impl::$s9Functions10genericRetyxxlF(reinterpret_cast(&returnValue), reinterpret_cast(&x), swift::getTypeMetadata()); diff --git a/test/Interop/SwiftToCxx/structs/swift-struct-in-cxx.swift b/test/Interop/SwiftToCxx/structs/swift-struct-in-cxx.swift index 92a414e628c..968ff81239d 100644 --- a/test/Interop/SwiftToCxx/structs/swift-struct-in-cxx.swift +++ b/test/Interop/SwiftToCxx/structs/swift-struct-in-cxx.swift @@ -69,11 +69,17 @@ // CHECK-NEXT: #pragma clang diagnostic ignored "-Wc++17-extensions" // CHECK-NEXT: template<> // CHECK-NEXT: static inline const constexpr bool isUsableInGenericContext = true; -// CHECK-NEXT: #pragma clang diagnostic pop // CHECK-NEXT: template<> // CHECK-NEXT: inline void * _Nonnull getTypeMetadata() { // CHECK-NEXT: return Structs::_impl::$s7Structs18StructWithIntFieldVMa(0)._0; // CHECK-NEXT: } +// CHECK-NEXT: namespace _impl{ +// CHECK-NEXT: template<> +// CHECK-NEXT: static inline const constexpr bool isValueType = true; +// CHECK-NEXT: template<> +// CHECK-NEXT: struct implClassFor { using type = Structs::_impl::_impl_StructWithIntField; }; +// CHECK-NEXT: } // namespace +// CHECK-NEXT: #pragma clang diagnostic pop // CHECK-NEXT: } // namespace swift // CHECK-EMPTY: // CHECK-NEXT: namespace Structs {