CODEWITHSUNDEEP
c++c++11accumulatestdarray
Amxx
Amxx

Reputation: 3070

invalid convertion in std::accumulate

I have a class representing a point in N dimensions with a min static function (minimum field by field)

template<typename T, std::size_t N>
class Point : public std::array<T,N>
{
public:
    template<typename... Args>
    Point(Args&&... args) : std::array<T,N>{{args...}} {}

    // ...

    static Point min(const Point&, const Point&) {
        // ...  
    }
};

Everything works well when I write

Point<float,3> a = {0.f, 1.f, 2.f};
Point<float,3> b = {2.f, 1.f, 0.f};
Point<float,3> c = Point<float,3>::min(a,b); // OK

But if I try to use std::accumulate over an array

Point<float,3> array[100] = ... ;
Point<float,3> min = std::accumulate(array, array+100, array[0], Point<float,3>::min); // Error

I get an error :

error: cannot convert ‘Point<float, 3ul>’ to ‘float’ in initialization
adimx::Point<T,N>::Point(Args&&... args) : std::array<T,N>{{args...}}

Is this an issue with std::accumulate implementation not being compatible with my constructor ?

Upvotes: 2

Views: 163

Answers (3)

Ron Tang
Ron Tang

Reputation: 1622

New solution:

template <bool J>
using disable_if=enable_if<!J>;

template <typename T,typename... Tail> struct getHead
{
    typedef typename decay<T>::type type;
};

template<typename... Args,typename = typename disable_if<is_same<typename getHead<Args...>::type,Point<T,N> >::value >::type >
    Point(Args&&... args) : std::array<T,N> {{args...}}
    {
        //...
    }

I believe this solution is perfect.We stop forwarding reference variable parameter constructor only when parameter is Point itself.Any other type still call forwarding reference variable parameter constructor. No matter Point<float,3> or Point<int,3> or Point<int,2> or Point<user-define,numx>it always OK.

There are at least three solutions for you to choose.

First, in order to avoid forwarding reference variable parameter constructor hijacked the copy constructor, so removal of this function, instead of

template<typename... Args>
        Point(Args... args) : std::array<T,N> {{args...}} {}//remove &&

this solution is to avoid problems instead of solving the problem.

Second, as TC said, stop forwarding reference variable parameter constructor hijacked constructor, as long as when all types all fit when enabled. This way is more complex, but also weaken the application scope of your template.

Third, as MSalters said, change array[0] to Point<float,3>(0,0,0),

Point<float,3> min = std::accumulate(array, array+100, Point<float,3>(0,0,0), Point<float,3>::min);

It is OK,but why?

As n4260 12.9 31.3 C++ Standrad says:

when a temporary class object that has not been bound to a reference (12.2) would be copied/moved to a class object with the same cv-unqualified type, the copy/move operation can be omitted by constructing the temporary object directly into the target of the omitted copy/move

So accumulate directly call Point constructor with three float ,doesn't call copy constructor.So doesn't call forwarding reference variable parameter constructor pass a Point Object as a parameter,this is where your compile error.

Drawback is that every time you use the accumulate function require incoming rvalues, and cannot be an lvalue.

Upvotes: 0

T.C.
T.C.

Reputation: 137315

Here's one way to constrain that constructor, so that it participates in overload resolution only if all arguments are implicitly convertible to float:

template<bool... > class bool_pack;
template<bool... b>
using all_true = std::is_same<bool_pack<true, b...>, bool_pack<b..., true>>;

template<typename... Args,
         class = typename std::enable_if<all_true<std::is_convertible<Args, float>::value...>::value>::type>
Point(Args&&... args) : std::array<T,N>{{args...}} {}

Upvotes: 4

user4630522
user4630522

Reputation: 31

Since the constructor wins by overload resolution, you can provide defaulted versions of the needed constructors:

Point(Point&) = default;
Point(const Point&) = default;
Point(Point&&) = default;
Point& operator=(const Point&) = default;

Upvotes: 3

Related Questions