0

I would like to understand where I am going wrong in trying to minimize the verbosity of my member functions template specialization. I get compilation errors when doing so rather arbitrarily. Here's the version that works, which hopefully will shed some light on what I am trying to achieve:

#include <iostream>
#include <type_traits>


typedef int i32;
template<class T>
struct rtvec
{
private:
    T* e;
    i32 d;

public:
    rtvec(i32 d) : d(d), e(new T[d]) {}
    //template<typename Args...>
    //rtvec()
    rtvec(const rtvec& in) : d(in.d), e(new T[in.d])
    {
        for (i32 i = 0; i < d; ++i)
            at(i) = in.at(i);
    }

    rtvec(rtvec<typename std::remove_pointer_t<T>>& in) : d(in.dim()), e(new T[in.dim()])
    {
        for (i32 i = 0; i < d; ++i)
            e[i] = &in.at(i);
    }
    ~rtvec() { delete[] e; }
    i32 dim() const { return d; }

    template<typename U=T, 
        typename std::enable_if_t<std::is_same_v<U,T>>* = nullptr,
        typename std::enable_if_t<!std::is_pointer_v<U>>* = nullptr>
    inline T& at(i32 i) 
    {
        return e[i];
    }

    template<typename U = T,
        typename std::enable_if_t<std::is_same_v<U, T>>* = nullptr,
        typename std::enable_if_t<std::is_pointer_v<U>>* = nullptr>
        inline typename std::remove_pointer_t<T>& at(i32 i)
    {
        return *e[i];
    }

};


int main()
{
    rtvec<float> v(2);
    v.at(0) = 1;
    v.at(1) = 2;
    rtvec<float*> p = v;
    p.at(0) = 5;
    std::cout << v.at(0) << " " << v.at(1) << "\n";
    return 0;
}

Basically I am trying to make a runtime variable dimensional vector class, which when instantiated with a pointer, can be used as a sort of reference to a vector of the same type (more precisely I have several arrays of each of the coordinates of a set of points and I want to use the "reference" vector to be able to work with those as if they were ordered the other way around in memory). When I try to simplify the code, however, by trying to remove what I perceive as an unnecessary typename U. I get the following compilation error in MSVC2017: std::enable_if_t<false,void>' : Failed to specialize alias template. Here's the less verbose version that I was aiming at achieving:

struct rtvec
{
private:
    T* e;
    i32 d;

public:
    rtvec(i32 d) : d(d), e(new T[d]) {}

    template<typename std::enable_if_t<!std::is_pointer_v<T>>* = nullptr>
    rtvec(const rtvec& in) : d(in.d), e(new T[in.d])
    {
        for (i32 i = 0; i < d; ++i)
            at(i) = in.at(i);
    }

    template<typename std::enable_if_t<std::is_pointer_v<T>>* = nullptr>
    rtvec(rtvec<typename std::remove_pointer_t<T>>& in) : d(in.dim()), e(new T[in.dim()])
    {
        for (i32 i = 0; i < d; ++i)
            e[i] = &in.at(i);
    }
    ~rtvec() { delete[] e; }
    i32 dim() const { return d; }


    template<typename std::enable_if_t<!std::is_pointer_v<T>>* = nullptr>
    inline T& at(i32 i)
    {
        return e[i];
    }

    template<typename std::enable_if_t<std::is_pointer_v<T>>* = nullptr>
    inline typename std::remove_pointer<T>::type& at(i32 i)
    {
        return *e[i];
    }

};

If I modify this a little, however, it does compile:

template<class T>
struct rtvec
{
private:
    T* e;
    i32 d;

public:
    rtvec(i32 d) : d(d), e(new T[d]) {}

    template<typename std::enable_if_t<!std::is_pointer_v<T>>* = nullptr>
    rtvec(const rtvec& in) : d(in.d), e(new T[in.d])
    {
        for (i32 i = 0; i < d; ++i)
            at(i) = in.at(i);
    }

    /*template<typename std::enable_if_t<std::is_pointer_v<T>>* = nullptr>
    rtvec(rtvec<typename std::remove_pointer_t<T>>& in) : d(in.dim()), e(new T[in.dim()])
    {
        for (i32 i = 0; i < d; ++i)
            e[i] = &in.at(i);
    }*/
    ~rtvec() { delete[] e; }
    i32 dim() const { return d; }


    template<typename std::enable_if_t<!std::is_pointer_v<T>>* = nullptr>
    inline T& at(i32 i)
    {
        return e[i];
    }

    /*template<typename std::enable_if_t<std::is_pointer_v<T>>* = nullptr>
    inline typename std::remove_pointer<T>::type& at(i32 i)
    {
        return *e[i];
    }*/


};

(as long as the part relating to the pointer is commented out in main too). I want to understand what is it that makes the 2nd code not compile.

The reason I am not directly specializing the class is that in my original implementation I have a lot of other member functions that will be equivalent between the two specializations which I do not want to repeat.

max66
  • 65,235
  • 10
  • 71
  • 111
lightxbulb
  • 1,251
  • 12
  • 29
  • 2
    Are you aware that if you make a copy constructor templated it's no longer a copy constructor? http://eel.is/c++draft/class.copy.ctor#1 – Daniel Langr Apr 07 '19 at 12:47
  • 2
    Anyway, I believe you find some related info here: [std::enable_if to conditionally compile a member function](https://stackoverflow.com/q/6972368/580083). According to answers there, SFINAE in your 3rd case does not work as well. – Daniel Langr Apr 07 '19 at 12:49
  • Actually I thought that the template should make the copy ctor simply a custom ctor, but then MSVC spat out that I had defined two copy ctors, and I lost it at that point. Basically I have no idea what's going on anymore, I usually go with what I expect to intuitively work, but the last 5 times I decided to make something more involved with templates, it ended up 5 times with bugs in the compiler that I reported to MS which I believe they fixed at some point, so at this point I just ask others whether it not working is as intended. – lightxbulb Apr 07 '19 at 13:04
  • *"more precisely I have several arrays of each of the coordinates of a set of points and I want to use the "reference" vector to be able to work with those as if they were ordered the other way around in memory"* are you aware of the concept of a [reverse iterator](https://en.cppreference.com/w/cpp/iterator/reverse_iterator)? – Michael Kenzel Apr 07 '19 at 13:10
  • 1
    Also, note that, contrary to what the name of your `typedef` may suggest, `i32` is not guaranteed to be a 32-Bit integer. Yes, it happens to be on MSVC. But if the correctness of your code really depends on having an integer with a guaranteed, fixed number of Bits, then why not just use the fixed-width integer types provided in [``](https://en.cppreference.com/w/cpp/types/integer)? (Note: I don't really see why the precise number of Bits used to represent the size of an `rtvec` would be essential here) – Michael Kenzel Apr 07 '19 at 13:15
  • @MichaelKenzel I actually have it tpedefd to int32_t in the original implementation, just forgot to add it when butchering the code to present the relevant part on here. – lightxbulb Apr 07 '19 at 13:25
  • @MichaelKenzel I don't believe a reverse iterator will be of use here. I basically have something like ```T arr[dim][n]``` (though it's T**, since dim and n will be set during runtime), and I want to access columns of that, but have the convenient vector operations I have defined in my original code for tvec, thus I want to construct a ```tvec``` from ```arr[0][i],arr[1][i],...,arr[dim-1][i]```. – lightxbulb Apr 07 '19 at 13:29
  • If you have a `T**` that points to the first element of an array of `T*`, where each of these `T*` points to one such column, you should be able just make a `reverse_iterator` to iterate through your columns the other way around. If you want to access each column as a `tvec`, then why not just make each column a `tvec` to begin with? – Michael Kenzel Apr 07 '19 at 13:45
  • @MichaelKenzel I initially thought of doing that, but I couldn't justify the extra int32 per point and all the invocations of new, since I would have point counts in the millions. It's also easier parallelize on my arrays in a per-coordinate layout first (performance and memory is somewhat of an issue in what I am currently programming, so I try to optimize things somewhat, I do agree that an array of vectors is a lot nicer to work with, that's the whole reason why I am trying to provide such a "reference" vector type which would fulfill this function in non-performance critical sections). – lightxbulb Apr 07 '19 at 13:50

2 Answers2

4

When I try to simplify the code, however, by trying to remove what I perceive as an unnecessary

Unfortunately (if I understand correctly) you have removed something that is necessary

If I understand correctly, you have simplified the following methods

template<typename U=T, 
    typename std::enable_if_t<std::is_same_v<U,T>>* = nullptr,
    typename std::enable_if_t<!std::is_pointer_v<U>>* = nullptr>
inline T& at(i32 i) 
{
    return e[i];
}

template<typename U = T,
    typename std::enable_if_t<std::is_same_v<U, T>>* = nullptr,
    typename std::enable_if_t<std::is_pointer_v<U>>* = nullptr>
    inline typename std::remove_pointer_t<T>& at(i32 i)
{
    return *e[i];
}

as follows

template<typename std::enable_if_t<!std::is_pointer_v<T>>* = nullptr>
inline T& at(i32 i)
{
    return e[i];
}

template<typename std::enable_if_t<std::is_pointer_v<T>>* = nullptr>
inline typename std::remove_pointer<T>::type& at(i32 i)
{
    return *e[i];
}

Unfortunately SFINAE, over a template method, works only when uses tests (std::enable_if_t) based on template parameters of the method itself.

I mean: SFINAE doesn't works when the std::enable_if_t tests involve T (and only T) because T is a template parameter of the struct, not a template parameter of the method.

So you need the trick

typename U = T

that "transform" the T type in a template parameter of the method.

You can simplify a little removing the typename before std::enable_if_t because the "_t" is exactly typename (see the definition of std::enable_if_t)

Off topic: I'm not a language layer but, as far I know,

 std::enable_if_t<std::is_same_v<U,T>>* = nullptr

isn't perfectly legal; I suggest to rewrite using int instead of void *

 std::enable_if_t<std::is_same_v<U,T>, int> = 0

or maybe bool

 std::enable_if_t<std::is_same_v<U,T>, bool> = true

or other integer types

Concluding, I suggest to rewrite your at() method as follows

template <typename U = T, 
          std::enable_if_t<std::is_same_v<U, T>, int> = 0,
          std::enable_if_t<!std::is_pointer_v<U>, int> = 0>
inline T& at(i32 i) 
{
    return e[i];
}

template<typename U = T,
         std::enable_if_t<std::is_same_v<U, T>, int> = 0,
         std::enable_if_t<std::is_pointer_v<U>, int> = 0>
    inline typename std::remove_pointer_t<T>& at(i32 i)
{
    return *e[i];
}
max66
  • 65,235
  • 10
  • 71
  • 111
  • I figured that may have been the issue, that's how I actually came up with the code that compiles. As far as the nullptr thing goes I actually referred to a number of answers on here and https://eli.thegreenplace.net/2014/sfinae-and-enable_if/ . Could you elaborate why you believe it's not a legal construct (or on which compiler would it break)? Also could you clarify why my 3rd code seems to work as long as the commented out lines stay commented out, but doesn't even if one of those is present? It seems a little bit counter intuitive for me why this is the case. The whole issue actually stems – lightxbulb Apr 07 '19 at 13:36
  • ... from the lack of support of function template partial specialization. But yeah, I try to work around it with SFINAE. – lightxbulb Apr 07 '19 at 13:37
  • 2
    @lightxbulb About (il)legality of `void*` non-type template parameter see, e.g.: [Why using 0 as default non type template parameter for void* is not allowed](https://stackoverflow.com/q/38154762/580083) or [pointer to void as template non-type parameter](https://cplusplus.github.io/EWG/ewg-active.html#175). – Daniel Langr Apr 07 '19 at 13:47
  • @DanielLangr So it's non-comformant but major compilers support it? Good to know, I'll try to use bool from now on. – lightxbulb Apr 07 '19 at 13:53
  • 1
    @lightxbulb - exactly: never seen a compiler that doesn't support it (and I've used it zillion of times before discover this problem) but, reading the standard, seems non-conformant. About the fact that your 3rd code works commenting the second version of the function, I suppose it's because SFINAE doesn't works, the functions are enabled, when you have only one function works, with two function with the same signature you have a collision and the compiled doesn't know which version use. – max66 Apr 07 '19 at 15:09
  • @max66 Thank you very much. This clarified everything. – lightxbulb Apr 07 '19 at 15:12
  • @lightxbulb I wouldn't say strictly non-conformant, it's a matter of opinion. Even experts discuss whether it is allowed or not, it's a bit unclear. The Standard allows pointers to objects as non-type template parameters, however, some argue that `void*` can hold a pointer to an object too. – Daniel Langr Apr 07 '19 at 15:34
1

There are several reasons and you need to provide the full compilation error. Regarding your code, you seem to use C++17.

For instance, in this code you are trying to return a reference to the object stored in the array, right?:

inline typename std::remove_pointer<T>::type& at(i32 i) {
        return *e[i];
}

Can be replaced with a more STL-Like code:

using reference = T&;
using const_reference = const T&;

reference at(i32 i) {
    return e[i];
}

const_reference at(i32 i) const {
    return e[i];
}

Or use auto:

auto at(i32 i) const {
    return e[i];
}

This is the way most of the STL containers work. For instance, if you access an std::vector<T*>, it will return a reference to a T*, not a reference to the data where T points.

Regarding the SFINAE technique you are using, I am not sure if it's properly written.

For instance, take a look into this post to find information about the proper ways to write the conditions for selecting constructors. Small sumary:

template <typename = typename std::enable_if<... condition...>::type>
explicit MyAwesomeClass(MyAwesomeClass<otherN> const &);

For instance, if you want to enable a constructor only for those instance that do not hold a pointer type:

template<typename = typename std::enable_if_t<!std::is_pointer_v<T>>>
explicit rtvec(const rtvec& in) : d(in.d), e(new T[in.d]) {
    for (i32 i = 0; i < d; ++i)
        at(i) = in.at(i);
}

Now, regarding the fact that you are using C++17, you can us constexpr if that will make your life much easy and handle the different situations. Something like this, I guess:

template <typename U>
explicit rtvec(const rtvec<U>& in) : d(in.d), e(new T[in.d]) {    
     for (i32 i = 0; i < d; ++i){
         if constexpr (std::is_pointer<T>::value &&
                       std::is_pointer<U>::value) {
             // ...
         } else if constexpr (!std::is_pointer<T>::value &&
                       std::is_pointer<U>::value) {
             // ...
         } else {
             //  rest of possible combinations
         }
     }
}   
mohabouje
  • 3,867
  • 2
  • 14
  • 28
  • Uhmmm... no: the SFINAE form `typename = std::enable_if_t` work but is preferable the form `std::enable_if_t = 0` because only the second one works when you want to write two alternative versions of the same method. – max66 Apr 07 '19 at 13:27
  • Constexpr seems like a good alternative, thank you for recommending it! I believe my SFINAE usage should be correct, I referred to numerous posts regarding this. – lightxbulb Apr 07 '19 at 13:46
  • Ah nvm, constexpr won't work with the ```at``` functions since the return type is different, I don't think the above will qualify as a copy constructor either. – lightxbulb Apr 07 '19 at 13:56