<valarray>: Implement copies for slice_array, gslice_array, mask_array, and indirect_array

[AlexGuteniev]

Resolves #940

[AlexGuteniev]

I think it is ready for initial review.

[AlexGuteniev]

I have a question on assignment operator with mistmatched size.

First, consider [valarray.assign]/1

valarray& operator=(const valarray& v);

Effects: Each element of the *this array is assigned the value of the corresponding element of v. If the length of v is not equal to the length of *this, resizes *this to make the two arrays the same length, as if by calling resize(v.size()), before performing the assignment.

(cppreference says it was UB before C++11)

Then [valarray.assign]/10

valarray& operator=(const slice_array<T>&);
valarray& operator=(const gslice_array<T>&);
valarray& operator=(const mask_array<T>&);
valarray& operator=(const indirect_array<T>&);

Preconditions: The length of the array to which the argument refers equals size(). The value of an element in the left-hand side of a valarray assignment operator does not depend on the value of another element in that left-hand side.

Note that precondition is somewhat ambiguous. The length of the array to which the argument refers could be both size of underlying array or size of resulting slice.

It seems that the later should be assumed, since the former does not make sense in context of [valarray.assign]/11:

These operators allow the results of a generalized subscripting operation to be assigned directly to a valarray.
Which I think implies that the view is applied only to the right-hand side. Also it matches with the current implementation,

Now, [slice.arr.assign], [gslice.array.assign], [mask.array.assign], [indirect.array.assign]:

void operator=(const valarray<T>&) const;
const slice_array& operator=(const slice_array&) const;

These assignment operators have reference semantics, assigning the values of the argument array elements to selected elements of the valarray<T> object to which the slice_­array object refers.

And that's it. Nothing about different destination size and other size.

---

So here's a comparison of what the Standard says and what implementation so far does:

Case	Operator	Standard says for size mismatch	Implementation does
a=a	valarray::operator=(const valarray& v);	resize *this (since C++11)	resize *this
a=v	valarray::operator=(const slice_array&);	precondition violation	resize *this
a=v	valarray::operator=(const gslice_array&);	precondition violation	resize *this
a=v	valarray::operator=(const mask_array&);	precondition violation	resize *this
a=v	valarray::operator=(const indirect_array&);	precondition violation	resize *this
v=a	slice_array::operator=(const valarray&);	nothing	assume *this size
v=a	gslice_array::operator=(const valarray&);	nothing	assume *this size
v=a	mask_array::operator=(const valarray&);	nothing	assume *this size
v=a	indirect_array::operator=(const valarray&);	nothing	assume *this size
v=v	slice_array::operator=(const slice_array&);	nothing	nothing until this PR
v=v	gslice_array::operator=(const gslice_array&);	nothing	nothing until this PR
v=v	mask_array::operator=(const mask_array&);	nothing	nothing until this PR
v=v	indirect_array::operator=(const indirect_array&);	nothing	nothing until this PR

In the current PR for v=v cases I'm treating size mismatch as precondition violation. And assuming *this size. It is consistent with pre-existing v=a cases. Out of two assumptions, assume *this is safer than assume that, since it will cause in worst case read buffer overrun, but not write buffer overrun.

Note that unlike a=a or a=v cases, I cannot assume indices of elements that do no exist in destination view, so resize *this is not an option.

The questions are:

• Is it a (known) Standard issue?
• What should I do with added assignment operators?
• Should precondition violation be handled somehow differently in existing a=v ?

[CaseyCarter]

Just some small things.

FWIW, I agree with your analysis of the specification or lack thereof. valarray is a neglected piece of the Standard Library that needs someone to pick it up and rework all of the wording. Until that happens I think it's basically the Wild West in here: we can do anything we like with all of the leeway the spec gives us.

[StephanTLavavej]

Thanks for fixing these high-priority customer-reported bugs! I'll go ahead and push a couple of #include directives to the test, and then this will be Ready To Merge! 🚀

[StephanTLavavej]

No change requested, just a stylistic comment - while we have tests that vary in their usage of using namespace std; and it's fine to std:: qualify everything, even when the namespace prevents ambiguity I think that naming things identically to Standard identifiers makes things unnecessarily hard to read/search. (Being able to search for all occurrences of slice_array in product and test code, and having few spurious results show up, is useful.) Any other names would be fine; e.g. slice_array_copy won't show up in a whole-word search.

Since it looks like this PR will be ready to merge with a couple of include fixups (yay!), I don't think it's worth renaming now, but I wanted to mention it.

[CaseyCarter]

Thanks for the bugfix! valarray isn't the most glamorous part of the STL, but we really should conform to C++11.