Compile time error if brace-closed list is the wrong size for class constructor
Asked Answered
I

2

10

I'm trying to write a class based around mathematical vectors:

template <unsigned N> class Vector{
public:
    Vector() = default;
    Vector(std::initializer_list<double> li) { *this = li;}
    Vector& operator=(std::initializer_list<double>);

private:
    std::array<double, N> x = {}
}

template <unsigned N> inline Vector<N>& Vector<N>::operator=(std::initializer_list<double> li){
     if(N != li.size()) throw std::length_error("Attempt to initialise Vector with an initializer_list of different size.");
     std::copy(li.begin(), li.end(), x.begin());
     return *this;
}

I want to be able to write code like this;

Vector<3> a = {1,2,3};
a = {3,5,1};

It would be natural for a user to expect to write code like that, right? However I want compile-time errors to occur if I use the wrong-sized initializer list, much like std::array does.

 std::array<double, 3> a = {2,4,2,4} //compile time error
 Vector<3> a = {3,5,1,5} //run-time error as of right now

My first idea was to use std::array as the constructor/operator parameter so implicit conversions would occur and then the constructor would hijack, from std::array, the compile time errors. Except of course I could only write code like this:

Vector<3> a({2,3,2}); //fine
Vector<3> b = {2,4,2}; //error, requires two user-defined conversions (list -> array<double,3> -> Vector<3>) 

I thought maybe to use a Variadic member template:

template <typename... Args> Vector(Args... li): x({li...}){
    static_assert(sizeof...(li) == N);
}

It has to be typename... rather than double... because nontype parameters must be integral types. But then I run in to a narrowing conversion error

Vector<2> a = {3,2} //error: narrowing conversion of 'li#0' from 'int' to 'double' inside { } [-Wnarrowing]|
 //error: narrowing conversion of 'li#1' from 'int' to 'double' inside { } [-Wnarrowing]|

Presumably for violating [8.5.4]/7

A narrowing conversion is an implicit conversion

— from an integer type or unscoped enumeration type to a floating-point type, except where the source is a constant expression and the actual value after conversion will fit into the target type and will produce the original value when converted back to the original type, or

The parameters from expanding li... aren't constant expressions and hence produce the narrowing conversion error. As far as I'm aware it wouldn't even be possible to make function parameters as constant expressions (nor would it make much sense?). So I'm not sure how to carry on down that route. Obviously Vector<2> a = {2.,3.} works fine but this puts a burden on the user to remember only to supply floating-point literals.

Indemnity answered 25/11, 2015 at 20:39 Comment(7)
do you want to allow shorter lists, e.g. Vector<3> a = { 1,2 }; ?Steffi
afaik initializer list can normally only be used for initialization not for assignment, thus I would anser (the only question I could find) with no.Skinned
@Steffi I wasn't too sure. Would a user expect to be able to do that like they can with built in arrays or containers? I personally would like to force the list to be the exact same size.Indemnity
It seems like you've really got 2 questions, (1) how to enforce length for initialization, (2) how to enforce length for assignment operator. Is that right?Steffi
@Steffi Yeah, but maybe the solution to one hints at the solution for the other.Indemnity
Inside your variadic template thingy... Any reason this won't work: static_assert(sizeof...(args) - 1 < Size, "Boom");Colvin
Taking your quote to the letter, the "source" in your failing example is a constant expression... There's just an extra level of indirection...Ambulance
A
5

You can make your constructor a variadic template so that any condition can be used:

#include <array>
#include <cstddef>

template<typename T, std::size_t N>
class vector
{
public:
    vector(T&& value)
    : data{static_cast<T>(value)}
    {}
    template<typename U>
    vector(const vector<U,N>& v)
    {
      std::copy(begin(v.data), end(v.data),
                begin(data));
    }
    template<typename U>
    vector(const vector<U,N>& v)
    {
        std::copy(begin(v.data), end(v.data),
                  begin(data));
    }
    template<typename... U,
             typename = typename std::enable_if<sizeof...(U)-1>::type>
    vector(U&&... values)
    : data{static_cast<T>(values)...}
    {
        static_assert(sizeof...(values) == N, "wrong size");
    }
    std::array<T,N> data;
};

int main()
{
    vector<int, 3> v = {1,2,3};
    vector<double, 4> vv = {5,4,3,2};

    vv = {1,2,3,4};

    //vector<float, 3> vf = {1,2,3,4}; // fails to compile
    vector<float,3> vf = v;
}

Live example on coliru.

It gets you a custom error message, easily adaptable/extensible condition for failure, and gets rid of the "narrowing conversion" problem by effectively forwarding the initialization to the std::array initializer like you wanted to do in the first place. Oh, and you get assignment for free.

As @M.M mentions, this solution ruins copy construction, unfortunately. You can solve it by adding an enable_if on the variadic arguments "array" size as shown above. Of course you need to be careful to not ruin assignment/copy-construction and single-element vectors, which is remedied by adding two extra constructors for these special cases.

Ambulance answered 25/11, 2015 at 21:24 Comment(10)
Copy-construction doesn't seem to work with this solution, e.g. vector<int, 3> w(v);.Steffi
@Steffi right, I figured that out (see edit) by looking at your answer :/. It seems there's no obvious/elegant/simplistic way out of that. It's either one or the other.... Wait, I can't seem to get the copy constructor selected over the variadic template (and I thought non-template functions were selected over template functions :o)... coliru.stacked-crooked.com/a/cc994313c1291e30Ambulance
After template argument deduction we have as viable functions the non-template copy constructor Vector(const Vector&) and the variadic member template constructor Vector(U&&) but with U replaced by Vector& giving Vector(Vector&) by reference collapsing. Since Vector(Vector&) doesn't require low-level const conversion it is selected (non-template functions aren't selected if there is a template with a better match).Indemnity
A solution would be to use const U& or const U&& or even just U, I don't think it's important to carry on the const or lvalue/rvalue property of my arguments. I guess by using U&& and preserving const and lvalue/rvalue properties you require no conversions and risk that function becoming a better match in overload resolution than the ordinary copy-control.Indemnity
This needs an enable_if; as written it's too greedy. (It also creates an implicit conversion from everything.)Hawken
I'm forwarding greediness to the initialization of std::array, really, so "implicit conversion from everything" only works as long as the "everything" is compatible with the brace initialization of std::array and T. I think I fixed the shortcomings now, @M.M. Not sure if this solution is "neater" than yours. It does avoid the arg-expansion trick though.Ambulance
It's still constructible from every combination of two or more arguments as far as std::is_constructible is concerned. The member initializer list doesn't SFINAE.Hawken
@Hawken That would be a failing in the trait, as it clearly isn't constructible from everything. I think this version satisfies all concerns the problem presents. If not, I'll be glad to think on concrete improvements!Ambulance
@Hawken Something like typename = typename enable_if<is_convertible<U, double>::value>::type... along with typename = typename enable_if<sizeof...(U) == N>::type inside the template parameter list, but there seems to be a restriction on expanding packs declared in the same list ([14.1]/15 of N3337).Indemnity
@Indemnity That's not valid syntax. You want something along the lines of typename = typename enable_if<std::conjunction_v<std::is_convertible<U, double>...>>::type (conjunction_v is C++17 but readily implementable in C++14 (or C++11 with ::value instead of _v)).Hawken
S
5

This code seems to work, for both constructor and assignment-operator:

#include <array>

template<size_t N>
struct Vector
{
    Vector() = default;

    template<typename...Args>
    Vector(double d, Args... args)
    {
        static_assert(sizeof...(args) == N-1, "wrong args count");

        size_t idx = 0;
        auto vhelp = [&](double d) { x[idx++] = d; };
        vhelp(d);
        double tmp[] { (vhelp(args), 1.0)... };
    }

    Vector &operator=(Vector const &other) = default;

private:
    std::array<double, N> x = {};
};

int main()
{
    Vector<5> v = { 1,2,3,4,5 };
    v = { 3,4,5,6,7 };

    Vector<1> w = { 1,2 };  // error
}

The assignment operator works because the constructor is implicit, so v = bla attempts to convert bla to match the only definition of operator=.

I made the first argument double d instead of just using all variadic args, to avoid the issue where all-variadic-args constructor catches calls that were supposed to be copy-construction.

The line involving double tmp[] uses what I call the variadic template comma operator hack. This hack has many uses, but here it lets us avoid the narrowing-conversion issue that double tmp[] { args... }; has.

(TBH though, incorporating rubvenvb's idea and using double tmp[] { static_cast<double>(args)... }; would be simpler)

Steffi answered 25/11, 2015 at 21:28 Comment(4)
Nice solution. Although I'm not all too familiar with syntax when it comes to unpacking. It seems odd that just something like vhelp(args)... ; isn't also valid syntax and it is indeed necessary to create a temporary array (isn't unpacking just the same as writing vhelp(a1), vhelp(a2), vhelp(a3), ...?)Indemnity
@Indemnity unfortunately there is no syntax for directly calling a function once for each member of the pack; that's what's achieved by the comma operator hack. I guess there would be a technical reason why they didn't make f(args)...; be possibleSteffi
It can be done in certain contexts. Like passing the expansion to a function. coliru.stacked-crooked.com/a/3288f60d506227f7Indemnity
@Indemnity looks like another hack..:)Steffi

© 2022 - 2024 — McMap. All rights reserved.