Observations from implementing layout_left_padded.
Tomasz Kaminski
tkaminsk@redhat.com
Thu Aug 21 15:49:53 GMT 2025
On Thu, Aug 21, 2025 at 5:31 PM Luc Grosheintz <luc.grosheintz@gmail.com>
wrote:
>
>
> On 8/21/25 12:19, Tomasz Kaminski wrote:
> > Hi,
> >
> > Now that we are done with all the other patches, I cleaned up my inbox,
> > and noticed that I haven't responded to this email.
> > I hope you are still interested in submitting patches for layouts,
>
> I was starting to think about using the PING feature. Yes, I've got
> "most" of this ready, so it feels wasteful to not finish it.
> (Other obligations and constant_wrapper interfered.)
>
> >
> > On Mon, Jun 30, 2025 at 10:34 AM Luc Grosheintz <
> luc.grosheintz@gmail.com>
> > wrote:
> >
> >> Dear libstdc++ developers,
> >>
> >> while implementing layout_left_padded, I noticed some unexpected
> >> behaviour and I'd like to ask for your input.
> >>
> >>
> >> 1. Odd mandate/prerequisite for padding value.
> >>
> >> The standard says the padding_value must be representable as
> >> index_type. This immediately precludes using `padding_value ==
> >> dynamic_extent` for any index_type < size_t [1]. Something
> >> similar happens in the ctors that accepts a padding value at
> >> runtime [2].
> >>
> > The padding value at runtime is never expected to be dynamic_extent (this
> > is only for static_extent),
> > so I think the [2] is correct.
>
> I agree, I don't understand anymore why I complained about [2].
>
> >
> > For the [1] I think we could be more lax, and incorporate it in the 5.3,
> > and saying that multiplication
> > of padding_value (if not dynamic) and 1 if ext.static_extent(0) is
> dynamic
> > of ext.static_extent(0) first
> > into index type. In other words we ignore padding values for ranks 0 and
> 1.
> > I would go for the above relaxed condition in the first implementation.
> >
>
> I'm not fully convinced we're understanding each other. I'll try
> an example:
>
> using Layout = std::layout_left_padded<std::dynamic_extent>;
> using Extent = std::extents<uint8_t, /* ... */>;
>
> // We can't write:
> Layout::mapping<Extents>
>
> because `padding_value` which is dynamic_extent, i.e. size_t(-1)
> is never representable as uint8_t; and then immediately violates
> the class mandate [1].
>
> Semantically, it doesn't seem right, because later we could still
> choose a perfectly suitable value (like `1`).
>
> If you look at the rest of the paper there's often this phrase:
>
> If [...] padding_value does not equal dynamic_extent, then [...]
>
> I believe that that caveat is missing in [1].
>
I think I understand your concern, and my suggestion is to completely
drop the unconditional mandate of padding_value fitting into index.
And instead, expand a bit the mandate in 5.3 to check that, if:
* rank > 1
* padding_value is not dynamic_extent
Then let FE be 1 if extent 0 is dynamic, and static_extent(0) otherwise,
*LEAST-MULTIPLE-AT-LEAST*(padding_value, FE).
So, padding value is ignored unless actually used by mapping.
>
> >
> >> Resolution: If padding_value != dynamic_extent, then ...
> >>
> >> [1]: https://eel.is/c++draft/mdspan.layout.leftpad#overview-5.2
> >> [2]: https://eel.is/c++draft/mdspan.layout.leftpad#cons-4.1
> >>
> >>
> >> 2. Not commonly-convertible.
> >>
> >> Usually, `Layout::mapping<E1>` can be converted to
> >> `Layout::mapping<E2>` if and only if `E1` can be converted to
> >> `E2`. Note that this is for different mappings of the same
> >> Layout.
> >>
> >> For layout_left_padded, the situation is different. If the
> >> extents differ, the following ctor [3] will be used:
> >>
> >> template<typename _LeftPaddedMapping>
> >> requires (...)
> >> constexpr explicit(_S_rank > 1
> >> && (padding_value != dynamic_extent
> >> || _LeftPaddedMapping::padding_value == dynamic_extent))
> >> mapping(const _LeftPaddedMapping& __other)
> >>
> >> Which means that, for rank > 1:
> >>
> >> static_assert(!is_convertible_v<
> >> layout_left_padded<2>::mapping<E1>,
> >> layout_left_padded<2>::mapping<E2>);
> >>
> >> even if E1 and E2 are convertible.
> >>
> >> At the same time that ctor allows too many conversions if rank <= 1,
> >> including:
> >>
> >> static_assert(!is_convertible_v<
> >> layout_left_padded<2>::mapping<std::extents<int, dyn>>,
> >> layout_left_padded<2>::mapping<std::extents<int, 1>>>);
> >>
> >> static_assert(!is_convertible_v<
> >> layout_left_padded<2>::mapping<std::extents<unsigned int, 1>>,
> >> layout_left_padded<2>::mapping<std::extents<int, 1>>>);
> >>
> >> This second half, I've included in:
> >> https://cplusplus.github.io/LWG/issue4272
> >>
> >> But the issue that they don't convert regularly is unaddressed.
> >>
> >> Resolution 1: Allow conversion if:
> >> - __is_layout_left_padded_mapping_of<_LeftPaddedMapping> is
> >> true,
> >> - same_as<layout_type,
> >> typename _LeftPaddedMapping::layout_type> is true.
> >>
> >> and keep the current conditions otherwise.
> >>
> >> Resolution 2: Leave as is.
> >>
> > I would suggest for the first implementation, leave it as is currently.
> > Then it would be great if you could create a RFC patch showing the
> > difference
> > of behavior on existing tests.
>
> Okay, I'll first create the first implementation, then (much) later the
> RFC.
>
Does not need to be that much later. Or you will have other obligations,
and will not be able to continue working on mdspan. This is fine of course,
and we are very grateful for your contributions. Just asking if I should
book
some time for review (if possible).
>
> >
> >>
> >> [3]: https://eel.is/c++draft/mdspan.layout.leftpad.cons#17
> >>
> >>
> >> 3. Generic Observation: Missing CTADs
> >>
> >> There's no deduction guides for mappings, e.g.:
> >>
> >> std::layout_left::mapping(std::extents<int, 2>{});
> >> std::layout_left_padded<1>::mapping m(ml); # Ugly error.
> >>
> > This seems like a feature request more than wording issue,
> > you could send the note to the paper authors of layouts. I find them very
> > responsive.
> >
>
> Definitely a feature request. I'll try if I can implement the deduction
> guide (I failed on my first try due to how mapping is nested inside another
> struct and gave up).
>
> >>
> >>
> >> Kind regards,
> >> Luc
> >>
> >>
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250821/45f5843b/attachment-0001.htm>
More information about the Libstdc++
mailing list