13

This is a simplified version of my type system:

#include <string>
#include <vector>

template<typename T>
class Box {
public:
    Box(const T& value) : _value(value) {};
private:
    T _value;
    /* ... */
};

typedef Box<int> Int;
typedef Box<double> Double;
typedef Box<std::string> String;

int main(int argc, char* argv[]) {
    String a("abc");
    std::vector<String> b = { std::string("abc"), std::string("def") };

    // error C2664: 'Box<std::string>::Box(const Box<std::string> &)' : cannot convert argument 1 from 'const char' to 'const std::string &'
    std::vector<String> c = { "abc", "def" };
}

While a and b compile, c does not and the reason seems to be that I try to initialize from const char. This raises two questions:

  1. Why is b possible but not c? Is it because of the nested template in std::vector<Box<std::string> >?

  2. Can I make c work without destroying the general boxing mechanism (cf. typedef Box<double> Double?

Jarod42
  • 203,559
  • 14
  • 181
  • 302
PhilLab
  • 4,777
  • 1
  • 25
  • 77
  • clang and gcc give me this error with your example : ``main.cpp:18:12: error: no viable conversion from 'const char [4]' to 'String' (aka 'Box >') String a = "abc"; `` (the error messages changes between the compiler but the error is the same). Maybe the error is that you can't use implicit conversion (by calling the constructor or with user defined cast operator) with template ? – nefas May 22 '17 at 09:04
  • Oops, you are right. Seems I made a mistake while verifying what compiles and what not. So half a mystery solved. Adapted the question – PhilLab May 22 '17 at 09:06

3 Answers3

18

c currently need 2 implicit user conversions (const char [N] -> std::string -> String) whereas only one is allowed.

You may add template constructor to Box

template<typename T>
class Box {
public:
    Box() = default;
    Box(const Box&) = default;
    Box(Box&&) default;
    ~Box() = default;

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

    template <typename U0, typename ...Us,
              std::enable_if_t<std::is_constructible<T, U0, Us...>::value
                               && (!std::is_same<Box, std::decay_t<U0>>::value
                                  || sizeof...(Us) != 0)>* = nullptr>
    Box(U0&& u0, Us&&... us) : _value(std::forward<U0>(u0), std::forward<Us>(us)...) {}
private:
    T _value;
    /* ... */
};

Demo Demo2

Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • I have been thought for 3 hours, experienced people are better :) – Soner from The Ottoman Empire May 22 '17 at 12:35
  • 1
    Any reason to limit this to a single argument? E.g. why not accept an iterator pair for `Box` ? – MSalters May 22 '17 at 13:54
  • 2
    This thing needs constraints. Lots of them. – T.C. May 22 '17 at 17:55
  • It is *not* a good idea to use forwarding references in an overloaded function like a class constructor. The forwarding overload will be hit first during overload resolution in many cases where you don't want it to. For example, `Box b1(1); Box b2(b1)` hits the forwarding constructor instead of the compiler-generated copy constructor. – 0x5453 May 22 '17 at 18:52
  • @0x5453: Version Improved – Jarod42 May 22 '17 at 19:00
  • `Boxed(U0&& u0, Us&&... us)`, disable on `sizeof...(Us==0)&&std::is_same,Boxed>{}` -- add `Boxed()=default;`. Now even if `T` can be constructed from `Boxed` you do not replace copy/move operations with `T` construction. – Yakk - Adam Nevraumont May 23 '17 at 03:51
  • This doesn't compile for me. I fixed the demo to use the code in this answer, and I also fixed the apparent typo (`Box(T&&) default` to `Box(T&&) = default`), but I [still get](http://coliru.stacked-crooked.com/a/3b17ea026268f12a) `only special member functions may be defaulted`, `copy constructor is implicitly deleted`, and `invokes deleted constructor` errors. – joshtch Jul 01 '18 at 22:56
  • @joshtch: Typos fixed too: [Demo2](http://coliru.stacked-crooked.com/a/72c17dd4e922b979) added. – Jarod42 Jul 02 '18 at 07:49
3

Looking at your source code in the main function part only:

int main(int argc, char* argv[]) {
    String a("abc");
    std::vector<String> b = { std::string("abc"), std::string("def") };

    // error C2664: 'Box<std::string>::Box(const Box<std::string> &)' :
    // cannot convert argument 1 from 'const char' to 'const std::string &'
    std::vector<String> c = { "abc", "def" };
}

Your first line of code:

String a("abc");

Is using the typedef version of Box<std::string> which this class template takes a const T& and since this version of the template is expecting a std::string it is using std::string's constructor to construct a std::string from a const char[3] and this is okay.

Your next line of code:

std::vector<String> b = { std::string("abc"), std::string("def") };

Is a std::vector<T> of the same above. So this works as well since you are initializing the vector<T> with valid std::string objects.

In your final line of code:

std::vector<String> c = { "abc", "def" };

Here are you declaring c as a vector<T> where T is a typedef version of Box<std::string> however you are not initializing the std::vector<T> with Box<std::string> types. You are trying to initialize it with const char[3] objects or string literals.

You can try doing this for the third line: I haven't tried to compile this but I think it should work.

std::vector<String> c = { String("abc"), String("def") };

EDIT -- I meant to use the constructor for String and not std::string made the appropriate edit.

Francis Cugler
  • 7,788
  • 2
  • 28
  • 59
2

You could use a helper function that creates the corresponding type:

template <typename T,typename R>
Box<T> make_boxed(const R& value){
    return Box<T>(value);
}

it may seem like additional complication that one has to specify the T, on the other hand you can use auto for the returned type. Complete example:

#include <string>

template<typename T>
class Box {
public:
    Box(const T& value) : _value(value) {};
private:
    T _value;
    /* ... */
};

typedef Box<std::string> String;

int main(int argc, char* argv[]) {
    auto a = make_boxed<std::string>("asd");
}
463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185