Skip to content
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

[libc++][tuple][utility] P2968R2: Make std::ignore a first-class object #97401

Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
bd82810
[libc++][tuple][utility] P2968R2 Make `std::ignore` a first-class object
H-G-Hristov Jul 2, 2024
1aa5e01
Fixed C++11
H-G-Hristov Jul 3, 2024
cf0e266
Updated test
H-G-Hristov Jul 3, 2024
dd7e838
Formatting
H-G-Hristov Jul 3, 2024
8e475a5
Merge branch 'main' into hgh/libcxx/P2968R2-Make-std-ignore-a-first-c…
Zingam Jul 3, 2024
99900ba
Updated tests
H-G-Hristov Jul 5, 2024
51db352
Merge branch 'hgh/libcxx/P2968R2-Make-std-ignore-a-first-class-object…
H-G-Hristov Jul 5, 2024
9423973
Merge branch 'main' into hgh/libcxx/P2968R2-Make-std-ignore-a-first-c…
H-G-Hristov Jul 5, 2024
be73bf4
Added `include<>` tests
H-G-Hristov Jul 6, 2024
a6c9452
Merge branch 'main' into hgh/libcxx/P2968R2-Make-std-ignore-a-first-c…
H-G-Hristov Jul 7, 2024
4a124c9
Addressed comments
H-G-Hristov Jul 7, 2024
f79cd96
Fixes
H-G-Hristov Jul 7, 2024
6ceb5c7
More fixes
H-G-Hristov Jul 7, 2024
764a47d
Merge branch 'main' into hgh/libcxx/P2968R2-Make-std-ignore-a-first-c…
H-G-Hristov Jul 7, 2024
48132c9
Fixes
H-G-Hristov Jul 7, 2024
42dfd37
Merge branch 'hgh/libcxx/P2968R2-Make-std-ignore-a-first-class-object…
H-G-Hristov Jul 7, 2024
1cacd71
Addressed review comments
H-G-Hristov Jul 7, 2024
1689a30
Fixed a typo
H-G-Hristov Jul 7, 2024
0e462fc
Merge branch 'main' into hgh/libcxx/P2968R2-Make-std-ignore-a-first-c…
H-G-Hristov Jul 9, 2024
1888965
Updated status pages
H-G-Hristov Jul 9, 2024
ed76b82
Merge branch 'main' into hgh/libcxx/P2968R2-Make-std-ignore-a-first-c…
H-G-Hristov Jul 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions libcxx/docs/ReleaseNotes/19.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Implemented Papers
- P2872R3 - Remove ``wstring_convert`` From C++26
- P3142R0 - Printing Blank Lines with ``println`` (as DR against C++23)
- P2944R3 - Comparisons for ``reference_wrapper`` (comparison operators for ``reference_wrapper`` only)
- P2968R2 - Make ``std::ignore`` a first-class object
- P2302R4 - ``std::ranges::contains``
- P1659R3 - ``std::ranges::starts_with`` and ``std::ranges::ends_with``
- P3029R1 - Better ``mdspan``'s CTAD
Expand Down Expand Up @@ -74,6 +75,9 @@ Improvements and New Features

- The formatting library is updated to Unicode 15.1.0.

- ``std::ignore``\s ``const __ignore_t& operator=(_Tp&&) const`` was changed to
``const __ignore_type& operator=(const _Tp&) const noexcept`` for all language versions.

Deprecations and Removals
-------------------------

Expand Down
1 change: 1 addition & 0 deletions libcxx/include/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,7 @@ set(files
__thread/timed_backoff_policy.h
__tree
__tuple/find_index.h
__tuple/ignore.h
__tuple/make_tuple_types.h
__tuple/sfinae_helpers.h
__tuple/tuple_element.h
Expand Down
38 changes: 38 additions & 0 deletions libcxx/include/__tuple/ignore.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//===----------------------------------------------------------------------===//
//
// 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
//
//===----------------------------------------------------------------------===//

#ifndef _LIBCPP___TUPLE_IGNORE_H
#define _LIBCPP___TUPLE_IGNORE_H

#include <__config>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
#endif

#ifndef _LIBCPP_CXX03_LANG

_LIBCPP_BEGIN_NAMESPACE_STD

struct __ignore_type {
template <class _Tp>
_LIBCPP_HIDE_FROM_ABI constexpr const __ignore_type& operator=(const _Tp&) const noexcept {
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously this was _LIBCPP_CONSTEXPR_SINCE_CXX14, which was synchonized with constexpr additions to tuple (via N3471 in C++14). Not sure further (?) backport of constexpr is desired...

return *this;
}
};

# if _LIBCPP_STD_VER >= 17
inline
# endif
_LIBCPP_CONSTEXPR_SINCE_CXX14 __ignore_type ignore;
Copy link
Member

Choose a reason for hiding this comment

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

The way the #if was done originally was easier to understand. Can you use that style here instead?


_LIBCPP_END_NAMESPACE_STD

#endif // _LIBCPP_CXX03_LANG

#endif // _LIBCPP___TUPLE_IGNORE_H
1 change: 1 addition & 0 deletions libcxx/include/module.modulemap
Original file line number Diff line number Diff line change
Expand Up @@ -1840,6 +1840,7 @@ module std_private_thread_thread [system] {
module std_private_thread_timed_backoff_policy [system] { header "__thread/timed_backoff_policy.h" }

module std_private_tuple_find_index [system] { header "__tuple/find_index.h" }
module std_private_tuple_ignore [system] { header "__tuple/ignore.h" }
module std_private_tuple_make_tuple_types [system] { header "__tuple/make_tuple_types.h" }
module std_private_tuple_tuple_like_no_subrange [system] {
header "__tuple/tuple_like_no_subrange.h"
Expand Down
24 changes: 7 additions & 17 deletions libcxx/include/tuple
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,12 @@ tuple(allocator_arg_t, Alloc, pair<T1, T2>) -> tuple<T1, T2>; // since C++
template <class Alloc, class ...T>
tuple(allocator_arg_t, Alloc, tuple<T...>) -> tuple<T...>; // since C++17

inline constexpr unspecified ignore;
+struct ignore-type { // exposition only
+ constexpr const ignore-type&
+ operator=(const auto &) const noexcept
+ { return *this; }
+};
inline constexpr ignore-type ignore;

template <class... T> tuple<V...> make_tuple(T&&...); // constexpr in C++14
template <class... T> tuple<ATypes...> forward_as_tuple(T&&...) noexcept; // constexpr in C++14
Expand Down Expand Up @@ -215,6 +220,7 @@ template <class... Types>
#include <__memory/allocator_arg_t.h>
#include <__memory/uses_allocator.h>
#include <__tuple/find_index.h>
#include <__tuple/ignore.h>
#include <__tuple/make_tuple_types.h>
#include <__tuple/sfinae_helpers.h>
#include <__tuple/tuple_element.h>
Expand Down Expand Up @@ -1112,22 +1118,6 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 tuple<_Tp&...> tie(_T
return tuple<_Tp&...>(__t...);
}

template <class _Up>
struct __ignore_t {
template <class _Tp>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 const __ignore_t& operator=(_Tp&&) const {
return *this;
}
};

# if _LIBCPP_STD_VER >= 17
inline constexpr __ignore_t<unsigned char> ignore = __ignore_t<unsigned char>();
# else
namespace {
constexpr __ignore_t<unsigned char> ignore = __ignore_t<unsigned char>();
} // namespace
# endif

template <class... _Tp>
inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 tuple<typename __unwrap_ref_decay<_Tp>::type...>
make_tuple(_Tp&&... __t) {
Expand Down
4 changes: 4 additions & 0 deletions libcxx/include/utility
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,10 @@ template <class T>
#include <compare>
#include <initializer_list>

// [tuple.creation]

#include <__tuple/ignore.h>
mordante marked this conversation as resolved.
Show resolved Hide resolved

// [tuple.helper]
#include <__tuple/tuple_element.h>
#include <__tuple/tuple_size.h>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//===----------------------------------------------------------------------===//
//
// 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

// <tuple>
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 add a comment along the lines of

std::ignore should be provided by the headers <tuple> and <utility>.
This tests validates its presence in <tuple>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The paper also "recommends" to mention the same in <tuple>. Do you have any recommendations what and where to write?

YES, make ignore available through , and not repeat the specification, but follow the example of ranges accessor functions by saying in [tuple] somewhere that ignore is available also from header .

Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely sure what you mean with "The paper also "recommends" to mention the same in <tuple>"

The wording section contains

In [tuple.general](https://eel.is/c++draft/tuple.general) append the following paragraph

<ins>In addition to being available via inclusion of the <tuple> header, ignore ([tuple.syn]) is available when <utility> ([utility]) is included.</ins>

This is not a recommendation but a requirement.

Can you explain what your question is? Feel free to do it on Discord if you prefer that.


// inline constexpr ignore-type ignore;

#include <tuple>

#include "test_macros.h"

int main(int, char**) {
{ [[maybe_unused]] TEST_CONSTEXPR_CXX14 auto& ignore_v = std::ignore; }
}
Copy link
Member

Choose a reason for hiding this comment

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

Since it's a compile-time test we don't need main.
I like to avoid adding other headers; that way we're sure no other Standard headers are included. That way we can verify provides std::ignore.

Suggested change
#include "test_macros.h"
int main(int, char**) {
{ [[maybe_unused]] TEST_CONSTEXPR_CXX14 auto& ignore_v = std::ignore; }
}
[[maybe_unused]] auto& ignore_v = std::ignore; }

77 changes: 45 additions & 32 deletions libcxx/test/std/utilities/tuple/tuple.general/ignore.pass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,51 +6,64 @@
//
//===----------------------------------------------------------------------===//

// <tuple>
// UNSUPPORTED: c++03

// constexpr unspecified ignore;
// <tuple>

// UNSUPPORTED: c++03
// inline constexpr ignore-type ignore;

#include <cassert>
#include <cstdint>
#include <tuple>
#include <type_traits>

#include "test_macros.h"

constexpr bool test_ignore_constexpr()
{
#if TEST_STD_VER > 11
{ // Test that std::ignore provides constexpr converting assignment.
auto& res = (std::ignore = 42);
assert(&res == &std::ignore);
}
{ // Test that std::ignore provides constexpr copy/move constructors
auto copy = std::ignore;
auto moved = std::move(copy);
((void)moved);
}
{ // Test that std::ignore provides constexpr copy/move assignment
auto copy = std::ignore;
copy = std::ignore;
auto moved = std::ignore;
moved = std::move(copy);
}
static_assert(std::is_trivial<decltype(std::ignore)>::value, "");

#if TEST_STD_VER >= 17
[[nodiscard]] constexpr int test_nodiscard() { return 8294; }
#endif
return true;

TEST_CONSTEXPR_CXX14 bool test() {
{ [[maybe_unused]] auto& ignore_v = std::ignore; }

{ // Test that std::ignore provides converting assignment.
auto& res = (std::ignore = 42);
static_assert(noexcept(res = (std::ignore = 42)), "Must be noexcept");
assert(&res == &std::ignore);
}
{ // Test bit-field binding.
mordante marked this conversation as resolved.
Show resolved Hide resolved
struct S {
unsigned int bf : 3;
};
S s{0b010};
auto& res = (std::ignore = s.bf);
assert(&res == &std::ignore);
}
{ // Test that std::ignore provides copy/move constructors
auto copy = std::ignore;
[[maybe_unused]] auto moved = std::move(copy);
}
{ // Test that std::ignore provides copy/move assignment
auto copy = std::ignore;
copy = std::ignore;
auto moved = std::ignore;
moved = std::move(copy);
}

#if TEST_STD_VER >= 17
{ std::ignore = test_nodiscard(); }
#endif

return true;
}

int main(int, char**) {
{
constexpr auto& ignore_v = std::ignore;
((void)ignore_v);
}
{
static_assert(test_ignore_constexpr(), "");
}
{
LIBCPP_STATIC_ASSERT(std::is_trivial<decltype(std::ignore)>::value, "");
}
test();
#if TEST_STD_VER >= 14
static_assert(test(), "");
#endif

return 0;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//===----------------------------------------------------------------------===//
//
// 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

// <utility>

// inline constexpr ignore-type ignore;

#include <utility>

#include "test_macros.h"

int main(int, char**) {
{ [[maybe_unused]] TEST_CONSTEXPR_CXX14 auto& ignore_v = std::ignore; }
}
Loading