[libcxx] applies integer-like changes from [P2393R1] #74161

cjdb · 2023-12-02T00:31:33Z

libc++ is already conforming with this paper since its integer-like types are extended integral types. Given that Clang supports _BitInt as a C++ extension, there isn't a need to support class-types' potential explicit conversion requirements.

We still do so, however, because software development is malleable. These changes don't impact codegen or correctness, and this will add resilience to change if the above paragraph ultimately proves false.

Closes #105171

llvmbot · 2023-12-02T00:31:52Z

@llvm/pr-subscribers-libcxx

Author: Christopher Di Bella (cjdb)

Changes

libc++ is already conforming with this paper since its integer-like types are extended integral types. Given that Clang supports _BitInt as a C++ extension, there isn't a need to support class-types' potential explicit conversion requriements.

We still do so, however, because software development is malleable. These changes don't impact codegen or correctness, and this will add resilience to change if the above paragraph ultimately proves false.

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

4 Files Affected:

(modified) libcxx/docs/Status/Cxx23Papers.csv (+1-1)
(modified) libcxx/include/__ranges/counted.h (+2-2)
(modified) libcxx/include/__ranges/subrange.h (+1-1)
(modified) libcxx/include/__ranges/take_view.h (+4-4)

diff --git a/libcxx/docs/Status/Cxx23Papers.csv b/libcxx/docs/Status/Cxx23Papers.csv
index 5cc9e488297b9f7..1fceeacc589cf63 100644
--- a/libcxx/docs/Status/Cxx23Papers.csv
+++ b/libcxx/docs/Status/Cxx23Papers.csv
@@ -35,7 +35,7 @@
 "`P2301R1 <https://wg21.link/P2301R1>`__","LWG","Add a ``pmr`` alias for ``std::stacktrace``","October 2021","",""
 "`P2321R2 <https://wg21.link/P2321R2>`__","LWG","``zip``","October 2021","|In Progress|","","|ranges|"
 "`P2340R1 <https://wg21.link/P2340R1>`__","LWG","Clarifying the status of the 'C headers'","October 2021","",""
-"`P2393R1 <https://wg21.link/P2393R1>`__","LWG","Cleaning up ``integer``-class types","October 2021","",""
+"`P2393R1 <https://wg21.link/P2393R1>`__","LWG","Cleaning up ``integer``-class types","October 2021","|Complete|","18.0"
 "`P2401R0 <https://wg21.link/P2401R0>`__","LWG","Add a conditional ``noexcept`` specification to ``std::exchange``","October 2021","|Complete|","14.0"
 "","","","","","",""
 "`P0323R12 <https://wg21.link/P0323R12>`__","LWG","``std::expected``","February 2022","|Complete|","16.0"
diff --git a/libcxx/include/__ranges/counted.h b/libcxx/include/__ranges/counted.h
index 882f90b1ed82e7c..d7e967c7627dee6 100644
--- a/libcxx/include/__ranges/counted.h
+++ b/libcxx/include/__ranges/counted.h
@@ -41,9 +41,9 @@ namespace __counted {
     template<contiguous_iterator _It>
     _LIBCPP_HIDE_FROM_ABI
     static constexpr auto __go(_It __it, iter_difference_t<_It> __count)
-      noexcept(noexcept(span(std::to_address(__it), static_cast<size_t>(__count))))
+      noexcept(noexcept(span(std::to_address(__it), static_cast<size_t>(static_cast<iter_difference_t<_It>>(__count)))))
       // Deliberately omit return-type SFINAE, because to_address is not SFINAE-friendly
-      { return          span(std::to_address(__it), static_cast<size_t>(__count)); }
+      { return          span(std::to_address(__it), static_cast<size_t>(static_cast<iter_difference_t<_It>>(__count))); }
 
     template<random_access_iterator _It>
     _LIBCPP_HIDE_FROM_ABI
diff --git a/libcxx/include/__ranges/subrange.h b/libcxx/include/__ranges/subrange.h
index 75f9284a582ff1a..0413f1821678750 100644
--- a/libcxx/include/__ranges/subrange.h
+++ b/libcxx/include/__ranges/subrange.h
@@ -129,7 +129,7 @@ namespace ranges {
     _LIBCPP_HIDE_FROM_ABI
     constexpr subrange(_Range&& __range)
       requires _StoreSize && sized_range<_Range>
-      : subrange(__range, ranges::size(__range))
+      : subrange(__range, static_cast<decltype(__size_)>(ranges::size(__range)))
     { }
 
     template<borrowed_range _Range>
diff --git a/libcxx/include/__ranges/take_view.h b/libcxx/include/__ranges/take_view.h
index 4204017d9249bcf..5331a0e3843dd91 100644
--- a/libcxx/include/__ranges/take_view.h
+++ b/libcxx/include/__ranges/take_view.h
@@ -87,7 +87,7 @@ class take_view : public view_interface<take_view<_View>> {
         return ranges::begin(__base_);
       } else {
         using _DifferenceT = range_difference_t<_View>;
-        auto __size = size();
+        auto __size = range_difference_t<_View>(size());
         return counted_iterator(ranges::begin(__base_), static_cast<_DifferenceT>(__size));
       }
     } else {
@@ -102,7 +102,7 @@ class take_view : public view_interface<take_view<_View>> {
         return ranges::begin(__base_);
       } else {
         using _DifferenceT = range_difference_t<const _View>;
-        auto __size = size();
+        auto __size = range_difference_t<_View>(size());
         return counted_iterator(ranges::begin(__base_), static_cast<_DifferenceT>(__size));
       }
     } else {
@@ -114,7 +114,7 @@ class take_view : public view_interface<take_view<_View>> {
   constexpr auto end() requires (!__simple_view<_View>) {
     if constexpr (sized_range<_View>) {
       if constexpr (random_access_range<_View>) {
-        return ranges::begin(__base_) + size();
+        return ranges::begin(__base_) + range_difference_t<_View>(size());
       } else {
         return default_sentinel;
       }
@@ -127,7 +127,7 @@ class take_view : public view_interface<take_view<_View>> {
   constexpr auto end() const requires range<const _View> {
     if constexpr (sized_range<const _View>) {
       if constexpr (random_access_range<const _View>) {
-        return ranges::begin(__base_) + size();
+        return ranges::begin(__base_) + range_difference_t<_View>(size());
       } else {
         return default_sentinel;
       }

github-actions · 2023-12-02T00:33:47Z

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

ldionne

Thanks for the patch!

Can we add some tests for this change? I think this amounts to asking which integer-class types (if any) we support in libc++ / Clang. IIUC, we have the following integer-class types in Clang: __int128_t, __uint128_t and _BitInt(N) -- are there more? Do we "support" them, and if so, in what places? It seems like std::atomic intentionally doesn't support _BitInt, see https://github.com/llvm/llvm-project/blob/main/libcxx/test/libcxx/atomics/bit-int.verify.cpp#L11, but I don't know about other classes.

If the answer is "yes, those are integer-class types, we support them and they should work in Ranges", then I think we should add some tests with those types to lock down the intent of P2393R1. We might not end up testing exactly the diff in this patch because of conversion behavior which might not strictly require those casts, but I think it would be important to make sure that they work before claiming support for P2393R1. WDYT?

mordante

Was the paper voted in as a DR?

Are there tests with _BitInt to validate this really works?

mordante · 2023-12-09T13:08:28Z

libcxx/docs/Status/Cxx23Papers.csv

Please add the paper to the release notes too.

May be related: P2393R1 also superseded a related section in P2321R2.

llvm-project/libcxx/docs/Status/ZipProjects.csv

Line 9 in 8566cd6

| `[iterator.concept.winc] <https://wg21.link/iterator.concept.winc>`_, "Update weakly_comparable", None, Hui Xie, |Not Started|

~~I think we should say "Superseded by P2393R1" and/or "Nothing To Do" for the status about [iterator.concept.winc] in ZipProjects.csv.~~ No change requested - ZipProjects.csv has been removed.

cjdb · 2023-12-13T00:18:04Z

Sorry, I've only just seen these comments, I'll look into them tomorrow.

cjdb · 2023-12-13T00:38:38Z

IIUC, we have the following integer-class types in Clang: __int128_t, __uint128_t and _BitInt(N) -- are there more? Do we "support" them, and if so, in what places? It seems like std::atomic intentionally doesn't support _BitInt, see https://github.com/llvm/llvm-project/blob/main/libcxx/test/libcxx/atomics/bit-int.verify.cpp#L11, but I don't know about other classes.

If the answer is "yes, those are integer-class types, we support them and they should work in Ranges", then I think we should add some tests with those types to lock down the intent of P2393R1. We might not end up testing exactly the diff in this patch because of conversion behavior which might not strictly require those casts, but I think it would be important to make sure that they work before claiming support for P2393R1. WDYT?

Yes to __int128 and unsigned __int128. "libc++ only supports integral types that the compiler supports" was a design decision early in the libc++ ranges design (see here). I would prefer to support _BitInt(N), but I'm not against supporting it on a best-effort basis (i.e. fix bugs as they are filed). My guess is that std::atomic has potential hardware-level atomic constraints, whereas ranges don't have that extra restriction.

A cursory glance tells me that there aren't any tests, so I'll fix those up tomorrow.

Was the paper voted in as a DR?

Yes, it resolves LWG 3366 and LWG 3376.

libc++ is already conforming with this paper since its _integer-like_ types are extended integral types. Given that Clang supports `_BitInt` as a C++ extension, there isn't a need to support class-types' potential explicit conversion requriements. We still do so, however, because software development is malleable. These changes don't impact codegen or correctness, and this will add resilience to change if the above paragraph ultimately proves false. [P2393R1]: http://wg21.link/p2393r1

ldionne · 2024-01-08T18:42:30Z

Yes to __int128 and unsigned __int128. "libc++ only supports integral types that the compiler supports" was a design decision early in the libc++ ranges design (see here). I would prefer to support _BitInt(N), but I'm not against supporting it on a best-effort basis (i.e. fix bugs as they are filed). My guess is that std::atomic has potential hardware-level atomic constraints, whereas ranges don't have that extra restriction.

A cursory glance tells me that there aren't any tests, so I'll fix those up tomorrow.

Thanks for the answer! So in that case, should we have tests for e.g. a take_view whose underlying view's size() returns a int128_t? That's what we're actually adding support for in this patch, right?

ldionne · 2024-01-08T18:39:05Z

...r.requirements/iterator.concepts/iterator.concept.winc/weakly_incrementable.compile.pass.cpp

+#if defined(__BIT_TYPES_DEFINED__)
+#  pragma GCC diagnostic push
+#  pragma GCC diagnostic ignored "-Wbit-int-extension"


Suggestion: use

TEST_DIAGNOSTIC_PUSH TEST_GCC_DIAGNOSTIC_IGNORED(-Wbit-int-extension)

from test_macros.h. Non-blocking comment.

cjdb · 2024-01-08T19:19:48Z

Yes to __int128 and unsigned __int128. "libc++ only supports integral types that the compiler supports" was a design decision early in the libc++ ranges design (see here). I would prefer to support _BitInt(N), but I'm not against supporting it on a best-effort basis (i.e. fix bugs as they are filed). My guess is that std::atomic has potential hardware-level atomic constraints, whereas ranges don't have that extra restriction.
A cursory glance tells me that there aren't any tests, so I'll fix those up tomorrow.

Thanks for the answer! So in that case, should we have tests for e.g. a take_view whose underlying view's size() returns a int128_t? That's what we're actually adding support for in this patch, right?

Nice catch, I'll get on that :)

libcxx/include/__ranges/counted.h

libcxx/include/__ranges/take_view.h

frederick-vs-ja · 2025-07-13T16:47:34Z

Thanks! I've tried to resolve merge conflicts and removed redundant casts noticed by @timsong-cpp.

frederick-vs-ja · 2025-07-13T17:02:41Z

libcxx/docs/Status/Cxx23Papers.csv

 "`P2321R2 <https://wg21.link/P2321R2>`__","``zip``","2021-10 (Virtual)","|In Progress|","",""
 "`P2340R1 <https://wg21.link/P2340R1>`__","Clarifying the status of the 'C headers'","2021-10 (Virtual)","|Nothing To Do|","",""
-"`P2393R1 <https://wg21.link/P2393R1>`__","Cleaning up ``integer``-class types","2021-10 (Virtual)","","",""
+"`P2393R1 <https://wg21.link/P2393R1>`__","Cleaning up ``integer``-class types","2021-10 (Virtual)","|Complete|","21",""


I believe this paper should be a DR. But the meeting minutes (N4898) didn't say so, even thought it said some other LWG papers were DR, which was weird.

Do we want to additional say "this is implemented as a DR against C++20, MSVC STL does the same"? @timsong-cpp @ldionne

Also, now libcxx/docs/ReleaseNotes/21.rst needs to be changed.

frederick-vs-ja · 2025-07-20T14:50:49Z

P2393R1 added a requirement that "the width of an integer-class type is greater than that of every integral type of the same signedness". So do we need some additional changes to ensure that _BitInt(2), … _BitInt(64) are not integer-class types, while _BitInt(65) and so on are?

cjdb added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 2, 2023

cjdb requested review from lnihlen and var-const December 2, 2023 00:31

cjdb requested a review from a team as a code owner December 2, 2023 00:31

cjdb force-pushed the p2393 branch from af3ef3b to 9c42b06 Compare December 4, 2023 19:21

ldionne self-assigned this Dec 6, 2023

ldionne reviewed Dec 6, 2023

View reviewed changes

mordante reviewed Dec 9, 2023

View reviewed changes

cjdb added 2 commits December 13, 2023 20:02

adds test for __int128 and _BitInt(N)

44a02f1

cjdb force-pushed the p2393 branch from 9c42b06 to 44a02f1 Compare December 13, 2023 20:19

ldionne reviewed Jan 8, 2024

View reviewed changes

timsong-cpp reviewed Jan 9, 2024

View reviewed changes

libcxx/include/__ranges/counted.h Outdated Show resolved Hide resolved

libcxx/include/__ranges/take_view.h Outdated Show resolved Hide resolved

Merge branch 'main' into p2393

3dfb143

frederick-vs-ja reviewed Jul 13, 2025

View reviewed changes

frederick-vs-ja mentioned this pull request Aug 24, 2025

[libc++][ranges] LWG3610: iota_view::size sometimes rejects integer-class types #155169

Open

[libcxx] applies integer-like changes from [P2393R1] #74161

Are you sure you want to change the base?

[libcxx] applies integer-like changes from [P2393R1] #74161

Uh oh!

Conversation

cjdb commented Dec 2, 2023 • edited by ldionne Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 2, 2023

Uh oh!

github-actions bot commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

mordante left a comment

Choose a reason for hiding this comment

Uh oh!

mordante Dec 9, 2023

Choose a reason for hiding this comment

Uh oh!

frederick-vs-ja Jan 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!

cjdb commented Dec 13, 2023

Uh oh!

cjdb commented Dec 13, 2023

Uh oh!

ldionne commented Jan 8, 2024

Uh oh!

ldionne Jan 8, 2024

Choose a reason for hiding this comment

Uh oh!

cjdb commented Jan 8, 2024

Uh oh!

Uh oh!

Uh oh!

frederick-vs-ja commented Jul 13, 2025

Uh oh!

frederick-vs-ja Jul 13, 2025

Choose a reason for hiding this comment

Uh oh!

frederick-vs-ja commented Jul 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cjdb commented Dec 2, 2023 •

edited by ldionne

Loading

github-actions bot commented Dec 2, 2023 •

edited

Loading

frederick-vs-ja Jan 13, 2024 •

edited

Loading