From 2d0863aba26feb35076f841daeea80a675d4c5fb Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Mon, 8 Jan 2024 20:36:36 +0000 Subject: [PATCH 1/5] [cxx-interop] Initial tests for `std::function` usage This makes sure that `std::function` is imported consistently on supported platforms, and that it allows basic usage: calling a function with `callAsFunction`, initializing an empty function, and passing a function retrieved from C++ back to C++ as a parameter. rdar://103979602 --- .../Cxx/stdlib/Inputs/module.modulemap | 5 ++++ test/Interop/Cxx/stdlib/Inputs/std-function.h | 16 ++++++++++ .../Interop/Cxx/stdlib/use-std-function.swift | 29 +++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 test/Interop/Cxx/stdlib/Inputs/std-function.h create mode 100644 test/Interop/Cxx/stdlib/use-std-function.swift diff --git a/test/Interop/Cxx/stdlib/Inputs/module.modulemap b/test/Interop/Cxx/stdlib/Inputs/module.modulemap index 3092b15d76a42..8ee26b41b3451 100644 --- a/test/Interop/Cxx/stdlib/Inputs/module.modulemap +++ b/test/Interop/Cxx/stdlib/Inputs/module.modulemap @@ -33,3 +33,8 @@ module StdUniquePtr { header "std-unique-ptr.h" requires cplusplus } + +module StdFunction { + header "std-function.h" + requires cplusplus +} diff --git a/test/Interop/Cxx/stdlib/Inputs/std-function.h b/test/Interop/Cxx/stdlib/Inputs/std-function.h new file mode 100644 index 0000000000000..1486f3a3e20de --- /dev/null +++ b/test/Interop/Cxx/stdlib/Inputs/std-function.h @@ -0,0 +1,16 @@ +#ifndef TEST_INTEROP_CXX_STDLIB_INPUTS_STD_FUNCTION_H +#define TEST_INTEROP_CXX_STDLIB_INPUTS_STD_FUNCTION_H + +#include + +using FunctionIntToInt = std::function; + +FunctionIntToInt getIdentityFunction() { + return [](int x) { return x; }; +} + +bool isEmptyFunction(FunctionIntToInt f) { return !(bool)f; } + +int invokeFunction(FunctionIntToInt f, int x) { return f(x); } + +#endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_FUNCTION_H diff --git a/test/Interop/Cxx/stdlib/use-std-function.swift b/test/Interop/Cxx/stdlib/use-std-function.swift new file mode 100644 index 0000000000000..271a89b8f8982 --- /dev/null +++ b/test/Interop/Cxx/stdlib/use-std-function.swift @@ -0,0 +1,29 @@ +// RUN: %target-run-simple-swift(-I %S/Inputs -Xfrontend -enable-experimental-cxx-interop) +// RUN: %target-run-simple-swift(-I %S/Inputs -cxx-interoperability-mode=upcoming-swift) + +// REQUIRES: executable_test + +import StdlibUnittest +import StdFunction + +var StdFunctionTestSuite = TestSuite("StdFunction") + +StdFunctionTestSuite.test("init empty") { + let f = FunctionIntToInt() + expectTrue(isEmptyFunction(f)) + + let copied = f + expectTrue(isEmptyFunction(copied)) +} + +StdFunctionTestSuite.test("call") { + let f = getIdentityFunction() + expectEqual(123, f(123)) +} + +StdFunctionTestSuite.test("retrieve and pass back as parameter") { + let res = invokeFunction(getIdentityFunction(), 456) + expectEqual(456, res) +} + +runAllTests() From 4ca4b9a290485e23b7a53fa39fcb04596c01a194 Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Thu, 11 Jan 2024 20:53:45 +0000 Subject: [PATCH 2/5] [cxx-interop] Check for type completeness consistently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `clangSema.isCompleteType` checks for decl visibility according to the module visibility rules, which we don't actually need. What we need to check is whether the record has a definition – this is the check we already use elsewhere in ClangImporter. This makes sure that we can import `std::function` on Windows. rdar://103979602 --- lib/ClangImporter/ImportDecl.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 8b782a8750e79..cc33d2a20703d 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -2959,9 +2959,7 @@ namespace { if (notInstantiated) return nullptr; } - if (!clangSema.isCompleteType( - decl->getLocation(), - Impl.getClangASTContext().getRecordType(decl))) { + if (!decl->getDefinition()) { // If we got nullptr definition now it means the type is not complete. // We don't import incomplete types. return nullptr; From e9dd9109e743f0978a6cd944fe171825067d4ef7 Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Fri, 12 Jan 2024 16:21:43 +0000 Subject: [PATCH 3/5] [cxx-interop] Update the `std::pair` safety test The test was passing by accident: we didn't properly check `std::pair` type for completeness, and assumed that it is incomplete. This updates the test to actually verify the behavior that is specified in the vision document. --- test/Interop/Cxx/stdlib/Inputs/std-pair.h | 2 +- .../Interop/Cxx/stdlib/use-std-pair-typechecker.swift | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/test/Interop/Cxx/stdlib/Inputs/std-pair.h b/test/Interop/Cxx/stdlib/Inputs/std-pair.h index 8fc2604600e32..ff0c213b8a778 100644 --- a/test/Interop/Cxx/stdlib/Inputs/std-pair.h +++ b/test/Interop/Cxx/stdlib/Inputs/std-pair.h @@ -35,7 +35,7 @@ struct __attribute__((swift_attr("import_iterator"))) Iterator {}; using PairUnsafeStructInt = std::pair; using PairIteratorInt = std::pair; -struct HasMethodThatReturnsUnsafePair { +struct __attribute__((swift_attr("import_owned"))) HasMethodThatReturnsUnsafePair { PairUnsafeStructInt getUnsafePair() const { return {}; } PairIteratorInt getIteratorPair() const { return {}; } }; diff --git a/test/Interop/Cxx/stdlib/use-std-pair-typechecker.swift b/test/Interop/Cxx/stdlib/use-std-pair-typechecker.swift index c24bc9337d109..7c366c2afc884 100644 --- a/test/Interop/Cxx/stdlib/use-std-pair-typechecker.swift +++ b/test/Interop/Cxx/stdlib/use-std-pair-typechecker.swift @@ -1,7 +1,12 @@ -// RUN: %target-typecheck-verify-swift -I %S/Inputs -enable-experimental-cxx-interop +// RUN: not %target-swift-frontend %s -typecheck -I %S/Inputs -enable-experimental-cxx-interop 2>&1 | %FileCheck %s import StdPair let u = HasMethodThatReturnsUnsafePair() -u.getUnsafePair() // expected-error {{value of type 'HasMethodThatReturnsUnsafePair' has no member 'getUnsafePair'}} -u.getIteratorPair() // expected-error {{value of type 'HasMethodThatReturnsUnsafePair' has no member 'getIteratorPair'}} +u.getUnsafePair() +// CHECK: error: value of type 'HasMethodThatReturnsUnsafePair' has no member 'getUnsafePair' +// CHECK: note: C++ method 'getUnsafePair' may return an interior pointer + +u.getIteratorPair() +// CHECK: error: value of type 'HasMethodThatReturnsUnsafePair' has no member 'getIteratorPair' +// CHECK: note: C++ method 'getIteratorPair' may return an interior pointer From ad7ff59ce4e3eab6eca4829d54c8cfa823de64b1 Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Mon, 15 Jan 2024 16:14:54 +0000 Subject: [PATCH 4/5] [cxx-interop] Do not treat `std::pair` as an owned type on Linux In libstdc++ 11, `std::pair` has a base class `std::__pair_base`, which defines a copy constructor. We still continue not treating `std::pair` as an owned type, just like on other platforms. --- lib/ClangImporter/ClangImporter.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 3f422f48d5b87..cf05fcd56ccf4 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -7368,12 +7368,6 @@ static bool hasDestroyTypeOperations(const clang::CXXRecordDecl *decl) { } static bool hasCustomCopyOrMoveConstructor(const clang::CXXRecordDecl *decl) { - // std::pair and std::tuple might have copy and move constructors, but that - // doesn't mean they are safe to use from Swift, e.g. std::pair - if (decl->isInStdNamespace() && - (decl->getName() == "pair" || decl->getName() == "tuple")) { - return false; - } return decl->hasUserDeclaredCopyConstructor() || decl->hasUserDeclaredMoveConstructor(); } @@ -7489,6 +7483,13 @@ CxxRecordAsSwiftType::evaluate(Evaluator &evaluator, } bool anySubobjectsSelfContained(const clang::CXXRecordDecl *decl) { + // std::pair and std::tuple might have copy and move constructors, or base + // classes with copy and move constructors, but they are not self-contained + // types, e.g. `std::pair`. + if (decl->isInStdNamespace() && + (decl->getName() == "pair" || decl->getName() == "tuple")) + return false; + if (!decl->getDefinition()) return false; From a61d9333e933f34d42a14edf5087f0f1c5b67901 Mon Sep 17 00:00:00 2001 From: Egor Zhdan Date: Tue, 16 Jan 2024 18:07:29 +0000 Subject: [PATCH 5/5] [cxx-interop] Re-export C++ stdlib in tests that require it --- test/Interop/Cxx/stdlib/Inputs/module.modulemap | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/Interop/Cxx/stdlib/Inputs/module.modulemap b/test/Interop/Cxx/stdlib/Inputs/module.modulemap index 8ee26b41b3451..187111acca6a2 100644 --- a/test/Interop/Cxx/stdlib/Inputs/module.modulemap +++ b/test/Interop/Cxx/stdlib/Inputs/module.modulemap @@ -1,26 +1,31 @@ module StdVector { header "std-vector.h" requires cplusplus + export * } module StdMap { header "std-map.h" requires cplusplus + export * } module StdOptional { header "std-optional.h" requires cplusplus + export * } module StdSet { header "std-set.h" requires cplusplus + export * } module StdPair { header "std-pair.h" requires cplusplus + export * } module MsvcUseVecIt { @@ -32,9 +37,11 @@ module MsvcUseVecIt { module StdUniquePtr { header "std-unique-ptr.h" requires cplusplus + export * } module StdFunction { header "std-function.h" requires cplusplus + export * }