-
Notifications
You must be signed in to change notification settings - Fork 15.4k
Fix __split_buffer_pointer_layout compatibility with swift's C++ interop #170957
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-libcxx Author: None (junov-google) ChangesWhen the swift compiler generates a bridging module for importing C++ symbols, it instantiates all methods of the class template instances used by the symbols imported from C++. This is because it cannot determine which methods will be called from swift at that stage. This means that it may attempt to generate invalid instantiations (due to missing type traits) that would not be generated in a normal C++ build. The default constructor of When the swift compiler's C++ importer generates an instantiation of This change fixes the issue by adding a There are no tests included in this change because the Full diff: https://github.com/llvm/llvm-project/pull/170957.diff 1 Files Affected:
diff --git a/libcxx/include/__split_buffer b/libcxx/include/__split_buffer
index 1e05e4df8ba0f..5e4e265b10296 100644
--- a/libcxx/include/__split_buffer
+++ b/libcxx/include/__split_buffer
@@ -30,6 +30,9 @@
#include <__type_traits/integral_constant.h>
#include <__type_traits/is_nothrow_assignable.h>
#include <__type_traits/is_nothrow_constructible.h>
+#if _LIBCPP_STD_VER >= 20
+# include <__type_traits/is_reference.h>
+#endif
#include <__type_traits/is_swappable.h>
#include <__type_traits/is_trivially_destructible.h>
#include <__type_traits/is_trivially_relocatable.h>
@@ -68,8 +71,13 @@ protected:
public:
// Can't be defaulted due to _LIBCPP_COMPRESSED_PAIR not being an aggregate in C++03 and C++11.
- _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __split_buffer_pointer_layout() : __back_cap_(nullptr) {}
-
+ _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI __split_buffer_pointer_layout()
+#if _LIBCPP_STD_VER >= 20
+ // Prevents Swift compiler's C++ interop from implicitly instantiating this ctor when it's not supported.
+ requires (!is_reference_v<allocator_type>)
+#endif
+ : __back_cap_(nullptr) {}
+
_LIBCPP_CONSTEXPR_SINCE_CXX20
_LIBCPP_HIDE_FROM_ABI explicit __split_buffer_pointer_layout(const allocator_type& __alloc)
: __back_cap_(nullptr), __alloc_(__alloc) {}
|
When the swift compiler generates a bridging module for importing C++ symbols, it instantiates all methods of the class template instances used by the symbols imported from C++. This is because it cannot determine which methods will be called from swift at that stage. This means that it may attempt to generate invalid instantiations (due to missing type traits) that would not be generated in a normal C++ build. The default constructor of `__split_buffer_pointer_layout` has this issue because it does not initialize `__alloc_` which may or may not be a reference depending on the `allocator_type` tempalte parameter. The initialization of class members that are references is mandatory. `vector` uses a reference type for the allocator and `deque` does not. Therefore, only `deque` is allowed to use the default constructor of `__split_buffer_pointer_layout`. When the swift compiler's C++ importer generates an instantiation of `std::vector`, it will also attempt to instantiate the default constructor of `__split_buffer_pointer_layout`, which fails to compile. This change fixes the issue by adding a `requires` clause to suppress the instantiation of the problematic constructor whenever its instatiation would be invalid. There are no tests included in this change because the `requires` statement has no observable effect on pure C++ builds. Test coverage will be automatically assured downstream in swift-project once it updates its fork of libc++.
When the swift compiler generates a bridging module for importing C++ symbols, it instantiates all methods of the class template instances used by the symbols imported from C++. This is because it cannot determine which methods will be called from swift at that stage. This means that it may attempt to generate invalid instantiations (due to missing type traits) that would not be generated in a normal C++ build.
The default constructor of
__split_buffer_pointer_layouthas this issue because it does not initialize__alloc_which may or may not be a reference depending on theallocator_typetempalte parameter. The initialization of class members that are references is mandatory.vectoruses a reference type for the allocator anddequedoes not. Therefore, onlydequeis allowed to use the default constructor of__split_buffer_pointer_layout.When the swift compiler's C++ importer generates an instantiation of
std::vector, it will also attempt to instantiate the default constructor of__split_buffer_pointer_layout, which fails to compile.This change fixes the issue by adding a
requiresclause to suppress the instantiation of the problematic constructor whenever its instatiation would be invalid.There are no tests included in this change because the
requiresstatement has no observable effect on pure C++ builds. Test coverage will be automatically assured downstream in swift-project once it updates its fork of libc++.