On Thu, 15 May 2025 at 16:11, Luc Grosheintz <luc.groshei...@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.
>
> 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.voutilai...@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 <jwak...@redhat.com>
> >> kirjoitti:
> >>
> >>> On Thu, 15 May 2025 at 11:14, Jonathan Wakely <jwak...@redhat.com> wrote:
> >>>>
> >>>> On Wed, 14 May 2025 at 20:18, Luc Grosheintz <luc.groshei...@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?
> >>>
> >>>
> >
>
