[PATCH v1] libstdc++: Fix class mandate for extents.

[Luc Grosheintz]

On 5/15/25 5:12 PM, Jonathan Wakely wrote:
> On Thu, 15 May 2025 at 16:12, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>>
>> On Thu, 15 May 2025 at 16:11, Luc Grosheintz <luc.grosheintz@gmail.com> wrote:
>>>
>>> Without would make sense to me, because whenever I wrote an
>>> identifier with _ I felt like I was presenting the user with
>>> a name that they shouldn't know about.
>>>
>>> A pedantic question: Can I also fix the line below, or do
>>> you prefer to be strict about one semantic change per commit?
>>
>> We don't need to be that strict. Don't worry, I've already changed it
>> locally and will push it for you.
> 
> To be clear, we do prefer separate changes to be separate commits, but
> I think "add a static assert and tweak the one on the line below" is
> fine to consider as a single change.

Because I've left out other opportunities in the spirit of keeping
the number of emails, I'll do so now:

Thank you for fixing this; thanks to Jonathan and Tomasz for the
very kind welcome. And finally the assurance that I aim to keep
the additional work I cause you as low as possible and if it's
easier for you to send something back for me to fix, please do so.

> 
> 
>>
>>
>>>
>>> On 5/15/25 2:40 PM, Tomasz Kaminski wrote:
>>>> No strong preference, but Ville's argument sounds reasonable.
>>>>
>>>> On Thu, May 15, 2025 at 12:25 PM Ville Voutilainen <
>>>> ville.voutilainen@gmail.com> wrote:
>>>>
>>>>> Mild preference against; use the names from the standard, not the
>>>>> implementation, in such diagnostics.
>>>>>
>>>>> to 15. toukok. 2025 klo 13.20 Jonathan Wakely <jwakely@redhat.com>
>>>>> kirjoitti:
>>>>>
>>>>>> On Thu, 15 May 2025 at 11:14, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>>>>>
>>>>>>> On Wed, 14 May 2025 at 20:18, Luc Grosheintz <luc.grosheintz@gmail.com>
>>>>>> wrote:
>>>>>>>>
>>>>>>>> The standard states that the IndexType must be a signed or unsigned
>>>>>>>> integer. This mandate was implemented using `std::is_integral_v`.
>>>>>> Which
>>>>>>>> also includes (among others) char and bool, which neither signed nor
>>>>>>>> unsigned integers.
>>>>>>>>
>>>>>>>> libstdc++-v3/ChangeLog:
>>>>>>>>
>>>>>>>>           * include/std/mdspan: Implement the mandate for extents as
>>>>>>>>           signed or unsigned integer and not any interal type.
>>>>>>>>           *
>>>>>> testsuite/23_containers/mdspan/extents/class_mandates_neg.cc: Check
>>>>>>>>           that extents<char,...> and extents<bool,...> are invalid.
>>>>>>>>           * testsuite/23_containers/mdspan/extents/misc.cc: Update
>>>>>>>>           tests to avoid `char` and `bool` as IndexType.
>>>>>>>> ---
>>>>>>>>    libstdc++-v3/include/std/mdspan                        |  3 ++-
>>>>>>>>    .../23_containers/mdspan/extents/class_mandates_neg.cc | 10
>>>>>> +++++++---
>>>>>>>>    .../testsuite/23_containers/mdspan/extents/misc.cc     |  8 ++++----
>>>>>>>>    3 files changed, 13 insertions(+), 8 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/libstdc++-v3/include/std/mdspan
>>>>>> b/libstdc++-v3/include/std/mdspan
>>>>>>>> index aee96dda7cd..22509d9c8f4 100644
>>>>>>>> --- a/libstdc++-v3/include/std/mdspan
>>>>>>>> +++ b/libstdc++-v3/include/std/mdspan
>>>>>>>> @@ -163,7 +163,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>>>>>>      template<typename _IndexType, size_t... _Extents>
>>>>>>>>        class extents
>>>>>>>>        {
>>>>>>>> -      static_assert(is_integral_v<_IndexType>, "_IndexType must be
>>>>>> integral.");
>>>>>>>> +      static_assert(__is_standard_integer<_IndexType>::value,
>>>>>>>> +                   "_IndexType must be a signed or unsigned
>>>>>> integer.");
>>>>>>>
>>>>>>> GCC's diagnostics never end with a full stop (aka period), and we
>>>>>>> follow that convention for our static assertions.
>>>>>>>
>>>>>>> So I'll remove the '.' at the end of the string literal, and then push
>>>>>>> this to trunk.
>>>>>>>
>>>>>>>
>>>>>>>>          static_assert(
>>>>>>>>             (__mdspan::__valid_static_extent<_Extents, _IndexType> &&
>>>>>> ...),
>>>>>>>>             "Extents must either be dynamic or representable as
>>>>>> _IndexType");
>>>>>>
>>>>>> I've just noticed that this static_assert refers to "Extents" without
>>>>>> the leading underscore, but "_IndexType" with a leading underscore.
>>>>>>
>>>>>> I think it's OK to omit the leading underscore, it might be a bit more
>>>>>> user-friendly and I don't think anybody will be confused by the fact
>>>>>> it's not identical to the real template parameter. But we should
>>>>>> either do it consistently for _Extents and _IndexType or for neither
>>>>>> of them.
>>>>>>
>>>>>> Anybody want to argue for or against underscores?
>>>>>>
>>>>>>
>>>>
>>>