Skip to content

Conversation

philnik777
Copy link
Contributor

@philnik777 philnik777 commented Mar 18, 2024

This moves the definition of a pair constructor for <tuple> to <__utility/pair.h> and uses the forward declaration of pair in <tuple> instead of including the definition.

@philnik777 philnik777 force-pushed the tuple_remove_pair_dependency branch 2 times, most recently from baf7780 to b8b120a Compare March 25, 2024 07:44
Copy link

✅ With the latest revision this PR passed the Python code formatter.

Copy link

✅ With the latest revision this PR passed the C/C++ code formatter.

@philnik777 philnik777 marked this pull request as ready for review April 1, 2024 06:58
@philnik777 philnik777 requested a review from a team as a code owner April 1, 2024 06:58
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 1, 2024

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

This moves the definition of a pair constructor for &lt;tuple&gt; to &lt;__utility/pair.h&gt; and uses the forward declaration of pair in &lt;tuple&gt; instead of including the definiton.


Full diff: https://github.com/llvm/llvm-project/pull/85621.diff

9 Files Affected:

  • (modified) libcxx/include/__ranges/zip_view.h (+1)
  • (modified) libcxx/include/__utility/pair.h (+3-1)
  • (modified) libcxx/include/module.modulemap (+8-2)
  • (modified) libcxx/include/tuple (+1-12)
  • (modified) libcxx/test/std/ranges/range.adaptors/range.zip/borrowing.compile.pass.cpp (-1)
  • (modified) libcxx/test/std/ranges/range.utility/range.subrange/operator.pair_like.pass.cpp (+1)
  • (modified) libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_incomplete.pass.cpp (+2-1)
  • (modified) libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp (+2-2)
  • (modified) libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_value_sfinae.pass.cpp (+1-1)
diff --git a/libcxx/include/__ranges/zip_view.h b/libcxx/include/__ranges/zip_view.h
index ce00a4e53a489b..51001bf9b75798 100644
--- a/libcxx/include/__ranges/zip_view.h
+++ b/libcxx/include/__ranges/zip_view.h
@@ -36,6 +36,7 @@
 #include <__utility/forward.h>
 #include <__utility/integer_sequence.h>
 #include <__utility/move.h>
+#include <__utility/pair.h>
 #include <tuple>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
diff --git a/libcxx/include/__utility/pair.h b/libcxx/include/__utility/pair.h
index 0056806877e13c..bcb0397b374772 100644
--- a/libcxx/include/__utility/pair.h
+++ b/libcxx/include/__utility/pair.h
@@ -440,7 +440,9 @@ struct _LIBCPP_TEMPLATE_VIS pair
        tuple<_Args1...>& __first_args,
        tuple<_Args2...>& __second_args,
        __tuple_indices<_I1...>,
-       __tuple_indices<_I2...>);
+       __tuple_indices<_I2...>)
+      : first(std::forward<_Args1>(std::get<_I1>(__first_args))...),
+        second(std::forward<_Args2>(std::get<_I2>(__second_args))...) {}
 #endif
 };
 
diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index f36a47cef00977..068d0a12187a7d 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -1729,7 +1729,10 @@ module std_private_ranges_transform_view             [system] {
 }
 module std_private_ranges_view_interface             [system] { header "__ranges/view_interface.h" }
 module std_private_ranges_views                      [system] { header "__ranges/views.h" }
-module std_private_ranges_zip_view                   [system] { header "__ranges/zip_view.h" }
+module std_private_ranges_zip_view                   [system] {
+  header "__ranges/zip_view.h"
+  export std_private_utility_pair
+}
 
 module std_private_span_span_fwd [system] { header "__fwd/span.h" }
 
@@ -1809,7 +1812,10 @@ module std_private_tuple_sfinae_helpers   [system] { header "__tuple/sfinae_help
 module std_private_tuple_tuple_element    [system] { header "__tuple/tuple_element.h" }
 module std_private_tuple_tuple_fwd        [system] { header "__fwd/tuple.h" }
 module std_private_tuple_tuple_indices    [system] { header "__tuple/tuple_indices.h" }
-module std_private_tuple_tuple_like       [system] { header "__tuple/tuple_like.h" }
+module std_private_tuple_tuple_like       [system] {
+  header "__tuple/tuple_like.h"
+  export *
+}
 module std_private_tuple_tuple_like_ext   [system] { header "__tuple/tuple_like_ext.h" }
 module std_private_tuple_tuple_size       [system] { header "__tuple/tuple_size.h" }
 module std_private_tuple_tuple_types      [system] { header "__tuple/tuple_types.h" }
diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index f78db061b844b0..b6df25663909f9 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -210,6 +210,7 @@ template <class... Types>
 #include <__config>
 #include <__functional/invoke.h>
 #include <__fwd/array.h>
+#include <__fwd/pair.h>
 #include <__fwd/tuple.h>
 #include <__memory/allocator_arg_t.h>
 #include <__memory/uses_allocator.h>
@@ -250,7 +251,6 @@ template <class... Types>
 #include <__utility/forward.h>
 #include <__utility/integer_sequence.h>
 #include <__utility/move.h>
-#include <__utility/pair.h>
 #include <__utility/piecewise_construct.h>
 #include <__utility/swap.h>
 #include <cstddef>
@@ -1348,17 +1348,6 @@ tuple_cat(_Tuple0&& __t0, _Tuples&&... __tpls) {
 template <class... _Tp, class _Alloc>
 struct _LIBCPP_TEMPLATE_VIS uses_allocator<tuple<_Tp...>, _Alloc> : true_type {};
 
-template <class _T1, class _T2>
-template <class... _Args1, class... _Args2, size_t... _I1, size_t... _I2>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<_T1, _T2>::pair(
-    piecewise_construct_t,
-    tuple<_Args1...>& __first_args,
-    tuple<_Args2...>& __second_args,
-    __tuple_indices<_I1...>,
-    __tuple_indices<_I2...>)
-    : first(std::forward<_Args1>(std::get<_I1>(__first_args))...),
-      second(std::forward<_Args2>(std::get<_I2>(__second_args))...) {}
-
 #  if _LIBCPP_STD_VER >= 17
 #    define _LIBCPP_NOEXCEPT_RETURN(...)                                                                               \
       noexcept(noexcept(__VA_ARGS__)) { return __VA_ARGS__; }
diff --git a/libcxx/test/std/ranges/range.adaptors/range.zip/borrowing.compile.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.zip/borrowing.compile.pass.cpp
index 0ff51c24b7e96d..1bf1d85a2ad719 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.zip/borrowing.compile.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.zip/borrowing.compile.pass.cpp
@@ -13,7 +13,6 @@
 //      (enable_borrowed_range<Views> && ...);
 
 #include <ranges>
-#include <tuple>
 
 struct Borrowed : std::ranges::view_base {
   int* begin() const;
diff --git a/libcxx/test/std/ranges/range.utility/range.subrange/operator.pair_like.pass.cpp b/libcxx/test/std/ranges/range.utility/range.subrange/operator.pair_like.pass.cpp
index 2641a9ad94ea76..82f5dd801e51a1 100644
--- a/libcxx/test/std/ranges/range.utility/range.subrange/operator.pair_like.pass.cpp
+++ b/libcxx/test/std/ranges/range.utility/range.subrange/operator.pair_like.pass.cpp
@@ -17,6 +17,7 @@
 #include <cassert>
 #include <concepts>
 #include <ranges>
+#include <utility>
 
 #include "test_macros.h"
 
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_incomplete.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_incomplete.pass.cpp
index cb8d3e7bde5122..7585911e2e8d2c 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_incomplete.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_incomplete.pass.cpp
@@ -16,9 +16,10 @@
 
 // UNSUPPORTED: c++03
 
-#include <tuple>
 #include <array>
+#include <tuple>
 #include <type_traits>
+#include <utility>
 
 #include "test_macros.h"
 
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp
index 937d47a38792c6..2324da3ad5f3ca 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp
@@ -16,10 +16,10 @@
 
 // UNSUPPORTED: c++03, c++11, c++14
 
-#include <tuple>
 #include <array>
-#include <type_traits>
 #include <cassert>
+#include <tuple>
+#include <utility>
 
 #include "test_macros.h"
 
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_value_sfinae.pass.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_value_sfinae.pass.cpp
index 6ea92ea8563219..7574bd47e3b427 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_value_sfinae.pass.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_value_sfinae.pass.cpp
@@ -17,7 +17,7 @@
 // UNSUPPORTED: c++03
 
 #include <tuple>
-#include <type_traits>
+#include <utility>
 
 #include "test_macros.h"
 

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, I never noticed that constructor was defined in <tuple>.

@philnik777 philnik777 force-pushed the tuple_remove_pair_dependency branch from b8b120a to 027fc7b Compare April 2, 2024 17:37
@philnik777 philnik777 merged commit 239236b into llvm:main Apr 2, 2024
@philnik777 philnik777 deleted the tuple_remove_pair_dependency branch April 2, 2024 17:37
Copy link
Member

@EricWF EricWF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Historically, using forward declarations has come back to bite us. The same can be said about having overload sets spread over headers.

My spidey sense suggests this might trigger build breakages due to changes in symbol completeness. But let's try it and find out.

@EricWF
Copy link
Member

EricWF commented Apr 2, 2024

@philnik777 I'm not finding a pre-commit run that completed prior to the commit. Was there one?

@philnik777
Copy link
Contributor Author

Yes. I only resolved a merge conflict in the last push, so I landed without waiting for the CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants