[libc++][ranges] LWG3715: `view_interface::empty` is overconstrained #85004

xiaoyang-sde · 2024-03-13T00:35:25Z

Abstract

This pull request implements LWG3715: view_interface::empty is overconstrained. Here is an example similar to those described in the report, which compiles with -stdlib=libstdc++ but failed to compile with -stdlib=libc++:

// https://godbolt.org/z/EWEoTzah3
std::istringstream input("1 2 3 4 5");
auto i = std::views::istream<int>(input);
auto r = std::views::counted(i.begin(), 4) | std::views::take(2);
assert(!r.empty());

Reference

github-actions · 2024-03-13T00:35:46Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-03-13T00:36:23Z

@llvm/pr-subscribers-libcxx

Author: Xiaoyang Liu (xiaoyang-sde)

Changes

Abstract

This pull request implements LWG3715: view_interface::empty is overconstrained. Here is an example similar to those described in the report, which compiles with -stdlib=libstdc++ but failed to compile with -stdlib=libc++:

// https://godbolt.org/z/EWEoTzah3
std::istringstream input("1 2 3 4 5");
auto i = std::views::istream&lt;int&gt;(input);
auto r = std::views::counted(i.begin(), 4) | std::views::take(2);
assert(!r.empty());

Reference

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

3 Files Affected:

(modified) libcxx/docs/Status/Cxx23Issues.csv (+1-1)
(modified) libcxx/include/__ranges/view_interface.h (+12-4)
(added) libcxx/test/std/ranges/range.utility/view.interface/lwg3715.pass.cpp (+31)

diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index 70480b33820580..a0c5a284e3291c 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -181,7 +181,7 @@
 "`3711 <https://wg21.link/LWG3711>`__","Missing preconditions for slide_view constructor","July 2022","","","|ranges|"
 "`3712 <https://wg21.link/LWG3712>`__","``chunk_view`` and ``slide_view`` should not be ``default_initializable``","July 2022","","","|ranges|"
 "`3713 <https://wg21.link/LWG3713>`__","Sorted with respect to comparator (only)","July 2022","",""
-"`3715 <https://wg21.link/LWG3715>`__","``view_interface::empty`` is overconstrained","July 2022","","","|ranges|"
+"`3715 <https://wg21.link/LWG3715>`__","``view_interface::empty`` is overconstrained","July 2022","|Complete|","19.0","|ranges|"
 "`3719 <https://wg21.link/LWG3719>`__","Directory iterators should be usable with default sentinel","July 2022","|Complete|","17.0","|ranges|"
 "`3721 <https://wg21.link/LWG3721>`__","Allow an ``arg-id`` with a value of zero for ``width`` in ``std-format-spec``","July 2022","|Complete|","16.0","|format|"
 "`3724 <https://wg21.link/LWG3724>`__","``decay-copy`` should be constrained","July 2022","|Complete|","14.0"
diff --git a/libcxx/include/__ranges/view_interface.h b/libcxx/include/__ranges/view_interface.h
index 84dd1c316de379..ecc14c0a9471cb 100644
--- a/libcxx/include/__ranges/view_interface.h
+++ b/libcxx/include/__ranges/view_interface.h
@@ -51,16 +51,24 @@ class view_interface {
 public:
   template <class _D2 = _Derived>
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool empty()
-    requires forward_range<_D2>
+    requires sized_range<_D2> || forward_range<_D2>
   {
-    return ranges::begin(__derived()) == ranges::end(__derived());
+    if constexpr (sized_range<_D2>) {
+      return ranges::size(__derived()) == 0;
+    } else {
+      return ranges::begin(__derived()) == ranges::end(__derived());
+    }
   }
 
   template <class _D2 = _Derived>
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool empty() const
-    requires forward_range<const _D2>
+    requires sized_range<const _D2> || forward_range<const _D2>
   {
-    return ranges::begin(__derived()) == ranges::end(__derived());
+    if constexpr (sized_range<const _D2>) {
+      return ranges::size(__derived()) == 0;
+    } else {
+      return ranges::begin(__derived()) == ranges::end(__derived());
+    }
   }
 
   template <class _D2 = _Derived>
diff --git a/libcxx/test/std/ranges/range.utility/view.interface/lwg3715.pass.cpp b/libcxx/test/std/ranges/range.utility/view.interface/lwg3715.pass.cpp
new file mode 100644
index 00000000000000..cc58f2facba012
--- /dev/null
+++ b/libcxx/test/std/ranges/range.utility/view.interface/lwg3715.pass.cpp
@@ -0,0 +1,31 @@
+//===----------------------------------------------------------------------===//
+//
+// 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, c++17
+
+// LWG 3715: `view_interface::empty` is overconstrained
+
+#include <cassert>
+#include <ranges>
+#include <sstream>
+
+bool test() {
+  std::istringstream input("1 2 3 4 5");
+  auto i = std::views::istream<int>(input);
+  auto r = std::views::counted(i.begin(), 4) | std::views::take(2);
+  static_assert(std::ranges::input_range<decltype(r)>);
+  static_assert(!std::ranges::forward_range<decltype(r)>);
+  static_assert(std::ranges::sized_range<decltype(r)>);
+  assert(!r.empty());
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  return 0;
+}

xiaoyang-sde · 2024-03-13T02:42:25Z

libcxx/include/__ranges/view_interface.h

  template <class _D2 = _Derived>
  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool empty()


Although unrelated to this pull request, I'm curious about the reason behind this method being a member template. It appears that this differs from the implementation in libstdc++.

This could be related to a Clang bug when we implemented this early on. I vaguely remember that there were bugs related to constraints on member functions in Clang a while back, and I think we had to work around it in a few places. But it could also be my memory playing tricks on me, so it's worth double-checking that information.

This could be related to a Clang bug when we implemented this early on. I vaguely remember that there were bugs related to constraints on member functions in Clang a while back, and I think we had to work around it in a few places. But it could also be my memory playing tricks on me, so it's worth double-checking that information.

Ah I see. Thanks for the explanation.

libcxx/test/std/ranges/range.utility/view.interface/lwg3715.pass.cpp

github-actions · 2024-03-13T07:16:57Z

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

libcxx/include/__ranges/view_interface.h

ldionne

This LGTM, thanks for the patch!

ldionne · 2024-03-18T01:00:03Z

(pending CI checks)

xiaoyang-sde · 2024-03-18T02:12:55Z

(pending CI checks)

Thanks for the review! It seems that a few CI checks have exceeded their time limits.

Zingam · 2024-03-20T05:20:13Z

(pending CI checks)

Thanks for the review! It seems that a few CI checks have exceeded their time limits.

This happens often. The CI can restart the timouted test automatically. If you have commit rights you can restart the gailed tests or you can rebase to trigger all tests.

github-actions · 2024-03-20T08:47:58Z

@xiaoyang-sde Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may recieve a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

…lvm#85004) ## Abstract This pull request implements LWG3715: `view_interface::empty` is overconstrained. Here is an example similar to those described in the report, which compiles with `-stdlib=libstdc++` but failed to compile with `-stdlib=libc++`: ```cpp // https://godbolt.org/z/EWEoTzah3 std::istringstream input("1 2 3 4 5"); auto i = std::views::istream<int>(input); auto r = std::views::counted(i.begin(), 4) | std::views::take(2); assert(!r.empty()); ``` ## Reference - [Draft C++ Standard: [view.interface.general]](https://eel.is/c++draft/view.interface.general) - [LWG3715](https://wg21.link/LWG3715)

Xiaoyang Liu added 2 commits March 12, 2024 17:21

[libc++][ranges] implement LWG3715

7a0c2c5

[libc++][ranges] implement 'lwg3715.pass.cpp'

29e95e1

xiaoyang-sde requested a review from a team as a code owner March 13, 2024 00:35

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

xiaoyang-sde commented Mar 13, 2024

View reviewed changes

Zingam reviewed Mar 13, 2024

View reviewed changes

libcxx/test/std/ranges/range.utility/view.interface/lwg3715.pass.cpp Outdated Show resolved Hide resolved

Zingam reviewed Mar 13, 2024

View reviewed changes

libcxx/test/std/ranges/range.utility/view.interface/lwg3715.pass.cpp Outdated Show resolved Hide resolved

xiaoyang-sde changed the title ~~[libc++][ranges] implement LWG3715~~ [libc++][ranges] implement LWG3715: view_interface::empty is overconstrained Mar 13, 2024

xiaoyang-sde changed the title ~~[libc++][ranges] implement LWG3715: view_interface::empty is overconstrained~~ [libc++][ranges] LWG3715: view_interface::empty is overconstrained Mar 13, 2024

[libc++][ranges] remove 'lwg3715.pass.cpp'

a34519f

[libc++][ranges] format code with 'clang-format'

56e2ee5

xiaoyang-sde requested a review from Zingam March 13, 2024 08:17

[libc++][ranges] add missing headers to 'view_interface.h'

e63525b

Zingam reviewed Mar 14, 2024

View reviewed changes

libcxx/include/__ranges/view_interface.h Show resolved Hide resolved

[libc++][ranges] add missing test cases to 'view.interface.pass.cpp'

a290ef5

ldionne approved these changes Mar 18, 2024

View reviewed changes

ldionne self-assigned this Mar 18, 2024

ldionne added the ranges Issues related to `<ranges>` label Mar 18, 2024

xiaoyang-sde mentioned this pull request Mar 19, 2024

[libc++][ranges] remove __workaround_52970 #85683

Merged

mordante merged commit cf09b7d into llvm:main Mar 20, 2024

xiaoyang-sde deleted the view_interface branch March 20, 2024 15:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++][ranges] LWG3715: `view_interface::empty` is overconstrained #85004

[libc++][ranges] LWG3715: `view_interface::empty` is overconstrained #85004

Uh oh!

xiaoyang-sde commented Mar 13, 2024

Uh oh!

github-actions bot commented Mar 13, 2024

Uh oh!

llvmbot commented Mar 13, 2024

Abstract

Reference

Uh oh!

xiaoyang-sde Mar 13, 2024 •

edited

Loading

Uh oh!

ldionne Mar 18, 2024

Uh oh!

xiaoyang-sde Mar 18, 2024

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 13, 2024 •

edited

Loading

Uh oh!

Uh oh!

ldionne left a comment

Uh oh!

ldionne commented Mar 18, 2024

Uh oh!

xiaoyang-sde commented Mar 18, 2024

Uh oh!

Zingam commented Mar 20, 2024

Uh oh!

github-actions bot commented Mar 20, 2024

Uh oh!

Uh oh!

		template <class _D2 = _Derived>
		[[nodiscard]] _LIBCPP_HIDE_FROM_ABI constexpr bool empty()

[libc++][ranges] LWG3715: view_interface::empty is overconstrained #85004

[libc++][ranges] LWG3715: view_interface::empty is overconstrained #85004

Uh oh!

Conversation

xiaoyang-sde commented Mar 13, 2024

Abstract

Reference

Uh oh!

github-actions bot commented Mar 13, 2024

Uh oh!

llvmbot commented Mar 13, 2024

Abstract

Reference

Uh oh!

xiaoyang-sde Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldionne Mar 18, 2024

Choose a reason for hiding this comment

Uh oh!

xiaoyang-sde Mar 18, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

ldionne commented Mar 18, 2024

Uh oh!

xiaoyang-sde commented Mar 18, 2024

Uh oh!

Zingam commented Mar 20, 2024

Uh oh!

github-actions bot commented Mar 20, 2024

Uh oh!

Uh oh!

[libc++][ranges] LWG3715: `view_interface::empty` is overconstrained #85004

[libc++][ranges] LWG3715: `view_interface::empty` is overconstrained #85004

xiaoyang-sde Mar 13, 2024 •

edited

Loading

github-actions bot commented Mar 13, 2024 •

edited

Loading