[C++03] Temporaries passed to constructor yield invalid member references
M Stefan
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)
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; }
};
3) Make the constructor take a StringT as opposed to a reference (Why
not: overhead and additional memory)
Please let me know if you have any better solutions to my problem.
Note that compatibility with C++03 is a must here, and performance is
kind of important.
Best Regards,
Stefan
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Razvan Cojocaru
> 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.
>
That seems like a rather convoluted way of trying to achieve this
(template part ommited as it seem irrelevant for the purposes of our
discussion):
struct A
{
A(const std::string& s) : str(s) {}
/* const, if you prefer */ std::string str;
}
Why is it terribly important to have a reference struct member, instead
of a regular string? Even if you're using (non-const) references, you
should be aware that if the string you're referring to in the struct
goes out of scope (destroyed) somewhere, and the struct instance
doesn't, you still have a big problem. So the safest (and simplest) way
is to simply copy the contents of the constructor string to the member
data string (if nothing else, because how else would you cover the case
you've described, where a temporary string gets constructed when passed
to A(), then destroyed?).
Cheers,
--
Razvan Cojocaru
KeyID: 0x04CA34DE
Dragan Milenkovic
> 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.
I don't think there is any help for you regarding references.
Even if you somehow manage the impossible and limit invocation
of the ctor to non-temporaries, the general problem with dangling
references will persist.
typedef A<std::string> AStr;
std::shared_ptr<AStr> a;
{
std::string foo;
a.reset(new AStr(foo));
}
// dangle
IMHO, it's best to clearly document the purpose and functionality of A.
And now for other solutions. Clearly, the C++ way to do things is
as in your "solution 3". You worry too much about performance.
Don't tell me those strings are ~10MB. Think again about it,
and if you really _need_ not to make copies, use std::shared_ptr
as a member.
{
A(const std::shared_ptr<StringT> & str) : str(str) {}
std::shared_ptr<StringT> str;
}
--
Dragan
Martin B.
> 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.
> 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)
> 2) Add an additional level of indirection: (Why not: likely overhead)
> struct A
> {
> boost::any str;
> A () : str() {}
> };
> 3) Make the constructor take a StringT as opposed to a reference (Why
> not: overhead and additional memory)
>
> Please let me know if you have any better solutions to my problem.
> Note that compatibility with C++03 is a must here, and performance is
> kind of important.
>
Use a const pointer. Use it for the argument passing and if it's not too
much hassle also use it for the member.
Also see:
http://groups.google.com/group/comp.lang.c++.moderated/browse_thread/thread/c561510cc7c53dc9
Bottom line for me is: Always use pointers for these parameters.
Use a pointer member in "90%" of cases.
br,
Martin
Goran
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.
Razvan Cojocaru
> much hassle also use it for the member.
>
> Use a pointer member in "90%" of cases.
I don't see the difference between his problem using a non-const
reference and what would ensue if he uses a const pointer (since a
reference is pretty much the functional equivalent of a const pointer,
dereferencing and checking for NULL aside).
--
Razvan Cojocaru
KeyID: 0x04CA34DE
Dragan Milenkovic
> M Stefan 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.
> IMHO, it's best to clearly document the purpose and functionality of A.
>
> And now for other solutions. Clearly, the C++ way to do things is
> as in your "solution 3". You worry too much about performance.
I didn't read carefully. You want to change only the constructor,
while I had in mind changing only the member field. So, either
make it StringT or shared_ptr<StringT> or pointer/reference
and write a nice documentation.
Someone else argumented that making it a pointer is not different
than making it a reference. Well, apart from gaining operator=
(should one need it), you might also force a (clueless) user actually
_think_ about what he/she is doing while typing operator "&". :-)
Martin B.
>> Use a const pointer. Use it for the argument passing and if it's not too
>> much hassle also use it for the member.
>>
>> Bottom line for me is: Always use pointers for these parameters.
>> Use a pointer member in "90%" of cases.
>
> I don't see the difference between his problem using a non-const
He stated his problem as quote:"The problem is that the constructor
allows passing temporaries (...)".
> reference and what would ensue if he uses a const pointer (since a
> reference is pretty much the functional equivalent of a const pointer,
> dereferencing and checking for NULL aside).
>
Why did you snip the link I provided? It was discussed there.
The difference is that you can pass a temporary as a const-ref without
seeing it, while you cannot pass a temporary as a pointer without some
syntactic overhead. (This statement does *only* apply to the immediate
call - it does not apply to general lifetime management.)
Example:
A::A(int const& ref)
: m_ref(ref)
{ }
vs.
B::B(int const* ptr)
: m_ptr(ptr) // or m_ref(*ptr)
{ }
void f() {
int x;
char y;
A a1(x); // Fine, a1 can reference x from it's ref member
A a2(y); // The ctor get's passed a temporary int object!
// It does not reference y!
A a3(42); // compiles just fine, will reference a temp!
B b1(&x); // ok
B b2(&y); // good: compiler error, because char* <> int*
B b3(&42); // good: compiler error
}
Refs and Ptrs are functionally equivalent, I agree. They are absolutely
not treated equivalent by the compiler+lang rules though.
br,
Martin
--
Martin B.
> On May 30, 1:39 am, M Stefan <mstefa...@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.
>> 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
>
I never ever claimed that my proposal is a solution to lifetime
management! What I continue to claim is that /if/ you need to reference
any object like in the OP, then you should use a pointer [1], because
the conversion rules for (const-)references make it nearly impossible to
get it right.
br,
Martin
[1]: Yes, a smart-pointer would be best, but that's not always possible.
M Stefan
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
Razvan Cojocaru
> seeing it, while you cannot pass a temporary as a pointer without some
> syntactic overhead. (This statement does *only* apply to the immediate
> call - it does not apply to general lifetime management.)
But lifetime management is a very important issue, and as another poster
has pointed out, the pointer solution is very easy to mess up by passing
NULL or the address of a temporary object (either like his example,
&TYPE(), or by passing &function_returning_a_temporary()).
Also, you can't talk about using pointers (and even to some extent
references) without having to consider the copy semantics of the class,
which complicates matters further.
And last but not least, you've recommended a const pointer, which in my
book is "type* const p", however the original poster has used a pointer
to const data in his example implementation ("const type* p") - so we
get into easy to misunderstand terminology as well.
Too easy to make a mistake in the future, even in good faith. So while
indeed he can fool the compiler into giving in, I think there's a strong
posibility that the solution is too smart for it's own good.
Just my 2 cents,
--
Razvan Cojocaru
KeyID: 0x04CA34DE
Goran
> Refs and Ptrs are functionally equivalent, I agree.
I don't ;-). Reference means: here's a reference to an object. Pointer
means: here's an ___optional___ reference to an object (IOW, if 0,
there's no object).
OTOH, neither references nor do anything to resolve lifetime issues.
In that light, they ___are___ equivalent ;-).
(I don't want too pick on you, please don't take it that way).
Goran.
Goran
But I didn't say that. I hoped that was clear through text structure
(I commented on your proposal in post scriptum).
> What I continue to claim is that /if/ you need to reference
> any object like in the OP, then you should use a pointer [1], because
> the conversion rules for (const-)references make it nearly impossible to
> get it right.
Despite examples of when your proposal breaks? (BTW, they're not mine,
really, and are quite obvious, too; I feel it just adds more dirt on
the screen, in the form of address-of operator, but does not solve the
problem).
Goran.
Martin B.
> On Jun 1, 7:59 pm, "Martin B." <0xCDCDC...@gmx.at> wrote:
>> Goran wrote:
>>> On May 30, 1:39 am, M Stefan <mstefa...@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.
>>>> (....)
>>>> Please let me know if you have any better solutions to my problem.
>>> 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
>> What I continue to claim is that /if/ you need to reference
>> any object like in the OP, then you should use a pointer [1], because
>> the conversion rules for (const-)references make it nearly impossible to
>> get it right.
>
> Despite examples of when your proposal breaks? (BTW, they're not mine,
> really, and are quite obvious, too; I feel it just adds more dirt on
> the screen, in the form of address-of operator, but does not solve the
> problem).
>
Regarding the example with NULL:
If the class uses a pointer member, then at least this is detectable at
runtime. (e.g. assert) That's why I recommended in another post to also
use a pointer as the member.
Regarding address-of-temp:
Yes you can do this. However, I do think that this is a lot harder to
get wrong than the version with the const-ref. const-ref will silently
create a temporary to do implicit conversion, pointer version will never
do this.
Somehow I have the feeling we are talking about different things:
I say: *If* you need to track another object, use a const-pointer and
especially use a const-pointer for argument passing to initialize your
tracking and never use a const-ref for this, as it will be easily
initialized with a temporary object without you noticing.
You (seem to) say: Don't track another object as you may mess up
lifetime management.
:-)
cheers,
Martin