Skip to content

Commit f3de4f0

Browse files
committed
New issue from Luc Grosheintz: "Missing move in mdspan layout mapping::operator()"
1 parent 64eaf56 commit f3de4f0

File tree

1 file changed

+281
-0
lines changed

1 file changed

+281
-0
lines changed

xml/issue4314.xml

Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
<?xml version='1.0' encoding='utf-8' standalone='no'?>
2+
<!DOCTYPE issue SYSTEM "lwg-issue.dtd">
3+
4+
<issue num="4314" status="New">
5+
<title>Missing move in `mdspan` layout `mapping::operator()`</title>
6+
<section>
7+
<sref ref="[mdspan.layout]"/>
8+
</section>
9+
<submitter>Luc Grosheintz</submitter>
10+
<date>13 Aug 2025</date>
11+
<priority>99</priority>
12+
13+
<discussion>
14+
<p>
15+
Numerous template classes in <tt>&lt;mdspan&gt;</tt> have template parameter `IndexType`.
16+
While this template parameter is restricted to be a signed or unsigned integer,
17+
these classes often accept user-defined classes that convert to `IndexType`.
18+
<p/>
19+
They're either passed as an array/span of `OtherIndexType`; or as a template parameter pack.
20+
When passed as a template parameter pack, the common pattern is
21+
</p>
22+
<blockquote><pre>
23+
template&lt;class... OtherIndexTypes&gt;
24+
requires std::is_convertible_v&lt;OtherIndexTypes, IndexType> &amp;&amp; ...
25+
void <i>dummy</i>(OtherIndexTypes... indices)
26+
{
27+
<i>something</i>(static_cast&lt;IndexType&gt;(std::move(indices))...);
28+
}
29+
</pre></blockquote>
30+
<p>
31+
This pattern allows passing in objects that convert to IndexType only as
32+
an rvalue reference, e.g.
33+
</p>
34+
<blockquote><pre>
35+
class RValueInt
36+
{
37+
constexpr
38+
operator int() &amp;&amp; noexcept
39+
{ return m_int; }
40+
41+
private:
42+
int m_int;
43+
};
44+
</pre></blockquote>
45+
<p>
46+
This pattern can be found in:
47+
</p>
48+
<ul>
49+
<li><p>a ctor of `extents`,</p></li>
50+
<li><p>a ctor of `mdspan`,</p></li>
51+
<li><p>`mdspan::operator[]`.</p></li>
52+
</ul>
53+
<p>
54+
The five standardized layout mappings use a different pattern in their
55+
operator(), namely,
56+
</p>
57+
<blockquote><pre>
58+
static_cast&lt;IndexType&gt;(indices)...
59+
</pre></blockquote>
60+
<p>
61+
This prevents the passing in objects of type `RValueInt`, because the
62+
conversion isn't happening from an rvalue reference. This is addressed by
63+
Items 1 - 5 in the Proposed Resolution.
64+
<p/>
65+
A different pattern can be found a ctor for `layout_{left,right}_padded`.
66+
Namely, directly passing an object of type `OtherIndexType` to
67+
<tt><i>LEAST-MULTIPLE-AT-LEAST</i></tt>. This is addressed in Items 6 and 7
68+
in the Proposed Resolution.
69+
<p/>
70+
This inconsistency was noticed while fixing
71+
<a href="https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121061">PR121061</a>
72+
and these changes have been applied to all layout mappings implemented in libstdc++.
73+
</p>
74+
</discussion>
75+
76+
<resolution>
77+
<p>
78+
This wording is relative to <paper num="N5014"/>.
79+
</p>
80+
81+
<ol>
82+
83+
<li><p>Modify <sref ref="[mdspan.layout.left.obs]"/> as indicated:</p>
84+
85+
<blockquote>
86+
<pre>
87+
template&lt;class... Indices&gt;
88+
constexpr index_type operator()(Indices... i) const noexcept;
89+
</pre>
90+
<blockquote>
91+
<p>
92+
-2- <i>Constraints</i>: [&hellip;]
93+
<p/>
94+
-3- <i>Preconditions</i>: [&hellip;]
95+
<p/>
96+
-4- <i>Effects</i>: Let `P` be a parameter pack such that
97+
</p>
98+
<blockquote><pre>
99+
is_same_v&lt;index_sequence_for&lt;Indices...&gt;, index_sequence&lt;P...&gt;&gt;
100+
</pre></blockquote>
101+
<p>
102+
is `true`. Equivalent to:
103+
</p>
104+
<blockquote><pre>
105+
return ((static_cast&lt;index_type&gt;(<ins>std::move(</ins>i<ins>)</ins>) * stride(P)) + ... + 0);
106+
</pre></blockquote>
107+
</blockquote>
108+
</blockquote>
109+
110+
</li>
111+
112+
<li><p>Modify <sref ref="[mdspan.layout.right.obs]"/> as indicated:</p>
113+
114+
<blockquote>
115+
<pre>
116+
template&lt;class... Indices&gt;
117+
constexpr index_type operator()(Indices... i) const noexcept;
118+
</pre>
119+
<blockquote>
120+
<p>
121+
-2- <i>Constraints</i>: [&hellip;]
122+
<p/>
123+
-3- <i>Preconditions</i>: [&hellip;]
124+
<p/>
125+
-4- <i>Effects</i>: Let `P` be a parameter pack such that
126+
</p>
127+
<blockquote><pre>
128+
is_same_v&lt;index_sequence_for&lt;Indices...&gt;, index_sequence&lt;P...&gt;&gt;
129+
</pre></blockquote>
130+
<p>
131+
is `true`. Equivalent to:
132+
</p>
133+
<blockquote><pre>
134+
return ((static_cast&lt;index_type&gt;(<ins>std::move(</ins>i<ins>)</ins>) * stride(P)) + ... + 0);
135+
</pre></blockquote>
136+
</blockquote>
137+
</blockquote>
138+
139+
</li>
140+
141+
<li><p>Modify <sref ref="[mdspan.layout.stride.obs]"/> as indicated:</p>
142+
143+
<blockquote>
144+
<pre>
145+
template&lt;class... Indices&gt;
146+
constexpr index_type operator()(Indices... i) const noexcept;
147+
</pre>
148+
<blockquote>
149+
<p>
150+
-2- <i>Constraints</i>: [&hellip;]
151+
<p/>
152+
-3- <i>Preconditions</i>: [&hellip;]
153+
<p/>
154+
-4- <i>Effects</i>: Let `P` be a parameter pack such that
155+
</p>
156+
<blockquote><pre>
157+
is_same_v&lt;index_sequence_for&lt;Indices...&gt;, index_sequence&lt;P...&gt;&gt;
158+
</pre></blockquote>
159+
<p>
160+
is `true`. Equivalent to:
161+
</p>
162+
<blockquote><pre>
163+
return ((static_cast&lt;index_type&gt;(<ins>std::move(</ins>i<ins>)</ins>) * stride(P)) + ... + 0);
164+
</pre></blockquote>
165+
</blockquote>
166+
</blockquote>
167+
168+
</li>
169+
170+
<li><p>Modify <sref ref="[mdspan.layout.leftpad.obs]"/> as indicated:</p>
171+
172+
<blockquote>
173+
<pre>
174+
template&lt;class... Indices&gt;
175+
constexpr index_type operator()(Indices... idxs) const noexcept;
176+
</pre>
177+
<blockquote>
178+
<p>
179+
-3- <i>Constraints</i>: [&hellip;]
180+
<p/>
181+
-4- <i>Preconditions</i>: [&hellip;]
182+
<p/>
183+
-5- <i>Returns</i>: <tt>((static_cast&lt;index_type&gt;(<ins>std::move(</ins>idxs<ins>)</ins>) * stride(P_rank)) + ... + 0)</tt>.
184+
</p>
185+
</blockquote>
186+
</blockquote>
187+
188+
</li>
189+
190+
<li><p>Modify <sref ref="[mdspan.layout.rightpad.obs]"/> as indicated:</p>
191+
192+
<blockquote>
193+
<pre>
194+
template&lt;class... Indices&gt;
195+
constexpr index_type operator()(Indices... idxs) const noexcept;
196+
</pre>
197+
<blockquote>
198+
<p>
199+
-3- <i>Constraints</i>: [&hellip;]
200+
<p/>
201+
-4- <i>Preconditions</i>: [&hellip;]
202+
<p/>
203+
-5- <i>Returns</i>: <tt>((static_cast&lt;index_type&gt;(<ins>std::move(</ins>idxs<ins>)</ins>) * stride(P_rank)) + ... + 0)</tt>.
204+
</p>
205+
</blockquote>
206+
</blockquote>
207+
208+
</li>
209+
210+
<li><p>Modify <sref ref="[mdspan.layout.leftpad.cons]"/> as indicated:</p>
211+
212+
<blockquote>
213+
<pre>
214+
template&lt;class OtherIndexType&gt;
215+
constexpr mapping(const extents_type&amp; ext, OtherIndexType pad<ins>ding</ins>);
216+
</pre>
217+
<blockquote>
218+
<p>
219+
<ins><ins>Let <tt>pad = static_cast&lt;index_type&gt;(std::move(padding))</tt>.</ins></ins>
220+
<p/>
221+
-3- <i>Constraints</i>: [&hellip;]
222+
<p/>
223+
-4- <i>Preconditions</i>:
224+
</p>
225+
<ol style="list-style-type: none">
226+
<li><p>(4.1) &mdash; <tt>pad<ins>ding</ins></tt> is representable as a value of type `index_type`.</p></li>
227+
<li><p>(4.2) &mdash; <del><tt>extents_type::<i>index-cast</i>(pad)</tt></del><ins>pad</ins> is greater than zero.</p></li>
228+
<li><p>(4.3) &mdash; If <tt><i>rank_</i></tt> is greater than one, then
229+
<tt><i>LEAST-MULTIPLE-AT-LEAST</i>(pad, ext.extent(0))</tt> is representable as a value of type `index_type`.</p></li>
230+
<li><p>(4.4) &mdash; If <tt><i>rank_</i></tt> is greater than one, then the product of
231+
<tt><i>LEAST-MULTIPLE-AT-LEAST</i>(pad, ext.extent(0))</tt> and all values <tt>ext.extent(<i>k</i>)</tt> with
232+
<tt><i>k</i></tt> in the range of <tt>[1, <i>rank_</i>)</tt> is representable as a value of type `index_type`.</p></li>
233+
<li><p>(4.5) &mdash; If `padding_value` is not equal to `dynamic_extent`, `padding_value` equals
234+
<tt><del>extents_type::<i>index-cast</i>(pad)</del><ins>pad</ins></tt>.</p></li>
235+
</ol>
236+
<p>
237+
-5- <i>Effects</i>: [&hellip;]
238+
</p>
239+
</blockquote>
240+
</blockquote>
241+
242+
</li>
243+
244+
<li><p>Modify <sref ref="[mdspan.layout.rightpad.cons]"/> as indicated:</p>
245+
246+
<blockquote>
247+
<pre>
248+
template&lt;class OtherIndexType&gt;
249+
constexpr mapping(const extents_type&amp; ext, OtherIndexType pad<ins>ding</ins>);
250+
</pre>
251+
<blockquote>
252+
<p>
253+
<ins><ins>Let <tt>pad = static_cast&lt;index_type&gt;(std::move(padding))</tt>.</ins></ins>
254+
<p/>
255+
-3- <i>Constraints</i>: [&hellip;]
256+
<p/>
257+
-4- <i>Preconditions</i>:
258+
</p>
259+
<ol style="list-style-type: none">
260+
<li><p>(4.1) &mdash; <tt>pad<ins>ding</ins></tt> is representable as a value of type `index_type`.</p></li>
261+
<li><p>(4.2) &mdash; <del><tt>extents_type::<i>index-cast</i>(pad)</tt></del><ins>pad</ins> is greater than zero.</p></li>
262+
<li><p>(4.3) &mdash; If <tt><i>rank_</i></tt> is greater than one, then
263+
<tt><i>LEAST-MULTIPLE-AT-LEAST</i>(pad, ext.extent(<i>rank_</i> - 1))</tt> is representable as a value of type `index_type`.</p></li>
264+
<li><p>(4.4) &mdash; If <tt><i>rank_</i></tt> is greater than one, then the product of
265+
<tt><i>LEAST-MULTIPLE-AT-LEAST</i>(pad, ext.extent(<i>rank_</i> - 1))</tt> and all values <tt>ext.extent(<i>k</i>)</tt> with
266+
<tt><i>k</i></tt> in the range of <tt>[1, <i>rank_</i> - 1)</tt> is representable as a value of type `index_type`.</p></li>
267+
<li><p>(4.5) &mdash; If `padding_value` is not equal to `dynamic_extent`, `padding_value` equals
268+
<tt><del>extents_type::<i>index-cast</i>(pad)</del><ins>pad</ins></tt>.</p></li>
269+
</ol>
270+
<p>
271+
-5- <i>Effects</i>: [&hellip;]
272+
</p>
273+
</blockquote>
274+
</blockquote>
275+
276+
</li>
277+
278+
</ol>
279+
</resolution>
280+
281+
</issue>

0 commit comments

Comments
 (0)