On Thu, 15 May 2025 at 16:12, Jonathan Wakely <jwakely....@gmail.com> wrote:
>
> 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.
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.
>
>
> >
> > 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?
> > >>>
> > >>>
> > >
> >
