1

When choosing to pass by const reference vs. by value, the choice seems clear for large classes: gotta use const ref to avoid an expensive copy since copy elision is permitted only in limited circumstances (if copy constructor has no side effects or if the copy was from an rvalue).

For smaller classes, it seems passing by value is more attractive:

  • it's just as fast as dereferencing
  • it avoids aliasing (which is both bug-prone and bad for performance as it forces the compiler to be more conservative)
  • if a copy is necessary anyway, it makes the copy in the scope where the compiler may be able to use copy elision

So what is the best practice when writing a template function, where it's not always obvious whether the class of the argument is large or small?

max
  • 49,282
  • 56
  • 208
  • 355
  • 2
    You can implement both: value for small POD types, const reference for everything else. – user7860670 Jul 03 '17 at 11:41
  • 1
    Note template functions can often be inlined, in which case the compiler might also eliminate treating the reference internally like a pointer. – aschepler Jul 03 '17 at 11:51
  • You can still use [call_traits](http://www.boost.org/doc/libs/1_61_0/libs/utility/call_traits.htm) or similar. – Jarod42 Jul 03 '17 at 13:47

3 Answers3

5

(Assuming that you only want to read from the value you're passing.)

I think that the best choice is passing by const&, as:

  • Some objects cannot be copied, are expensive to copy, or contain side-effects in their copy constructor.

  • While taking primitives by const& might result in a minor performance degradation, this is a smaller loss compared to the problems described in the bullet point above.


Ideally, you would want to do something like this (I'm not being careful about small classes that have side-effects in the copy constructor here):

template <typename T>
using readonly = std::conditional_t<
    sizeof(T) <= sizeof(void*),
    T,
    const T&
>;

template <typename T>
void foo(readonly<T> x);

The problem is that T cannot be deduced from a call to foo, as it is in a "non-deducible context".

This means that your users will have to call foo<int>(0) instead of foo(0), as T=int cannot be deduced from the compiler.

(I want to reiterate that the condition I'm using above is very naive and potentially dangerous. You might want to simply check if T is a primitive or a POD smaller than void* instead.)


Another possible thing you can do is use std::enable_if_t to control what function gets called:

template <typename T>
auto foo(T x) -> std::enable_if_t<(sizeof(T) <= sizeof(void*))>;

template <typename T>
auto foo(const T& x) -> std::enable_if_t<(sizeof(T) > sizeof(void*))>;

live example on wandbox

This obviously requires a lot of extra boilerplate... maybe there will be a better solution using code generation when (if?) we'll get constexpr blocks and "metaclasses".

Vittorio Romeo
  • 90,666
  • 33
  • 258
  • 416
  • `sizeof(T) <= sizeof(void*)` That's implementation defined behavior though, no? Wouldn't a type trait like `std::is_class` be better because it doesn't rely on implementation behavior? – Rakete1111 Jul 03 '17 at 11:55
  • @Rakete1111: it is - as I said in the answer it's just an example. `is_class` is not that great either because it wouldn't work for things like "strong typedefs" or empty structs. There probably is a good and safe combination of traits, but I need to think about it... – Vittorio Romeo Jul 03 '17 at 12:02
2

I think as a basic rule, you should just pass by const&, preparing your generic template code for the general case of expensive-to-copy objects.

For example, if you take a look at std::vector's constructors, in the overload that takes a count and a value, the value is simply passed by const&:

explicit vector( size_type count, 
                 const T& value = T(),
                 const Allocator& alloc = Allocator())
Mr.C64
  • 41,637
  • 14
  • 86
  • 162
2

What you want is a type trait that tests if the type is a scalar and then switches on that

template <typename Type>
using ConstRefFast = std::conditional_t<
    std::is_scalar<std::decay_t<Type>>::value,
    std::add_const_t<Type>,
    std::add_lvalue_reference_t<std::add_const_t<std::decay_t<Type>>>
>;

And then pass an object by reference like this

template <typename Type>
void foo(typename ConstRefFast<Type>::type val);

Note that this means that the function will not be able to deduce the type T automatically anymore. But in some situations it might give you what you want.


Note that when it comes to template functions, sometimes the question of ownership is more important than just whether you want to pass the value by const ref or by value.

For example consider a method that accepts a shared_ptr to some type T and does some processing on that pointer (either at some point in the future or immediately), you have two options

void insert_ptr(std::shared_ptr<T> ptr);
void insert_ptr(const std::shared_ptr<T>& ptr);

When the user looks at both of these functions, one conveys meaning and semantics clearly while the other just leaves questions in their mind. The first one obviously makes a copy of the pointer before the method starts, thus incrementing the ref count. But the second one's semantics is not quite clear. In an asynchronous setting this might leave room for doubt in the user's mind. Is my pointer going to be safely used (and object safely released) if for example the pointed to object is used at some point in the future asynchronously?


You can also consider another case that does not consider asynchronous settings. A container that copies values of type T into internal storage

template <typename T>
class Container {
public:
    void set_value(T val) {
        this->insert_into_storage(std::move(val));
    }
};

or

template <typename T>
class Container {
public:
    void set_value(const T& val) {
        this->insert_into_storage(val);
    }
};

Here the first one does convey the fact the value is copied into the method, after which the container presumably stores the value internally. But if the question of lifetime of the copied object is not important, then the second one is more efficient, simply because it avoids an extra move.

So in the end it just comes down to whether you need clarity of your API or performance.

Curious
  • 20,870
  • 8
  • 61
  • 146
  • `void set_value(const T& val) {this->insert_into_storage(std::move(val));}` ... how does that make sense as you can't steal resources from a const object? – zett42 Jul 03 '17 at 12:27
  • @zett42 my bad I copy pasted that and forgot to delete the `std::move` part – Curious Jul 03 '17 at 12:28