Temporaries passed to constructor yield invalid member references
in [C++]
|
Prev: Temporaries passed to constructor yield invalid member references
Next: std::memset, std::fill, hand-written for loop
From: Goran on 31 May 2010 00:11 On May 30, 1:39 am, M Stefan <mstefa...(a)gmail.com> wrote: > Hello. I am trying to write a class similar to this: > template <class StringT> > struct A > { > A (StringT const &str) : str(str) { } > StringT const &str;}; > > The problem is that the constructor allows passing temporaries, which > get destroyed as soon as the constructor finishes executing, not after > the object is destroyed. > For instance, constructing the class: A<std::string> a("hello"); would > cause an implicit conversion from char const* to std::string const&, > and the converted temporary will be passed to the ctor. The reference > str of the class is then bound to this temporary, which gets destroyed > before the object, thus leaving the object with an invalid reference. > Then, I modified it so that no implicit conversions are allowed: > template <class StringT> > struct A > { > A (StringT const &str) : str(str) { } > template <class U> explicit A (U) { BOOST_STATIC_ASSERT(false); } > StringT const &str;}; > > This fixes the problem with the conversion from char const* to > std::string const&, but still allows the following way of constructing > the class: A<std::string> a( (std::string("a")) ); > This constructs A with a temporary which again gets destroyed before > the object, thus leaving an invalid ref behind. > Discussing with some other people, we have come up with several > solutions, none of which fully satisfy me: > 1) Make the constructor take a non-const ref as opposed to a const > ref. (Why not: breaks const correctness) No. Example: TYPE* p; { TYPE t; p = &t; } TYPE& r = *p; // ref broken your_class o(r); // object broken. > 2) Add an additional level of indirection: (Why not: likely overhead) > struct A > { > boost::any str; > A () : str() {} > template <class StringT> void set_str(StringT const &s) { str = s; }}; No. Using any is more overhead than just copying data (could change with movable types of new standard? perhaps). > 3) Make the constructor take a StringT as opposed to a reference (Why > not: overhead and additional memory) No, no, no! If parameter is a temporary, it will be destroyed when call finishes, leaving your object with a dangling reference just the same. > Please let me know if you have any better solutions to my problem. In languages like C and C++, there are no, zilch, nada solutions. You have to handle lifetime of your stuff at any point manually. That's how the language works, so use it that way. You passed a temporary and stored a reference to it. That does not work, end of. Goran. P.S. Martin B's "solution" isn't, e.g. class x { public: x(const TYPE* p) : _r(*p) {} TYPE& _r; }; is broken with x(NULL); // bad x._r, no help from compiler and x(&TYPE()); // bad x._r, no help from compiler Goran. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: M Stefan on 1 Jun 2010 05:33
I may have omitted a few details which at the beginning have seemed irrelevant: I am writing some generic string algorithms and the template parameter is actually of range concept (Boost.Range). Basically, it allows any thing with a begin(), an end() and a few other things. Additionally, my range concept also allows char* and wchar_t*. After giving it considerable thought, I have managed to come up with a solution to my problem: template <class T> struct My : boost::noncopyable { public: My () : t_(), range_() { } //a pointer is passed, we are relying on the fact that its lifetime //will be bigger than that of the object, so we don't make a copy //basically we default-construct our t_ (an empty string) //and initialize our range (COPY AVOIDED) //! \warning This is a fast constructor, because your string is not copied. //! However, in order to use this constructor, you *MUST* be able //! to guarantee that the lifetime of the pointee is at least as long //! as the lifetime of this object explicit My (T const *t) : t_(), range_(boost::as_literal(*t)) { } //a const ref is passed, we cannot rely on its lifetime, so we're making a copy. //store the copy in t_, and the range is initialized with t_'s range //(COPY MADE) //! \warning This constructor performs a copy of your string. //! If you want to avoid copying strings around, use the ctor taking a //! pointer. explicit My (T const &t) : t_(t), range_(boost::as_literal(t_)) { } # ifdef BOOST_HAS_RVALUE_REFS //we have rvalue refs and we were passed a temporary //we're just gonna move the temporary to t_ //and initialize our range with the range of t_ (COPY AVOIDED) explicit My (T const &&t) : t_(std::move(t)), range_(boost::as_literal(t_)) { } # endif private: T t_; //typename make_properly_copyable<T>::type t_; boost::iterator_range<typename boost::range_const_iterator<T>::type> range_; }; As you can see, the constructors may or may not copy the string into t_, but they always properly copy the string's range into range_. The class' algorithms will never rely on the contents on t_, but only on the contents of range_. In certain cases, t_ may be forced to make a copy of the passed string, whereas in other cases a move is made, or t_ is default-constructed. This obviously assumes that default-constructing is a cheap operation, much cheaper than copy constructing a large string. Here are a few use cases of the class: //we are passing a temporary, and the class has no choice but to make a copy //(or a move if the compiler supports it) My<std::string> a("copy or move is made"); //we are passing a lvalue reference, the class has no choice but //to make a copy std::string s("copy is made"); My<std::string> b(s); //no copy is made, the range is initialized with the range of s2 std::string s2("copy is not made, but you must make sure the lifetime" "of this object is long enough"); My<std::string> c(&s2); As you can see, I've commented out "typename make_properly_copyable<T>::type t_;". What this did was it basically checked if T was char*, and if it was, then it wrapped it into a class that allowed proper copying using strlen() and strncpy(). Let me know what do you think about my solution, and thank you for your answers. Yours sincerely, Stefan -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |