[libc++] Remove <tuple> include from <__format/concepts.h> #80214

philnik777 · 2024-01-31T23:20:40Z

This also moves tuple_size_v into tuple_size as a drive-by.

llvmbot · 2024-01-31T23:21:08Z

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

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

5 Files Affected:

(modified) libcxx/include/__format/concepts.h (+3-1)
(modified) libcxx/include/__tuple/tuple_size.h (+6-1)
(modified) libcxx/include/tuple (-3)
(modified) libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_v.verify.cpp (+1-1)
(modified) libcxx/test/tools/clang_tidy_checks/CMakeLists.txt (+1-1)

diff --git a/libcxx/include/__format/concepts.h b/libcxx/include/__format/concepts.h
index 299c5f40ee35b..37181d837ebf2 100644
--- a/libcxx/include/__format/concepts.h
+++ b/libcxx/include/__format/concepts.h
@@ -15,10 +15,12 @@
 #include <__config>
 #include <__format/format_fwd.h>
 #include <__format/format_parse_context.h>
+#include <__fwd/tuple.h>
+#include <__tuple/tuple_size.h>
 #include <__type_traits/is_specialization.h>
 #include <__type_traits/remove_const.h>
+#include <__type_traits/remove_reference.h>
 #include <__utility/pair.h>
-#include <tuple>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
diff --git a/libcxx/include/__tuple/tuple_size.h b/libcxx/include/__tuple/tuple_size.h
index b8320106fb269..18a17fd4d5878 100644
--- a/libcxx/include/__tuple/tuple_size.h
+++ b/libcxx/include/__tuple/tuple_size.h
@@ -43,7 +43,7 @@ struct _LIBCPP_TEMPLATE_VIS tuple_size<__enable_if_tuple_size_imp< volatile _Tp,
 
 template <class _Tp>
 struct _LIBCPP_TEMPLATE_VIS
-    tuple_size<__enable_if_tuple_size_imp< const volatile _Tp, integral_constant<size_t, sizeof(tuple_size<_Tp>)>>>
+tuple_size<__enable_if_tuple_size_imp<const volatile _Tp, integral_constant<size_t, sizeof(tuple_size<_Tp>)>>>
     : public integral_constant<size_t, tuple_size<_Tp>::value> {};
 
 #else
@@ -63,6 +63,11 @@ struct _LIBCPP_TEMPLATE_VIS tuple_size<tuple<_Tp...> > : public integral_constan
 template <class... _Tp>
 struct _LIBCPP_TEMPLATE_VIS tuple_size<__tuple_types<_Tp...> > : public integral_constant<size_t, sizeof...(_Tp)> {};
 
+#  if _LIBCPP_STD_VER >= 17
+template <class _Tp>
+inline constexpr size_t tuple_size_v = tuple_size<_Tp>::value;
+#  endif
+
 #endif // _LIBCPP_CXX03_LANG
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index 0e5f0b4831b41..a293f978c9f6f 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -1410,9 +1410,6 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<_T1, _T2>::pair(
       second(std::forward<_Args2>(std::get<_I2>(__second_args))...) {}
 
 #  if _LIBCPP_STD_VER >= 17
-template <class _Tp>
-inline constexpr size_t tuple_size_v = tuple_size<_Tp>::value;
-
 #    define _LIBCPP_NOEXCEPT_RETURN(...)                                                                               \
       noexcept(noexcept(__VA_ARGS__)) { return __VA_ARGS__; }
 
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_v.verify.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_v.verify.cpp
index 7570230b4b074..573d973b706c2 100644
--- a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_v.verify.cpp
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_v.verify.cpp
@@ -20,5 +20,5 @@ void f() {
     (void)std::tuple_size_v<std::tuple<> &>; // expected-note {{requested here}}
     (void)std::tuple_size_v<int>; // expected-note {{requested here}}
     (void)std::tuple_size_v<std::tuple<>*>; // expected-note {{requested here}}
-    // expected-error@tuple:* 3 {{implicit instantiation of undefined template}}
+    // expected-error@*:* 3 {{implicit instantiation of undefined template}}
 }
diff --git a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
index 978e709521652..ab448061f52dc 100644
--- a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
+++ b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
@@ -5,7 +5,7 @@
 set(LLVM_DIR_SAVE ${LLVM_DIR})
 set(Clang_DIR_SAVE ${Clang_DIR})
 
-find_package(Clang 18)
+find_package(Clang 19)
 if (NOT Clang_FOUND)
   find_package(Clang 17)
 endif()

github-actions · 2024-01-31T23:23:07Z

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

mordante · 2024-02-04T14:49:56Z

libcxx/test/libcxx/transitive_includes/cxx23.csv

@@ -86,7 +86,6 @@ chrono sstream
 chrono stdexcept
 chrono string
 chrono string_view
-chrono tuple


AFAIK we wanted to keep our transitive set stable. Maybe we should discuss this policy during a monthly meeting. Based on the several remarks on the size by the Chromium team and recently #80196 I think we should revise our header stability policy or at least have a plan how to remove transitive includes.

@vitaut recently blogged about this too, and _LIBCPP_REMOVE_TRANSITIVE_INCLUDES does not seem to be well known.

@ldionne can you put this on the agenda?

Until now we've only kept the includes up to C++20 stable. C++23 was always fair game. I agree that we should rethink our policy on this, since the amount of transitive includes that we could avoid has become massive. I'm currently also working on a patch to reduce the transitive includes further by including things in the top level headers by version.

Until now we've only kept the includes up to C++20 stable. C++23 was always fair game. I agree that we should rethink our policy on this, since the amount of transitive includes that we could avoid has become massive.

I know, but at some point it was said only the C++ development version is fair game, which would mean C++26. Hence the proposal to discuss this. I was already considering to put it on the agenda after a private conversation with Victor.

I'm currently also working on a patch to reduce the transitive includes further by including things in the top level headers by version.

The last time I had a patch to improve the transitive includes by #ifdef-ing it got a lukewarm response. So nice to hear the response now is better.

Based on the last monthly meeting we didn't declare C++23 includes frozen.

mordante · 2024-02-04T14:52:52Z

libcxx/include/vector

@@ -2972,6 +2972,7 @@ _LIBCPP_POP_MACROS
 #  include <atomic>
 #  include <concepts>
 #  include <cstdlib>
+#  include <tuple>


I added the tuple dependency for C++20 and now we need to keep it for C++11 - C++17 where tuple was never used. IMO a good reason to discuss our policy.

mordante

LGTM! modulo a nit.

mordante · 2024-03-11T16:56:05Z

libcxx/include/__tuple/tuple_size.h

+#  if _LIBCPP_STD_VER >= 17
+template <class _Tp>
+inline constexpr size_t tuple_size_v = tuple_size<_Tp>::value;
+#  endif
+


Can you mention this in the description?

philnik777 requested a review from a team as a code owner January 31, 2024 23:20

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 31, 2024

philnik777 force-pushed the remove_tuple_include branch 3 times, most recently from 8d45507 to e10abb8 Compare February 1, 2024 11:47

mordante reviewed Feb 4, 2024

View reviewed changes

philnik777 force-pushed the remove_tuple_include branch 3 times, most recently from d34138a to 9e46767 Compare March 9, 2024 15:16

[libc++] Remove <tuple> include from <__format/concepts.h>

7e1a4ed

philnik777 force-pushed the remove_tuple_include branch from 9e46767 to 7e1a4ed Compare March 11, 2024 12:49

mordante approved these changes Mar 11, 2024

View reviewed changes

philnik777 merged commit 4528c44 into llvm:main Mar 14, 2024

philnik777 deleted the remove_tuple_include branch March 14, 2024 11:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++] Remove <tuple> include from <__format/concepts.h> #80214

[libc++] Remove <tuple> include from <__format/concepts.h> #80214

Uh oh!

philnik777 commented Jan 31, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jan 31, 2024

Uh oh!

github-actions bot commented Jan 31, 2024 •

edited

Loading

Uh oh!

mordante Feb 4, 2024

Uh oh!

philnik777 Feb 4, 2024

Uh oh!

mordante Feb 4, 2024

Uh oh!

mordante Mar 11, 2024

Uh oh!

mordante Feb 4, 2024

Uh oh!

mordante left a comment

Uh oh!

mordante Mar 11, 2024

Uh oh!

Uh oh!

[libc++] Remove <tuple> include from <__format/concepts.h> #80214

[libc++] Remove <tuple> include from <__format/concepts.h> #80214

Uh oh!

Conversation

philnik777 commented Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 31, 2024

Uh oh!

github-actions bot commented Jan 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mordante Feb 4, 2024

Choose a reason for hiding this comment

Uh oh!

philnik777 Feb 4, 2024

Choose a reason for hiding this comment

Uh oh!

mordante Feb 4, 2024

Choose a reason for hiding this comment

Uh oh!

mordante Mar 11, 2024

Choose a reason for hiding this comment

Uh oh!

mordante Feb 4, 2024

Choose a reason for hiding this comment

Uh oh!

mordante left a comment

Choose a reason for hiding this comment

Uh oh!

mordante Mar 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

philnik777 commented Jan 31, 2024 •

edited

Loading

github-actions bot commented Jan 31, 2024 •

edited

Loading