[libcxx-commits] [libcxx] [libc++] `views::split` and `views::lazy_split` shouldn't be range adaptor closures (PR #75266)
via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Dec 12 17:02:19 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Stephan T. Lavavej (StephanTLavavej)
<details>
<summary>Changes</summary>
Fixes #<!-- -->75002. Found while running libc++'s tests with MSVC's STL.
This is a superset of #<!-- -->74961 that also attempts to fix the product code and add a regression test. Thanks again, @<!-- -->cpplearner!
<details><summary>Click to expand Standardese citations:</summary>
* [N4964](https://isocpp.org/files/papers/N4964.pdf) \[range.split.overview\]/2:
> The name `views::split` denotes a range adaptor object (\[range.adaptor.object\]). Given subexpressions `E` and `F`, the expression `views::split(E, F)` is expression-equivalent to `split_view(E, F)`.
* \[range.lazy.split.overview\]/2:
> The name `views::lazy_split` denotes a range adaptor object (\[range.adaptor.object\]). Given subexpressions `E` and `F`, the expression `views::lazy_split(E, F)` is expression-equivalent to `lazy_split_view(E, F)`.
* \[range.adaptor.object\]/1:
> A *range adaptor closure object* is a unary function object that accepts a range argument. For a range adaptor
closure object `C` and an expression `R` such that `decltype((R))` models `range`, the following expressions are
equivalent:
> `C(R)`
> `R | C`
* \[range.adaptor.object\]/6-8:
> A *range adaptor object* is a customization point object (\[customization.point.object\]) that accepts a `viewable_range` as its first argument and returns a view.
> If a range adaptor object accepts only one argument, then it is a range adaptor closure object.
> If a range adaptor object `adaptor` accepts more than one argument, then let `range` be an expression such that `decltype((range))` models `viewable_range`, let `args...` be arguments such that `adaptor(range, args...)` is a well-formed expression as specified in the rest of subclause \[range.adaptors\], and let `BoundArgs` be a pack that denotes `decay_t<decltype((args))>...`. The expression `adaptor(args...)` produces a range adaptor closure object `f` that is a perfect forwarding call wrapper (\[func.require\]) with the following properties: \[...\]
</details>
To summarize: `views::split` and `views::lazy_split` aren't unary, aren't range adaptor **closure** objects, and can't be piped. However, \[range.adaptor.object\]/8 says that `views::split(pattern)` and `views::lazy_split(pattern)` produce unary, pipeable, range adaptor closure objects.
This PR adjusts the test coverage accordingly, allowing it to portably pass for libc++ and MSVC's STL.
<details><summary>Click to expand the Clang compiler error that discovered this:</summary>
```
D:\GitHub\STL\llvm-project\libcxx\test\std\ranges\range.adaptors\range.split\adaptor.pass.cpp(42,1): error: static assertion failed
static_assert( CanBePiped<SomeView&, decltype(std::views::split)>);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
D:\GitHub\STL\llvm-project\libcxx\test\std\ranges\range.adaptors\range.split\adaptor.pass.cpp(42,16): note: because 'CanBePiped<SomeView &, decltype(std::views::split)>' evaluated to false
static_assert( CanBePiped<SomeView&, decltype(std::views::split)>);
^
D:\GitHub\STL\llvm-project\libcxx\test\std\ranges\range.adaptors\range.split\adaptor.pass.cpp(25,30): note: because 'std::forward<View>(view) | std::forward<T>(t)' would be invalid: invalid operands to binary expression ('SomeView' and 'const std::ranges::views::_Split_fn')
{ std::forward<View>(view) | std::forward<T>(t) };
^
D:\GitHub\STL\llvm-project\libcxx\test\std\ranges\range.adaptors\range.split\adaptor.pass.cpp(43,1): error: static assertion failed
static_assert( CanBePiped<char(&)[10], decltype(std::views::split)>);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
D:\GitHub\STL\llvm-project\libcxx\test\std\ranges\range.adaptors\range.split\adaptor.pass.cpp(43,16): note: because 'CanBePiped<char (&)[10], decltype(std::views::split)>' evaluated to false
static_assert( CanBePiped<char(&)[10], decltype(std::views::split)>);
^
D:\GitHub\STL\llvm-project\libcxx\test\std\ranges\range.adaptors\range.split\adaptor.pass.cpp(25,30): note: because 'std::forward<View>(view) | std::forward<T>(t)' would be invalid: invalid operands to binary expression ('char[10]' and 'const std::ranges::views::_Split_fn')
{ std::forward<View>(view) | std::forward<T>(t) };
^
```
</details>
---
Full diff: https://github.com/llvm/llvm-project/pull/75266.diff
4 Files Affected:
- (modified) libcxx/include/__ranges/lazy_split_view.h (+1-1)
- (modified) libcxx/include/__ranges/split_view.h (+1-1)
- (modified) libcxx/test/std/ranges/range.adaptors/range.lazy.split/adaptor.pass.cpp (+10-4)
- (modified) libcxx/test/std/ranges/range.adaptors/range.split/adaptor.pass.cpp (+10-4)
``````````diff
diff --git a/libcxx/include/__ranges/lazy_split_view.h b/libcxx/include/__ranges/lazy_split_view.h
index 2c654bfd325a63..8ed4bcfdeb56d4 100644
--- a/libcxx/include/__ranges/lazy_split_view.h
+++ b/libcxx/include/__ranges/lazy_split_view.h
@@ -437,7 +437,7 @@ lazy_split_view(_Range&&, range_value_t<_Range>)
namespace views {
namespace __lazy_split_view {
-struct __fn : __range_adaptor_closure<__fn> {
+struct __fn {
template <class _Range, class _Pattern>
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI
constexpr auto operator()(_Range&& __range, _Pattern&& __pattern) const
diff --git a/libcxx/include/__ranges/split_view.h b/libcxx/include/__ranges/split_view.h
index a27ac4ef7a1965..7f03be3c346a42 100644
--- a/libcxx/include/__ranges/split_view.h
+++ b/libcxx/include/__ranges/split_view.h
@@ -194,7 +194,7 @@ struct split_view<_View, _Pattern>::__sentinel {
namespace views {
namespace __split_view {
-struct __fn : __range_adaptor_closure<__fn> {
+struct __fn {
// clang-format off
template <class _Range, class _Pattern>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI
diff --git a/libcxx/test/std/ranges/range.adaptors/range.lazy.split/adaptor.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.lazy.split/adaptor.pass.cpp
index da4bd9fbbe1794..6bfa0ab487ba1b 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.lazy.split/adaptor.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.lazy.split/adaptor.pass.cpp
@@ -40,10 +40,16 @@ static_assert(!std::is_invocable_v<decltype(std::views::lazy_split), SomeView, N
static_assert(!std::is_invocable_v<decltype(std::views::lazy_split), NotAView, SomeView>);
static_assert( std::is_invocable_v<decltype(std::views::lazy_split), SomeView, SomeView>);
-static_assert( CanBePiped<SomeView&, decltype(std::views::lazy_split)>);
-static_assert( CanBePiped<char(&)[10], decltype(std::views::lazy_split)>);
-static_assert(!CanBePiped<char(&&)[10], decltype(std::views::lazy_split)>);
-static_assert(!CanBePiped<NotAView, decltype(std::views::lazy_split)>);
+// Regression test for #75002, views::lazy_split shouldn't be a range adaptor closure
+static_assert(!CanBePiped<SomeView&, decltype(std::views::lazy_split)>);
+static_assert(!CanBePiped<char (&)[10], decltype(std::views::lazy_split)>);
+static_assert(!CanBePiped<char (&&)[10], decltype(std::views::lazy_split)>);
+static_assert(!CanBePiped<NotAView, decltype(std::views::lazy_split)>);
+
+static_assert(CanBePiped<SomeView&, decltype(std::views::lazy_split('x'))>);
+static_assert(CanBePiped<char (&)[10], decltype(std::views::lazy_split('x'))>);
+static_assert(!CanBePiped<char (&&)[10], decltype(std::views::lazy_split('x'))>);
+static_assert(!CanBePiped<NotAView, decltype(std::views::lazy_split('x'))>);
static_assert(std::same_as<decltype(std::views::lazy_split), decltype(std::ranges::views::lazy_split)>);
diff --git a/libcxx/test/std/ranges/range.adaptors/range.split/adaptor.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.split/adaptor.pass.cpp
index cd12011daeab5d..85d13ac5c29dfb 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.split/adaptor.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.split/adaptor.pass.cpp
@@ -39,10 +39,16 @@ static_assert(!std::is_invocable_v<decltype(std::views::split), SomeView, NotAVi
static_assert(!std::is_invocable_v<decltype(std::views::split), NotAView, SomeView>);
static_assert( std::is_invocable_v<decltype(std::views::split), SomeView, SomeView>);
-static_assert( CanBePiped<SomeView&, decltype(std::views::split)>);
-static_assert( CanBePiped<char(&)[10], decltype(std::views::split)>);
-static_assert(!CanBePiped<char(&&)[10], decltype(std::views::split)>);
-static_assert(!CanBePiped<NotAView, decltype(std::views::split)>);
+// Regression test for #75002, views::split shouldn't be a range adaptor closure
+static_assert(!CanBePiped<SomeView&, decltype(std::views::split)>);
+static_assert(!CanBePiped<char (&)[10], decltype(std::views::split)>);
+static_assert(!CanBePiped<char (&&)[10], decltype(std::views::split)>);
+static_assert(!CanBePiped<NotAView, decltype(std::views::split)>);
+
+static_assert(CanBePiped<SomeView&, decltype(std::views::split('x'))>);
+static_assert(CanBePiped<char (&)[10], decltype(std::views::split('x'))>);
+static_assert(!CanBePiped<char (&&)[10], decltype(std::views::split('x'))>);
+static_assert(!CanBePiped<NotAView, decltype(std::views::split('x'))>);
static_assert(std::same_as<decltype(std::views::split), decltype(std::ranges::views::split)>);
``````````
</details>
https://github.com/llvm/llvm-project/pull/75266
More information about the libcxx-commits
mailing list