Skip to content

[libc++] Implement LWG3528 (make_from_tuple can perform (the equivalent of) a C-style cast)#85263

Merged
yronglin merged 16 commits intollvm:mainfrom
yronglin:LWG3528
Mar 22, 2024
Merged

[libc++] Implement LWG3528 (make_from_tuple can perform (the equivalent of) a C-style cast)#85263
yronglin merged 16 commits intollvm:mainfrom
yronglin:LWG3528

Conversation

@yronglin
Copy link
Contributor

@yronglin yronglin commented Mar 14, 2024

Implement LWG3528.
Based on LWG3528(https://wg21.link/LWG3528) and http://eel.is/c++draft/description#structure.requirements-9, the standard allows to impose requirements, we constraint std::make_from_tuple to make std::make_from_tuple SFINAE friendly and also avoid worse diagnostic messages. We still keep the constraints of std::__make_from_tuple_impl so that std::__make_from_tuple_impl will have the same advantages when used alone.

…lent of) a C-style cast)

Signed-off-by: yronglin <yronglin777@gmail.com>
@yronglin yronglin requested a review from a team as a code owner March 14, 2024 16:53
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 14, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 14, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-libcxx

Author: None (yronglin)

Changes

Implement LWG3528.


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

3 Files Affected:

  • (modified) libcxx/docs/Status/Cxx23Issues.csv (+1-1)
  • (modified) libcxx/include/tuple (+10-2)
  • (added) libcxx/test/std/utilities/tuple/tuple.tuple/tuple.apply/make_from_tuple.verify.cpp (+74)
diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index e00345533b865d..0fd58649b0cf64 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -77,7 +77,7 @@
 `3523 <https://wg21.link/LWG3523>`__,"``iota_view::sentinel`` is not always ``iota_view``'s sentinel","June 2021","","","|ranges|"
 `3526 <https://wg21.link/LWG3526>`__,"Return types of ``uses_allocator_construction_args`` unspecified","June 2021","",""
 `3527 <https://wg21.link/LWG3527>`__,"``uses_allocator_construction_args`` handles rvalue pairs of rvalue references incorrectly","June 2021","",""
-`3528 <https://wg21.link/LWG3528>`__,"``make_from_tuple`` can perform (the equivalent of) a C-style cast","June 2021","",""
+`3528 <https://wg21.link/LWG3528>`__,"``make_from_tuple`` can perform (the equivalent of) a C-style cast","June 2021","|Complete|","19.0"
 `3529 <https://wg21.link/LWG3529>`__,"``priority_queue(first, last)`` should construct ``c`` with ``(first, last)``","June 2021","|Complete|","14.0"
 `3530 <https://wg21.link/LWG3530>`__,"``BUILTIN-PTR-MEOW`` should not opt the type out of syntactic checks","June 2021","",""
 `3532 <https://wg21.link/LWG3532>`__,"``split_view<V, P>::inner-iterator<true>::operator++(int)`` should depend on ``Base``","June 2021","","","|ranges|"
diff --git a/libcxx/include/tuple b/libcxx/include/tuple
index 8808db6739fb9b..1ef546c5b2153d 100644
--- a/libcxx/include/tuple
+++ b/libcxx/include/tuple
@@ -1423,8 +1423,16 @@ inline _LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) apply(_Fn&& __f, _Tuple&&
         typename __make_tuple_indices<tuple_size_v<remove_reference_t<_Tuple>>>::type{}))
 
 template <class _Tp, class _Tuple, size_t... _Idx>
-inline _LIBCPP_HIDE_FROM_ABI constexpr _Tp __make_from_tuple_impl(_Tuple&& __t, __tuple_indices<_Idx...>)
-    _LIBCPP_NOEXCEPT_RETURN(_Tp(std::get<_Idx>(std::forward<_Tuple>(__t))...))
+inline _LIBCPP_HIDE_FROM_ABI constexpr _Tp __make_from_tuple_impl(_Tuple&& __t, __tuple_indices<_Idx...>) 
+  noexcept(noexcept(_Tp(std::get<_Idx>(std::forward<_Tuple>(__t))...)))
+#if _LIBCPP_STD_VER >= 20
+  requires is_constructible_v<_Tp, decltype(std::get<_Idx>(std::forward<_Tuple>(__t)))...>
+#endif
+{
+  static_assert(is_constructible_v<_Tp, decltype(std::get<_Idx>(std::forward<_Tuple>(__t)))...>, 
+                "Cannot constructible target type from the fields of the argument tuple.");
+  return _Tp(std::get<_Idx>(std::forward<_Tuple>(__t))...);
+}
 
 template <class _Tp, class _Tuple>
 inline _LIBCPP_HIDE_FROM_ABI constexpr _Tp make_from_tuple(_Tuple&& __t)
diff --git a/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.apply/make_from_tuple.verify.cpp b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.apply/make_from_tuple.verify.cpp
new file mode 100644
index 00000000000000..9bdca4dc7813cb
--- /dev/null
+++ b/libcxx/test/std/utilities/tuple/tuple.tuple/tuple.apply/make_from_tuple.verify.cpp
@@ -0,0 +1,74 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14
+
+// <tuple>
+
+// template <class T, class Tuple> constexpr T make_from_tuple(Tuple&&);
+
+#include <tuple>
+
+int main() {
+  // clang-format off
+#if _LIBCPP_STD_VER <= 20
+  // reinterpret_cast
+  {
+    struct B { int b; } b;
+    std::tuple<B *> t{&b};
+    auto a = std::make_from_tuple<int *>(t); // expected-error-re@*:* {{static assertion failed {{.*}}Cannot constructible target type from the fields of the argument tuple.}}
+    (void)a;
+  }
+
+  // const_cast
+  {
+    const char *str = "Hello";
+    std::tuple<const char *> t{str};
+    auto a = std::make_from_tuple<char *>(t); // expected-error-re@*:* {{static assertion failed {{.*}}Cannot constructible target type from the fields of the argument tuple.}}
+    (void)a;
+  }
+
+  // static_cast
+  {
+    struct B {};
+    struct C : public B {} c;
+    B &br = c;
+    std::tuple<const B&> t{br};
+    auto a = std::make_from_tuple<const C&>(t); // expected-error-re@*:* {{static assertion failed {{.*}}Cannot constructible target type from the fields of the argument tuple.}}
+    (void)a;
+  }
+#else
+  // reinterpret_cast
+  {
+    struct B { int b; } b;
+    std::tuple<B *> t{&b};
+    auto a = std::make_from_tuple<int *>(t); // expected-error-re@*:*2 {{no matching function for call to{{.*}}}}
+    (void)a;
+  }
+
+  // const_cast
+  {
+    const char *str = "Hello";
+    std::tuple<const char *> t{str};
+    auto a = std::make_from_tuple<char *>(t); // expected-error-re@*:*2 {{no matching function for call to{{.*}}}}
+    (void)a;
+  }
+
+  // static_cast
+  {
+    struct B {};
+    struct C : public B {} c;
+    B &br = c;
+    std::tuple<const B &> t{br};
+    auto a = std::make_from_tuple<const C&>(t); // expected-error-re@*:*2 {{no matching function for call to{{.*}}}}
+    (void)a;
+  }
+#endif
+  // clang-format on
+  return 0;
+}

Signed-off-by: yronglin <yronglin777@gmail.com>
Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! I've some comments regarding the design prior to C++20.

@mordante mordante self-assigned this Mar 14, 2024
…load set

Signed-off-by: yronglin <yronglin777@gmail.com>
@github-actions
Copy link

github-actions bot commented Mar 19, 2024

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

Signed-off-by: yronglin <yronglin777@gmail.com>
Signed-off-by: yronglin <yronglin777@gmail.com>
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Mar 19, 2024
@yronglin
Copy link
Contributor Author

Does this is a bug of GCC? https://godbolt.org/z/h5af6f5x9

This reverts commit d468378.
Signed-off-by: yronglin <yronglin777@gmail.com>
@yronglin yronglin removed the clang Clang issues not falling into any other category label Mar 19, 2024
@yronglin
Copy link
Contributor Author

Does this is a bug of GCC? https://godbolt.org/z/h5af6f5x9

I have reported an issue for GCC https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114395

Comment on lines +219 to +220
// const_cast
static_assert(!can_make_from_tuple<char*, std::tuple<const char*>>::value);
Copy link
Member

Choose a reason for hiding this comment

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

Can you test the const_cast with volatile and const volatile?
I also like to see some tests that valid cases are accepted; then we know can_make_from_tuple doesn't just reject everything.

Copy link
Contributor Author

@yronglin yronglin Mar 20, 2024

Choose a reason for hiding this comment

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

Thanks for your review! Added more test now and sorry, somehow I missed this before.

Signed-off-by: yronglin <yronglin777@gmail.com>
Signed-off-by: yronglin <yronglin777@gmail.com>
Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks a lot! I have one minor comment after that I'm happy. I'd like to have a quick look at your fix.

Signed-off-by: yronglin <yronglin777@gmail.com>
Signed-off-by: yronglin <yronglin777@gmail.com>
Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks LGTM!

@yronglin
Copy link
Contributor Author

Thanks LGTM!

Thanks for your review!

@yronglin yronglin merged commit 31a9a4b into llvm:main Mar 22, 2024
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
…lent of) a C-style cast) (llvm#85263)

Implement [LWG3528](https://wg21.link/LWG3528).
Based on LWG3528(https://wg21.link/LWG3528) and
http://eel.is/c++draft/description#structure.requirements-9, the
standard allows to impose requirements, we constraint
`std::make_from_tuple` to make `std::make_from_tuple` SFINAE friendly
and also avoid worse diagnostic messages. We still keep the constraints
of `std::__make_from_tuple_impl` so that `std::__make_from_tuple_impl`
will have the same advantages when used alone.

---------

Signed-off-by: yronglin <yronglin777@gmail.com>
XavierRaynaud pushed a commit to kalray/llvm-project that referenced this pull request Oct 28, 2025
…lent of) a C-style cast) (llvm#85263)

Implement [LWG3528](https://wg21.link/LWG3528).
Based on LWG3528(https://wg21.link/LWG3528) and
http://eel.is/c++draft/description#structure.requirements-9, the
standard allows to impose requirements, we constraint
`std::make_from_tuple` to make `std::make_from_tuple` SFINAE friendly
and also avoid worse diagnostic messages. We still keep the constraints
of `std::__make_from_tuple_impl` so that `std::__make_from_tuple_impl`
will have the same advantages when used alone.

---------

Signed-off-by: yronglin <yronglin777@gmail.com>
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