Why does C++ not attempt to use the second template overload?

Viewed 108

I was implementing a simple smart pointer class, and I decided I wanted to make my own versions of the std::make_unique / std::make_shared functions to go along with it once I finished. I created these two overloads:

// note: Box<T> is my "unique pointer" type, it has a partial specialization for T[],
// and it works as expected when created outside of these functions

template <class T, class... Args> Box<T> make_box(Args &&... args) {
  auto ptr = new T(std::forward<Args>(args)...);

  return Box<T>(std::move(ptr));
}

template <class T> Box<T> make_box(std::size_t size) {
  auto ptr = new std::remove_extent_t<T>[size];

  return Box<T>(std::move(ptr));
}

The first overload works just fine, at least in this example:

struct Point3D {
  double x, y, z;

  Point3D() = default;

  Point3D(double x, double y, double z) : x{x}, y{y}, z{z} {};
};

// works exactly as expected, Box is created and does what it's supposed to
auto box = make_box<Point3D>(1.0, 2.0, 3.0); 

However, the overload for arrays doesn't seem to be used. If I try to call it with an array type as T, the program won't compile. The following code gives me an error while attempting to use the first overload, and doesn't even attempt to use the second one:

// Gives an error about "allocation of incomplete type 'Point3D []' 
// from inside a template instantiation of 'make_box<Point3D [], int>'.
// the overload with one template parameter isn't used
auto box = make_box<Point3D[]>(20);

// Note that this works fine, and uses the Box specialization that calls delete[]: 
Box<Point3D[]> boxed(new Point3D[20]);

What is the reason for this? The two overloads seem effectively identical to the implementations of std::make_unique inside LLVM's libc++ and GNU's libstdc++. It does it on multiple compilers as well (tested with GCC 10.1 and Clang 10.0.1, both compiled with -std=c++17 -Wall -Wextra -pedantic).

Edit: Definition(s) for the Box class:

template <class T> class Box {
  T *m_ptr;

public:
  explicit Box(T *&&ptr) : m_ptr{ptr} {}

  Box() = delete;

  Box(const Box &) = delete;

  Box(Box &&other) : m_ptr{other.m_ptr} {}

  ~Box() { delete m_ptr; }

  T &operator*() const { return *m_ptr; }

  T *operator->() const { return m_ptr; }
};

template <class T> class Box<T[]> {
  T *m_ptr;

public:
  explicit Box(T *&&ptr) : m_ptr{ptr} {}

  Box() = delete;

  Box(const Box &) = delete;

  Box(Box &&other) : m_ptr{other.m_ptr} {}

  ~Box() { delete[] m_ptr; }

  T &operator*() const { return *m_ptr; }

  T *operator->() const { return m_ptr; }

  T &operator[](std::size_t idx) { return m_ptr[idx]; }
};
3 Answers

"Forwarding reference" deduced template type arguments are greedy, which is interfering with your overload resolution.

When you call:

auto box = make_box<Point3D[]>(20);

This is actually calling make_box<T,Args...> with T = Point32[] and Args = int -- which is unambiguously resolved as a better overload than calling make_box<T[]>(std::size_t). This is due to the fact that 20 is a PR-value of an int, which requires a conversion to std::size_t to be an accurate match for the second overload. Since overload resolution will always prefer to choose the overload that requires no conversions, it selects the first overload.


The fix for this would be to use SFINAE to prevent the Args... overload from being selected when T is an array type. This is what is done for std::make_unique in order to choose between a T[] and T type. The way that std::make_unique is commonly implemented is by SFINAE-detecting when T is a scalar, bounded array, or an unbounded array type, and presenting overloads accordingly.

Using this approach, your code could be rewritten as:

  namespace detail {
    template <typename T>
    struct make_box_result
    {
      using object = T;
    };
    template <typename T>
    struct make_box_result<T[]>
    {
      using unbounded_array = T[];
    };
    template <typename T, std::size_t N>
    struct make_box_result<T[N]>
    {
      using bounded_array = T[N];
    };
  }

  // Only enable 'Args...' overload for non-array types
  template <typename T, typename...Args>
  Box<typename detail::make_box_result<T>::object>
    make_box(Args&&...args);

  // Only enable 'size_t' overload for array types (unbounded arrays).
  // Prevents the greedy lookup
  template <typename T>
  Box<typename detail::make_box_result<T>::unbounded_array>
    make_box(std::size_t size);

  // Disabled for fixed types
  template <typename T>
  Box<typename detail::make_box_result<T>::bounded_array>
    make_box() = delete;

There are other ways that you can SFINAE-prevent this from occurring; I just use this approach as an example since it also prevents T[N] from being specified.

20 is an int literal, not an std::size_t literal. This means that the first overload is preferred, as no conversion is required. You can force the type like so, and then it works:

auto box = make_box<Point3D[]>(std::size_t{20});

Overloading in this way, you will also never be able to call make_box() on a type whose constructor takes a single std::size_t because the second form will be preferred and the whole thing will fall over.

You can fix both of these issues by enabling each function appropriately depending on whether T refers to an array type:

template <class T, class... Args>
std::enable_if_t<!std::is_array_v<T>, Box<T>> make_box(Args &&... args) {
  auto ptr = new T(std::forward<Args>(args)...);

  return Box<T>(std::move(ptr));
}

template <class T>
std::enable_if_t<std::is_array_v<T>, Box<T>> make_box(std::size_t size) {
  auto ptr = new std::remove_extent_t<T>[size];

  return Box<T>(std::move(ptr));
}

This technique would allow you to implement another overload for array types, taking the array elements directly as arguments (though you would run into the same pitfall if calling make_box<std::size_t>(10) -- this would create an array of 10 elements, where the intent could be to create an array with 1 element equal to 10):

template <class T, class... Args>
std::enable_if_t<std::is_array_v<T>, Box<T>> make_box(Args && ... values) {
  auto ptr = new std::remove_extent_t<T>[sizeof...(values)] { std::forward<Args>(values)... };

  return Box<T>(std::move(ptr));
}

The problem is with your std::size parameter. 20 is of type int, so this call involves a conversion. The first overload is a perfect match.

make_box<Point3D[]>(static_cast<std::size_t>(20))

would work but of course nobody wants to type that.

You may want to SFINAE out array types from the first overload.

template <class T, class... Args> 
     auto make_box(Args &&... args) -> 
         std::enable_if_t<!std::is_array_v<T>, Box<T>>
Related