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?
>
>
