How to prevent a constructor from being called with a temporary

Question

I have a class whose constructor takes some vectors and stores them.

struct X {
    X(std::vector<int> const& ints, std::vector<float> const& floats, std::vector<std::string> const& strings)
    : ints_{ints}
    , floats_{floats}
    , strings_{strings}      
    {};

    std::vector<int> ints_;
    std::vector<float> floats_;
    std::vector<std::string> strings_;
};

I would like to make the member variables into references, because in production code the values passed to the constructor are lvalues, whose lifetimes are longer than the object of class X.

However, unit tests usually construct Xes with temporaries, like this:

X x{ {42}, {3.14f}, {"hello"} };

If the members of X are to be references, such calls should be prevented. This can be done by writing a constructor that takes rvalue references and making it =delete.

X(std::vector<int> && ints, std::vector<float> && floats, std::vector<std::string> && strings) = delete;

This would prevent the instantiation if all arguments are temporaries. Unfortunately, it lets through calls where at least one argument is an lvalue:

std::vector<std::string> no_strings;
X x{ {42}, {3.14f}, no_strings };

because lvalue references are willing to bind to rvalues, so the (const&,const&,const&) constructor can be called.

Do I have to write every combination of lvalue/rvalue ref arguments (all seven of them) and mark all of them as deleted?

[This](https://stackoverflow.com/questions/8349595/why-does-a-perfect-forwarding-function-have-to-be-templated) answer discusses a similar problem, but I don't think it will work with reference members since IIRC `std::move(const T&)` will always cause a copy. — 0x5453, Jan 22 '18 at 16:46

score 3 · Answer 1 · answered Jan 22 '18 at 18:52

template<class T, class U>
using is_lvalue_reference_to_possibly_const = std::integral_constant<bool,
    std::is_same<T, const U&>::value || std::is_same<T, U&>::value>;

template<class VI, class VF, class VS>
using check_value_cat_for_ctor = typename std::enable_if<
    is_lvalue_reference_to_possibly_const<VI, std::vector<int>> &&
    is_lvalue_reference_to_possibly_const<VF, std::vector<float>> &&
    is_lvalue_reference_to_possibly_const<VS, std::vector<string>>>::type;

template<class VI, class VF, class VS, class = check_value_cat_for_ctor<VI, VF, VS>>
X(VI&&, VF&&, VS&&) {
    // ...
}

I'd rather not make the constructor a template just for this. — Bulletmagnet, Jan 26 '18 at 14:37

rubenvb · Answer 2 · 2018-01-22T20:09:15.343

How about a function with non-const lvalue ref arguments? Temporaries cannot bind to those so this should be what you are looking for.

Something like this:

#include <string>
#include <vector>

struct X {
    X(std::vector<int>& ints,
      std::vector<float>& floats,
      std::vector<std::string>& strings)
    : ints_{ints}
    , floats_{floats}
    , strings_{strings}      
    {};

    std::vector<int>& ints_;
    std::vector<float>& floats_;
    std::vector<std::string>& strings_;
};

int main()
{
    X x{ {42}, {3.14f}, {"hello"} };
}

The only thing you lose is a bit of const correctness., which can be as thin as only the constructor. That is, if you want const at all. Your question does not make that clear at all.

Because the templates in @T.C.'s answer make my eyes bleed, here's my version of those if you were more inclined towards that solution:

#include <string>
#include <type_traits>
#include <vector>

template<typename ... Ts>
constexpr bool are_not_temporaries = (std::is_lvalue_reference<Ts>::value&&...);

struct X
{
  template<typename VI, typename VF, typename VS,
           typename = std::enable_if_t<are_not_temporaries<VI, VF, VS>, void>>
  X(VI&& ints, VF&& floats, VS&& strings)
    : ints_{ints}
    , floats_{floats}
    , strings_{strings}      
    {};

    std::vector<int>& ints_;
    std::vector<float>& floats_;
    std::vector<std::string>& strings_;
};

int main()
{
    X x{ {42}, {3.14f}, {"hello"} };
}

This uses C++17 fold expressions to expand the parameter pack over the variadic template arguments. In case C++17 is unavailable, you can replace that with:

template<typename T1, typename T2, typename T3>
constexpr bool are_not_temporaries = std::is_lvalue_reference<T1>::value 
                                  && std::is_lvalue_reference<T2>::value 
                                  && std::is_lvalue_reference<T3>::value;

Which is admittedly a lot less nice.

If OP does want `const`, then your `is_lvalue_reference` check fails to block cases where the user passes a `converts_to_a_temporary_vector` lvalue. That's why I went for requiring exact match. — T.C., Jan 22 '18 at 20:23

score 2 · Accepted Answer · edited Jan 26 '18 at 14:39

2

What if you just defer to the library:

template <typename T>
using vector_ref = std::reference_wrapper<std::vector<T> const>;

struct X {
  X(vector_ref<int> ints, vector_ref<float> floats, vector_ref<std::string> strings);
};

std::reference_wrapper is already only constructible from lvalues, so we don't need to do all that work ourselves.

edited Jan 26 '18 at 14:39

Bulletmagnet

5,665
2
26
56

answered Jan 22 '18 at 20:57

Barry

286,269
29
621
977

Note: if the members are `const &`, the initializer must look like this : `ints_{ints.get()}` rather than `ints_{ints}`. Otherwise the compiler complains of "binding a reference member to a temporary (which persists until the constructor exits)". – Bulletmagnet Jan 26 '18 at 14:43
@Bulletmagnet Hm? `.get()` does the same thing as `operator T&()`. What compiler/version? – Barry Jan 26 '18 at 17:03
I thought the same, but clang 4.0, clang 6.0, gcc 7.2 and gcc 8.0 all warn if a `reference_wrapper` is passed directly (without `.get()`) to a member initializer with braces (as I wrote). There are no warnings if the member initializer uses parens. – Bulletmagnet Jan 29 '18 at 09:53
List-initialization of reference is odd and indeed [creates a temporary in this case](https://timsong-cpp.github.io/cppwp/dcl.init.list#3.10). – T.C. Mar 30 '18 at 07:40

How to prevent a constructor from being called with a temporary

3 Answers3