Skip to content

Commit 978bf4f

Browse files
llodalloda
committed
Fix bad forwarding in array constructors
Not only was it unnecessary, it also caused conflicts by (auto && x) grabbing (Container & x), which required extra overloads, etc. * ra/big.hh (ViewBig): Remove special operator=(X && x), just reuse macro. (Container): Remove (Container &) constructor and operator=. * ra/small.hh (ViewSmall): Remove special operator=(X && x), just reuse macro. (SmallArray): Remove (X && x) constructor for non-T.
1 parent cd5dc4b commit 978bf4f

File tree

2 files changed

+63
-89
lines changed

2 files changed

+63
-89
lines changed

‎ra/big.hh‎

Lines changed: 52 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ struct ViewBig
6767

6868
constexpr ViewBig(): cp() {} // FIXME used by Container constructors
6969
constexpr ViewBig(Dimv const & dimv_, P cp_): dimv(dimv_), cp(cp_) {} // [ra36]
70-
constexpr ViewBig(auto && s, P cp_): cp(cp_)
70+
constexpr ViewBig(auto const & s, P cp_): cp(cp_)
7171
{
7272
ra::resize(dimv, ra::size(s)); // [ra37]
7373
if constexpr (std::is_convertible_v<value_t<decltype(s)>, Dim>) {
@@ -77,22 +77,8 @@ struct ViewBig
7777
}
7878
}
7979
constexpr ViewBig(std::initializer_list<dim_t> s, P cp_): ViewBig(start(s), cp_) {}
80-
// cf RA_ASSIGNOPS_SELF [ra38] [ra34]
81-
ViewBig const & operator=(ViewBig const & x) const { start(*this) = x; return *this; }
82-
constexpr ViewBig(ViewBig const &) = default;
83-
84-
template <class X> requires (!std::is_same_v<std::decay_t<X>, T>)
85-
constexpr ViewBig const & operator=(X && x) const { start(*this) = x; return *this; }
86-
#define ASSIGNOPS(OP) \
87-
constexpr ViewBig const & operator OP (auto && x) const { start(*this) OP x; return *this; }
88-
FOR_EACH(ASSIGNOPS, *=, +=, -=, /=)
89-
#undef ASSIGNOPS
90-
// if T isn't is_scalar [ra44]
91-
constexpr ViewBig const &
92-
operator=(T const & t) const
93-
{
94-
start(*this) = ra::scalar(t); return *this;
95-
}
80+
// T not is_scalar [ra44]
81+
constexpr ViewBig const & operator=(T const & t) const { start(*this) = ra::scalar(t); return *this; }
9682
// row-major ravel braces
9783
constexpr ViewBig const &
9884
operator=(std::initializer_list<T> const x) const requires (1!=RANK)
@@ -114,6 +100,13 @@ struct ViewBig
114100
}
115101
FOR_EACH(RA_BRACES_ANY, 2, 3, 4);
116102
#undef RA_BRACES_ANY
103+
constexpr ViewBig(ViewBig const &) = default;
104+
// cf RA_ASSIGNOPS_SELF [ra38] [ra34]
105+
ViewBig const & operator=(ViewBig const & x) const { start(*this) = x; return *this; }
106+
#define ASSIGNOPS(OP) \
107+
constexpr ViewBig const & operator OP (auto const & x) const { start(*this) OP x; return *this; }
108+
FOR_EACH(ASSIGNOPS, =, *=, +=, -=, /=)
109+
#undef ASSIGNOPS
117110

118111
constexpr dim_t
119112
select(Dim * dim, int k, dim_t i) const
@@ -188,14 +181,14 @@ struct ViewBig
188181
operator[](this auto && self, auto && ... i) { return RA_FW(self)(RA_FW(i) ...); }
189182

190183
constexpr decltype(auto)
191-
at(auto && i) const
184+
at(auto const & i) const
192185
{
193186
// can't say 'frame rank 0' so -size wouldn't work. FIXME What about ra::len
194187
constexpr rank_t crank = rank_diff(RANK, ra::size_s(i));
195188
if constexpr (ANY==crank) {
196-
return iter(rank()-ra::size(i)).at(RA_FW(i));
189+
return iter(rank()-ra::size(i)).at(i);
197190
} else {
198-
return iter<crank>().at(RA_FW(i));
191+
return iter<crank>().at(i);
199192
}
200193
}
201194

@@ -266,59 +259,43 @@ template <class Store, rank_t RANK>
266259
struct Container: public ViewBig<typename storage_traits<Store>::T *, RANK>
267260
{
268261
Store store;
262+
constexpr auto data(this auto && self) { return self.view().data(); }
269263
using T = typename storage_traits<Store>::T;
270264
using View = ViewBig<T *, RANK>;
271265
using ViewConst = ViewBig<T const *, RANK>;
272266
constexpr ViewConst const & view() const { return *this; }
273267
constexpr View & view() { return *this; }
274268
using View::size, View::rank, View::dimv, View::cp;
275269

276-
// Needed to set cp. FIXME Remove duplication as in SmallArray.
277-
Container(Container && w): store(std::move(w.store))
270+
// A(shape 2 3) = A-type [1 2 3] initializes, so it doesn't behave as A(shape 2 3) = not-A-type [1 2 3] which uses View::operator=. This is used by operator>>(std::istream &, Container &). See test/ownership.cc [ra20].
271+
// TODO don't require copyable T in constructors, see fill1. That requires operator= to initialize, not update.
272+
constexpr Container & operator=(Container && w)
278273
{
274+
store = std::move(w.store);
279275
dimv = std::move(w.dimv);
280276
cp = storage_traits<Store>::data(store);
277+
return *this;
281278
}
282-
Container(Container const & w): store(w.store)
279+
constexpr Container & operator=(Container const & w)
283280
{
281+
store = w.store;
284282
dimv = w.dimv;
285283
cp = storage_traits<Store>::data(store);
284+
return *this;
286285
}
287-
Container(Container & w): Container(std::as_const(w)) {}
288-
289-
// A(shape 2 3) = A-type [1 2 3] initializes, so it doesn't behave as A(shape 2 3) = not-A-type [1 2 3] which uses View::operator=. This is used by operator>>(std::istream &, Container &). See test/ownership.cc [ra20].
290-
// TODO don't require copyable T in constructors, see fill1. That requires operator= to initialize, not update.
291-
Container & operator=(Container && w)
286+
constexpr Container(Container && w): store(std::move(w.store))
292287
{
293-
store = std::move(w.store);
294288
dimv = std::move(w.dimv);
295289
cp = storage_traits<Store>::data(store);
296-
return *this;
297290
}
298-
Container & operator=(Container const & w)
291+
constexpr Container(Container const & w): store(w.store)
299292
{
300-
store = w.store;
301293
dimv = w.dimv;
302294
cp = storage_traits<Store>::data(store);
303-
return *this;
304295
}
305-
Container & operator=(Container & w) { return *this = std::as_const(w); }
306-
307-
constexpr decltype(auto) back(this auto && self) { return RA_FW(self).view().back(); }
308-
constexpr auto data(this auto && self) { return self.view().data(); }
309-
constexpr decltype(auto) operator()(this auto && self, auto && ... a) { return RA_FW(self).view()(RA_FW(a) ...); }
310-
constexpr decltype(auto) operator[](this auto && self, auto && ... a) { return RA_FW(self).view()(RA_FW(a) ...); }
311-
constexpr decltype(auto) at(this auto && self, auto && i) { return RA_FW(self).view().at(RA_FW(i)); }
312-
// always compact/row-major, so STL-like iterators can be raw pointers.
313-
constexpr auto begin(this auto && self) { assert(is_c_order(self.view())); return self.view().data(); }
314-
constexpr auto end(this auto && self) { return self.view().data()+self.size(); }
315-
template <rank_t c=0> constexpr auto iter(this auto && self) { if constexpr (1==RANK && 0==c) { return ptr(self.data(), self.size()); } else { return RA_FW(self).view().template iter<c>(); } }
316-
constexpr operator T & () { return view(); }
317-
constexpr operator T const & () const { return view(); }
318-
319-
// these need repeating bc of various reasons; constness, no TAD for initializer_list, or constructor overriding.
296+
// repeated bc of constness re. ViewBig
320297
#define ASSIGNOPS(OP) \
321-
constexpr Container & operator OP (auto && x) { view() OP x; return *this; }
298+
constexpr Container & operator OP (auto const & x) { view() OP x; return *this; }
322299
FOR_EACH(ASSIGNOPS, =, *=, +=, -=, /=)
323300
#undef ASSIGNOPS
324301
constexpr Container & operator=(std::initializer_list<T> const x) requires (1!=RANK) { view() = x; return *this; }
@@ -329,7 +306,7 @@ struct Container: public ViewBig<typename storage_traits<Store>::T *, RANK>
329306
#undef RA_BRACES_ANY
330307

331308
void
332-
init(auto && s) requires (1==rank_s(s) || ANY==rank_s(s))
309+
init(auto const & s) requires (1==rank_s(s) || ANY==rank_s(s))
333310
{
334311
static_assert(!std::is_convertible_v<value_t<decltype(s)>, Dim>);
335312
RA_CK(1==ra::rank(s), "Rank mismatch for init shape.");
@@ -340,18 +317,18 @@ struct Container: public ViewBig<typename storage_traits<Store>::T *, RANK>
340317
}
341318
void init(dim_t s) { init(std::array {s}); } // scalar allowed as shape if rank is 1.
342319

343-
// provided so that {} calls shape_arg constructor below.
320+
// provided so that {} calls sharg constructor below.
344321
Container() requires (ANY==RANK): View({ Dim {0, 1} }, nullptr) {}
345322
Container() requires (ANY!=RANK && 0!=RANK): View(typename View::Dimv(Dim {0, 1}), nullptr) {}
346323
Container() requires (0==RANK): Container({}, none) {}
347324

348-
using shape_arg = decltype(shape(std::declval<View>().iter()));
349-
// shape_arg overloads handle {...} arguments. Size check is at conversion if shape_arg is Small.
350-
Container(shape_arg const & s, none_t) { init(s); }
351-
Container(shape_arg const & s, auto && x): Container(s, none) { view() = x; }
352-
Container(shape_arg const & s, braces<T, RANK> x) requires (1==RANK): Container(s, none) { view() = x; }
325+
using sharg = decltype(shape(std::declval<View>().iter()));
326+
// sharg overloads handle {...} arguments. Size check is at conversion if sharg is Small.
327+
Container(sharg const & s, none_t) { init(s); }
328+
Container(sharg const & s, auto && x): Container(s, none) { view() = x; }
329+
Container(sharg const & s, braces<T, RANK> x) requires (1==RANK): Container(s, none) { view() = x; }
353330

354-
Container(auto && x): Container(ra::shape(x), none) { view() = x; }
331+
Container(auto const & x): Container(ra::shape(x), none) { view() = x; }
355332
Container(braces<T, RANK> x) requires (RANK!=ANY): Container(braces_shape<T, RANK>(x), none) { view() = x; }
356333
#define RA_BRACES_ANY(N) \
357334
Container(braces<T, N> x) requires (RANK==ANY): Container(braces_shape<T, N>(x), none) { view() = x; }
@@ -369,19 +346,14 @@ struct Container: public ViewBig<typename storage_traits<Store>::T *, RANK>
369346
// shape + row-major ravel.
370347
// FIXME explicit it-is-ravel mark. Also iter<n> initializers.
371348
// FIXME regular (no need for fill1) for ANY if rank is 1.
372-
Container(shape_arg const & s, std::initializer_list<T> x) requires (1!=RANK)
373-
: Container(s, none) { fill1(x.begin(), x.size()); }
374-
// FIXME remove
375-
Container(shape_arg const & s, auto * p)
376-
: Container(s, none) { fill1(p, size()); } // FIXME fake check
377-
// FIXME remove
378-
Container(shape_arg const & s, auto && pbegin, dim_t psize)
379-
: Container(s, none) { fill1(RA_FW(pbegin), psize); }
380-
381-
// for shape arguments that doesn't convert implicitly to shape_arg
382-
Container(auto && s, none_t) { init(RA_FW(s)); }
383-
Container(auto && s, auto && x): Container(RA_FW(s), none) { view() = x; }
384-
Container(auto && s, std::initializer_list<T> x): Container(RA_FW(s), none) { fill1(x.begin(), x.size()); }
349+
Container(sharg const & s, std::initializer_list<T> x) requires (1!=RANK): Container(s, none) { fill1(x.begin(), x.size()); }
350+
// FIXME remove these two
351+
Container(sharg const & s, auto * p): Container(s, none) { fill1(p, size()); } // FIXME fake check
352+
Container(sharg const & s, auto && pbegin, dim_t psize): Container(s, none) { fill1(RA_FW(pbegin), psize); }
353+
// for shape arguments that doesn't convert implicitly to sharg
354+
Container(auto const & s, none_t) { init(s); }
355+
Container(auto const & s, auto const & x): Container(s, none) { view() = x; }
356+
Container(auto const & s, std::initializer_list<T> x): Container(s, none) { fill1(x.begin(), x.size()); }
385357

386358
// resize first axis or full shape. Only for some kinds of store.
387359
void resize(dim_t const s)
@@ -433,6 +405,16 @@ struct Container: public ViewBig<typename storage_traits<Store>::T *, RANK>
433405
--dimv[0].len;
434406
store.pop_back();
435407
}
408+
constexpr decltype(auto) back(this auto && self) { return RA_FW(self).view().back(); }
409+
constexpr decltype(auto) operator()(this auto && self, auto && ... a) { return RA_FW(self).view()(RA_FW(a) ...); }
410+
constexpr decltype(auto) operator[](this auto && self, auto && ... a) { return RA_FW(self).view()(RA_FW(a) ...); }
411+
constexpr decltype(auto) at(this auto && self, auto const & i) { return RA_FW(self).view().at(i); }
412+
// always compact/row-major, so STL-like iterators can be raw pointers.
413+
constexpr auto begin(this auto && self) { assert(is_c_order(self.view())); return self.view().data(); }
414+
constexpr auto end(this auto && self) { return self.view().data()+self.size(); }
415+
template <rank_t c=0> constexpr auto iter(this auto && self) { if constexpr (1==RANK && 0==c) { return ptr(self.data(), self.size()); } else { return RA_FW(self).view().template iter<c>(); } }
416+
constexpr operator T & () { return view(); }
417+
constexpr operator T const & () const { return view(); }
436418
};
437419

438420
// rely on std::swap; else ambiguous

‎ra/small.hh‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ constexpr bool
5252
is_c_order(auto const & v, bool step1=true) { return is_c_order_dimv(v.dimv, step1); }
5353

5454
constexpr dim_t
55-
filldim(auto && shape, auto & dimv)
55+
filldim(auto const & shape, auto & dimv)
5656
{
5757
map(&Dim::len, dimv) = shape;
5858
dim_t s = 1;
@@ -198,7 +198,7 @@ indexer(Q const & q, P const & pp)
198198
}
199199
return c;
200200
} else {
201-
auto loop = [&](this auto && loop, auto && k, dim_t c)
201+
auto loop = [&](this auto && loop, auto k, dim_t c)
202202
{
203203
if constexpr (k==rank_s(q)) {
204204
return c;
@@ -397,14 +397,12 @@ struct ViewSmall
397397
{
398398
ra::iter<-1>(*this) = x; return *this;
399399
}
400+
constexpr ViewSmall(ViewSmall const & s) = default;
400401
// cf RA_ASSIGNOPS_SELF [ra38] [ra34]
401402
ViewSmall const & operator=(ViewSmall const & x) const { start(*this) = x; return *this; }
402-
constexpr ViewSmall(ViewSmall const & s) = default;
403-
template <class X> requires (!std::is_same_v<std::decay_t<X>, T>)
404-
constexpr ViewSmall const & operator=(X && x) const { start(*this) = x; return *this; }
405403
#define ASSIGNOPS(OP) \
406-
constexpr ViewSmall const & operator OP(auto && x) const { start(*this) OP x; return *this; }
407-
FOR_EACH(ASSIGNOPS, *=, +=, -=, /=)
404+
constexpr ViewSmall const & operator OP(auto const & x) const { start(*this) OP x; return *this; }
405+
FOR_EACH(ASSIGNOPS, =, *=, +=, -=, /=)
408406
#undef ASSIGNOPS
409407

410408
template <int k>
@@ -458,12 +456,12 @@ struct ViewSmall
458456
operator[](this auto && self, auto && ... i) { return RA_FW(self)(RA_FW(i) ...); }
459457

460458
constexpr decltype(auto)
461-
at(auto && i) const
459+
at(auto const & i) const
462460
{
463461
// can't say 'frame rank 0' so -size wouldn't work. FIXME What about ra::len
464462
constexpr rank_t crank = rank_diff(rank(), ra::size_s(i));
465463
static_assert(crank>=0); // to make out the output type
466-
return iter<crank>().at(RA_FW(i));
464+
return iter<crank>().at(i);
467465
}
468466
// maybe remove if ic becomes easier to use
469467
template <int s, int o=0> constexpr auto as() const { return operator()(ra::iota(ra::ic<s>, o)); }
@@ -509,7 +507,6 @@ SmallArray<T, Dimv, std::tuple<nested_args ...>>
509507
T cp[size()];
510508
[[no_unique_address]] struct {} prevent_zero_size; // or reuse std::array
511509
constexpr auto data(this auto && self) { return self.cp; }
512-
513510
using View = ViewSmall<T *, Dimv>;
514511
using ViewConst = ViewSmall<T const *, Dimv>;
515512
constexpr View view() { return View(data()); }
@@ -519,7 +516,7 @@ SmallArray<T, Dimv, std::tuple<nested_args ...>>
519516

520517
constexpr SmallArray() {}
521518
constexpr SmallArray(ra::none_t) {}
522-
// if T isn't is_scalar [ra44]
519+
// T not is_scalar [ra44]
523520
constexpr SmallArray(T const & t) { std::ranges::fill(cp, t); }
524521
// row-major ravel braces
525522
constexpr SmallArray(T const & x0, std::convertible_to<T> auto const & ... x)
@@ -533,21 +530,16 @@ SmallArray<T, Dimv, std::tuple<nested_args ...>>
533530
{
534531
view() = { x ... };
535532
}
536-
// X && x makes this a better match than nested_args ... for 1 argument.
537-
template <class X> requires (!std::is_same_v<std::decay_t<X>, T>)
538-
constexpr SmallArray(X && x)
539-
{
540-
view() = RA_FW(x);
541-
}
533+
constexpr SmallArray(auto const & x) { view() = x; }
542534
#define ASSIGNOPS(OP) \
543-
constexpr SmallArray & operator OP(auto && x) { view() OP RA_FW(x); return *this; }
535+
constexpr SmallArray & operator OP(auto const & x) { view() OP x; return *this; }
544536
FOR_EACH(ASSIGNOPS, =, *=, +=, -=, /=)
545537
#undef ASSIGNOPS
546538

547539
constexpr decltype(auto) back(this auto && self) { return RA_FW(self).view().back(); }
548540
constexpr decltype(auto) operator()(this auto && self, auto && ... a) { return RA_FW(self).view()(RA_FW(a) ...); }
549541
constexpr decltype(auto) operator[](this auto && self, auto && ... a) { return RA_FW(self).view()(RA_FW(a) ...); }
550-
constexpr decltype(auto) at(this auto && self, auto && i) { return RA_FW(self).view().at(RA_FW(i)); }
542+
constexpr decltype(auto) at(this auto && self, auto const & i) { return RA_FW(self).view().at(i); }
551543
constexpr auto begin(this auto && self) { return self.view().begin(); }
552544
constexpr auto end(this auto && self) { return self.view().end(); }
553545
template <rank_t c=0> constexpr auto iter(this auto && self) { return RA_FW(self).view().template iter<c>(); }

0 commit comments

Comments
 (0)