-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[libc++][ranges] LWG3610: iota_view::size sometimes rejects integer-class types
#155169
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
base: main
Are you sure you want to change the base?
[libc++][ranges] LWG3610: iota_view::size sometimes rejects integer-class types
#155169
Conversation
|
@llvm/pr-subscribers-libcxx Author: Hristo Hristov (H-G-Hristov) ChangesFixes #104948 Full diff: https://github.com/llvm/llvm-project/pull/155169.diff 3 Files Affected:
diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index 189f8452e0678..38571dabae463 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -143,7 +143,7 @@
"`LWG3598 <https://wg21.link/LWG3598>`__","``system_category().default_error_condition(0)`` is underspecified","2022-02 (Virtual)","","",""
"`LWG3601 <https://wg21.link/LWG3601>`__","common_iterator's postfix-proxy needs ``indirectly_readable`` ","2022-02 (Virtual)","","",""
"`LWG3607 <https://wg21.link/LWG3607>`__","``contiguous_iterator`` should not be allowed to have custom ``iter_move`` and ``iter_swap`` behavior","2022-02 (Virtual)","|Nothing To Do|","",""
-"`LWG3610 <https://wg21.link/LWG3610>`__","``iota_view::size`` sometimes rejects integer-class types","2022-02 (Virtual)","","",""
+"`LWG3610 <https://wg21.link/LWG3610>`__","``iota_view::size`` sometimes rejects integer-class types","2022-02 (Virtual)","|Complete|","22",""
"`LWG3612 <https://wg21.link/LWG3612>`__","Inconsistent pointer alignment in ``std::format`` ","2022-02 (Virtual)","|Complete|","14",""
"`LWG3616 <https://wg21.link/LWG3616>`__","LWG 3498 seems to miss the non-member ``swap`` for ``basic_syncbuf`` ","2022-02 (Virtual)","|Complete|","18",""
"`LWG3618 <https://wg21.link/LWG3618>`__","Unnecessary ``iter_move`` for ``transform_view::iterator`` ","2022-02 (Virtual)","|Complete|","19",""
diff --git a/libcxx/include/__ranges/iota_view.h b/libcxx/include/__ranges/iota_view.h
index 4b84585258b91..cf401a4665fcd 100644
--- a/libcxx/include/__ranges/iota_view.h
+++ b/libcxx/include/__ranges/iota_view.h
@@ -348,7 +348,7 @@ class iota_view : public view_interface<iota_view<_Start, _BoundSentinel>> {
_LIBCPP_HIDE_FROM_ABI constexpr auto size() const
requires(same_as<_Start, _BoundSentinel> && __advanceable<_Start>) ||
- (integral<_Start> && integral<_BoundSentinel>) || sized_sentinel_for<_BoundSentinel, _Start>
+ (__integer_like<_Start> && __integer_like<_BoundSentinel>) || sized_sentinel_for<_BoundSentinel, _Start>
{
if constexpr (__integer_like<_Start> && __integer_like<_BoundSentinel>) {
return (__value_ < 0)
diff --git a/libcxx/test/std/ranges/range.factories/range.iota.view/size.pass.cpp b/libcxx/test/std/ranges/range.factories/range.iota.view/size.pass.cpp
index b894bc542be10..90afec2fba177 100644
--- a/libcxx/test/std/ranges/range.factories/range.iota.view/size.pass.cpp
+++ b/libcxx/test/std/ranges/range.factories/range.iota.view/size.pass.cpp
@@ -16,8 +16,21 @@
#include <ranges>
#include "test_macros.h"
+#define TEST_HAS_NO_INT128
+#include "type_algorithms.h"
#include "types.h"
+template <typename T>
+concept HasSize = requires(const T t) { t.size(); };
+
+struct CheckForSize {
+ template <class T>
+ constexpr void operator()() {
+ static_assert(HasSize<std::ranges::iota_view<T, int>>);
+ static_assert(HasSize<std::ranges::iota_view<T, T>>);
+ }
+};
+
constexpr bool test() {
// Both are integer like and both are less than zero.
{
@@ -99,6 +112,11 @@ constexpr bool test() {
assert(sz == 10);
}
+ // LWG3610: `iota_view::size` sometimes rejects integer-class types
+ {
+ types::for_each(types::integer_types{}, CheckForSize{});
+ }
+
return true;
}
|
integer-like types bigger than
|
Oh... Then we also need to test using integer-class type as the (And if we want to properly support extended integer types, the restriction should be relaxed.) |
For the record: there was mentioning of supporting |
…s-rejects-integer-class-types
…s-rejects-integer-class-types
…s-rejects-integer-class-types
…s-rejects-integer-class-types
…s-rejects-integer-class-types
…s-rejects-integer-class-types
…s-rejects-integer-class-types
…s-rejects-integer-class-types
|
Verify here: https://ci.chromium.org/p/fuchsia/g/llvm/console |
|
@frederick-vs-ja #167869 should have resolved the int128_t issue. |
ldionne
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This generally looks fine, but I'd like @frederick-vs-ja to chime in.
|
|
||
| constexpr bool test() { | ||
| testType<SomeInt>(); | ||
| #ifndef TEST_HAS_NO_INT128 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're at it, I suppose we could/should add int128_t support for the remaining member functions of iota_view?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean updating the remaining tests or something else? These specific changes are from my initial attempt to update iota_view to support int128_t, which was then implemented in #167869 and after I rebased I didn't remove them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, what I'm saying is that I think we can expand the testing for int128_t further. This should arguably have been done in #167869 but late is better than never.
Fixes #104948
Depends on #167869 for
int128_tsupport.References