initial value of reference to non-const must be an lvalue
Markus Werle
Hi!
I always feel a little uncomfortable when it comes to the
lvalue/rvalue error messages, because it guess I have not
understood this very well.
Please try to explain why this toy code here has a problem.
I was trying to "optimize" away some copy assignments
between std::lists when I fell into this trap.
For compilers this seems not to be a well-known error.
gcc-3.0: not catched (wrong executable),
KCC: warns - and creates code working the way I wanted (!)
hp's aCC: no warning - again wrong executable.
The problem is with operator, when a reference to a list
as argument is itself returned as a reference.
Can You explain it for the stupid?
#include <list>
#include <iostream>
#include <iterator>
#define MARK() { \
std::cerr << "file " \
<< __FILE__ << " line " << __LINE__ << std::endl; }
///////////////////////////////////////////////////////////////////////////////
// Some example of Value Holder
///////////////////////////////////////////////////////////////////////////////
class Value {
public:
Value(const double val) : val_(val) {}
double data() const { return val_; }
private:
double val_;
};
std::ostream& operator<< (std::ostream& os, const Value& V) {
os << V.data();
return os;
}
///////////////////////////////////////////////////////////////////////////////
void DebugPrint(std::list<const Value*>& lexpr) {
std::cerr << "Values stored:\n ";
typedef std::list<const Value*>::const_iterator iterator;
iterator end = lexpr.end();
for (iterator iter = lexpr.begin(); iter != end; ++iter) {
std::cerr << (**iter) << std::endl;
}
}
///////////////////////////////////////////////////////////////////////////////
// create a list out of Value, Value
std::list<const Value*>
operator,(const Value& a, const Value& b) {
std::list<const Value*> tmp;
tmp.push_back(&a);
tmp.push_back(&b);
MARK();
DebugPrint(tmp);
return tmp;
}
///////////////////////////////////////////////////////////////////////////////
//*****************************************************************************
// BIG problem!
std::list<const Value*>&
operator,(std::list<const Value*>& li, const Value& a) {
li.push_back(&a);
MARK();
DebugPrint(li);
return li;
}
//*****************************************************************************
// END OF PROBLEM
// fixed version (uses copy)
// std::list<const Value*>
// operator,(std::list<const Value*> li, const Value& a) {
// li.push_back(&a);
// MARK();
// DebugPrint(li);
// return li;
// }
class Output {
public:
void operator<<(const std::list<const Value*>& l) {
std::cout << "\nResult:\n";
typedef std::list<const Value*>::const_iterator iterator;
iterator end = l.end();
for (iterator iter = l.begin(); iter != end; ++iter) {
std::cout << (**iter) << std::endl;
}
}
};
int main() {
Value a(1.0), b(2.0), c(3.0), d(4.0);
Output O;
O << (a, b, c, d);
}
[ Send an empty e-mail to c++-...@netlab.cs.rpi.edu for info ]
[ about comp.lang.c++.moderated. First time posters: do this! ]
Martin Fabian
>
<snip>
> The problem is with operator, when a reference to a list
> as argument is itself returned as a reference.
>
> // BIG problem!
> std::list<const Value*>&
> operator,(std::list<const Value*>& li, const Value& a) {
> li.push_back(&a);
> MARK();
> DebugPrint(li);
> return li;
> }
>
<code snipped>
BCC5.5 compiles this fine, while MSVC++6 complains about a use of
operator, in xstring (which implements std::string). I guess your
definition of operator, intrudes on some templates. I haven't tried to
get to the heart of the problem, though, for two reasons:
1. Why would you want to define operator, in the first place? It's
obscure, easily missed by the reader and leads to confusing code. It's
not even always obvious where operator, is actually invoked (not to us
non-gurus, at least). Avoid using it at all, not to mention redefining
it.
2. You've posted too much code. Please try to strip your posted code
down to the bare essentials, the smallest compilable code that exhibits
the problem. For one thing, it helps your helpers understanding the
problem. For another, it helps yourself understanding the problem; many
times I've found out the real problem while stripping down the code.
--
Martin Fabian http://www.s2.chalmers.se/~fabian/
--
Ask enough experts. Eventually you'll get the answer you want.
/* Remove NOSPAM from reply-to address to mail me */
Markus Werle
> BCC5.5 compiles this fine, while MSVC++6 complains about a use of
> operator, in xstring (which implements std::string). I guess your
> definition of operator, intrudes on some templates.
So if compiler vendors are not sure on this code how could I
get it then?
My question was about code correctness and ANSI compliance.
If someone can explain the error messages and misbehaviour
I will be satisfied.
> Please try to strip your posted code
> down to the bare essentials, the smallest compilable code that exhibits
> the problem.
Let us see if this really helps You further:
(of course now I had to strip all the DebugPrint etc.
It was too much code ... ;-)
--- new-version ----------------
#include <list>
#include <iostream>
struct Value {
Value(const double d) : val(d) {}
double val;
};
std::list<const Value*>
operator,(const Value& a, const Value& b) {
std::list<const Value*> tmp;
tmp.push_back(&a);
tmp.push_back(&b);
return tmp;
}
// BIG problem! reference makes the head bang
std::list<const Value*>&
operator,(std::list<const Value*>& li, const Value& a) {
li.push_back(&a);
return li;
}
int main() {
Value a(1.0), b(2.0), c(3.0), d(4.0);
std::list<const Value*> l = (a, b, c, d); // how convenient !
for (std::list<const Value*>::iterator iter = l.begin();
iter != l.end(); ++iter) {
std::cout << (*(*iter)).val << std::endl;
}
}
--- end new version ------------
> I haven't tried to
> get to the heart of the problem, though, for two reasons:
>
> [ 1. see below ]
> 2. You've posted too much code.
IMHO one single page of code is not too much code.
The point was clear. It was readable and contained
enough comfort code to really test it with different compilers.
> For one thing, it helps your helpers understanding the
> problem. For another, it helps yourself understanding the problem;
May gain in understanding this already converged to zero.
Condition tested before this->Post(comp.lang.c++.moderated);
> many
> times I've found out the real problem while stripping down the code.
I disagree.
Nothing becomes clearer in the new version.
It is just a little bit smaller, but the original version
contained stuff every STL-user reads like a "Helle World".
Hello experts!
What is wrong with the code?
> 1. Why would you want to define operator, in the first place?
I do not want to discuss the why and why not.
Using a special C++ feature can always have a reason.
I for myself have a really good one: comfort.
> It's
> obscure, easily missed by the reader and leads to confusing code.
I think it is safe to use it here, isn't it?
How can You misuse it in that context?
> It's
> not even always obvious where operator, is actually invoked (not to us
> non-gurus, at least).
operator, is invoked when a comma occurs between
objects of type Value. When else do You expect it to choke?
> Avoid using it at all, not to mention redefining
> it.
Who gave this advice?
Markus
Francis Glassborow
<mar...@lufmech.rwth-aachen.de> writes
>--- new-version ----------------
>#include <list>
>#include <iostream>
>
>struct Value {
> Value(const double d) : val(d) {}
> double val;
>};
>
>
>std::list<const Value*>
>operator,(const Value& a, const Value& b) {
> std::list<const Value*> tmp;
> tmp.push_back(&a);
> tmp.push_back(&b);
> return tmp;
>}
>
>
>// BIG problem! reference makes the head bang
>std::list<const Value*>&
>operator,(std::list<const Value*>& li, const Value& a) {
> li.push_back(&a);
> return li;
>}
>
>
>int main() {
> Value a(1.0), b(2.0), c(3.0), d(4.0);
>
> std::list<const Value*> l = (a, b, c, d); // how convenient !
Not if it does not work :)
a,b returns a temporary std::list<const Value*> which is an rvalue and
so cannot be bound to the first parameter of your second operator,()
because that takes non-const reference (if it did not you could not
change it before returning a reference to it, and you could not return a
non-const reference. OK, you do not like this, but that is the way the
language is required to work.
Now you say you have good reasons for overloading operator,(); fine but
unless you tell us them they are not reasons for changing the language.
If you are simply wanting to initialise a vector use the simpler idiom:
Value vals[4] = {Value(a), Value(b), Value(c), Value(d)};
std::list<Value const *> l(vals, vals+4);
>
> for (std::list<const Value*>::iterator iter = l.begin();
> iter != l.end(); ++iter) {
> std::cout << (*(*iter)).val << std::endl;
> }
>}
Francis Glassborow ACCU
64 Southfield Rd
Oxford OX4 1PA +44(0)1865 246490
All opinions are mine and do not represent those of any organisation
Nicola Musatti
Markus Werle wrote:
>
> Martin Fabian <fabian...@s2.chalmers.se> writes:
[...]
> My question was about code correctness and ANSI compliance.
> If someone can explain the error messages and misbehaviour
> I will be satisfied.
The error message states in accordance with the standard that a
reference to a non const type must be initialized with an lvalue, that
is, roughly, something that can be addressed. The term lvalue refers
historically to something that can sit on the left side of an assignment
statement, i.e. something you can assign to. Specifically, temporary
values as returned by functions are generally *not* lvalues.
[...]
> #include <list>
> #include <iostream>
>
> struct Value {
> Value(const double d) : val(d) {}
> double val;
> };
>
> std::list<const Value*>
> operator,(const Value& a, const Value& b) {
> std::list<const Value*> tmp;
> tmp.push_back(&a);
> tmp.push_back(&b);
> return tmp;
> }
>
> // BIG problem! reference makes the head bang
> std::list<const Value*>&
> operator,(std::list<const Value*>& li, const Value& a) {
> li.push_back(&a);
> return li;
> }
>
> int main() {
> Value a(1.0), b(2.0), c(3.0), d(4.0);
>
> std::list<const Value*> l = (a, b, c, d); // how convenient !
I suspect that the first call is to your first comma operator (the only
one that can match), which returns a temporary list, which in turn is
not a valid match for the list reference parameter in your second comma
operator.
[...]
> > I haven't tried to
> > get to the heart of the problem, though, for two reasons:
> >
> > [ 1. see below ]
> > 2. You've posted too much code.
>
> IMHO one single page of code is not too much code.
> The point was clear. It was readable and contained
> enough comfort code to really test it with different compilers.
I originally didn't even bother checking your code for the same reason
as Martin, even though I formed the opinion I exposed above from your
subject line. What you consider comfort code actually only clouds the
issue.
> > For one thing, it helps your helpers understanding the
> > problem. For another, it helps yourself understanding the problem;
>
> May gain in understanding this already converged to zero.
> Condition tested before this->Post(comp.lang.c++.moderated);
Pity. You should at least have learned that the best way to deal with a
problem such as yours is to chop away from your code all the things that
are not relevant to your problem. The smallest example of what your
problem is is probably something like:
[not tested]
int f() { return 5; }
int g(int & i) { return i; }
int main() { return g(f()); }
> > many
> > times I've found out the real problem while stripping down the code.
>
> I disagree.
> Nothing becomes clearer in the new version.
> It is just a little bit smaller, but the original version
> contained stuff every STL-user reads like a "Helle World".
Which we find soooo boring that we are not willing to wade through it,
especially since you aren't also.
[...]
> > 1. Why would you want to define operator, in the first place?
>
> I do not want to discuss the why and why not.
> Using a special C++ feature can always have a reason.
> I for myself have a really good one: comfort.
A false sense of confort, more exactly.
> > It's
> > obscure, easily missed by the reader and leads to confusing code.
>
> I think it is safe to use it here, isn't it?
> How can You misuse it in that context?
It is a non obvious use of an obscure functionality of the language.
Personally, I almost never use the comma operator, let alone redefine
it.
> > It's
> > not even always obvious where operator, is actually invoked (not to us
> > non-gurus, at least).
>
> operator, is invoked when a comma occurs between
> objects of type Value. When else do You expect it to choke?
Not necessarily, as that may be the separator between actual parameters
in a function call.
> > Avoid using it at all, not to mention redefining
> > it.
>
> Who gave this advice?
Martin and I for starters, and I'm sure many others on the newsgroup
would join us.
Cheers,
Nicola Musatti
Ron Hunsinger
> Martin Fabian <fabian...@s2.chalmers.se> writes:
<snip>
> --- new-version ----------------
> #include <list>
> #include <iostream>
>
> struct Value {
> Value(const double d) : val(d) {}
> double val;
> };
>
>
> std::list<const Value*>
> operator,(const Value& a, const Value& b) {
> std::list<const Value*> tmp;
> tmp.push_back(&a);
> tmp.push_back(&b);
> return tmp;
> }
>
>
> // BIG problem! reference makes the head bang
> std::list<const Value*>&
> operator,(std::list<const Value*>& li, const Value& a) {
> li.push_back(&a);
> return li;
> }
>
>
> int main() {
> Value a(1.0), b(2.0), c(3.0), d(4.0);
>
> std::list<const Value*> l = (a, b, c, d); // how convenient !
>
> for (std::list<const Value*>::iterator iter = l.begin();
> iter != l.end(); ++iter) {
> std::cout << (*(*iter)).val << std::endl;
> }
> }
> --- end new version ------------
> Hello experts!
> What is wrong with the code?
>
>
> > 1. Why would you want to define operator, in the first place?
>
> I do not want to discuss the why and why not.
> Using a special C++ feature can always have a reason.
> I for myself have a really good one: comfort.
Quite right. The question here is not "why did you do it?" but rather "why
didn't it work?" Answering the latter question can provide insights into
the language and how its features interact. And, yes,
may also provide insight into whether it was a good idea in the first
place, but that judgement call should be supported by concrete examples of
problems it causes.
In this case, the problem boils down to the question of what happens when
temporaries are bound to references. In the line:
> std::list<const Value*> l = (a, b, c, d); // how convenient !
look at how the three commas are bound to operators. First off, the comma
operator is left associative, so (a, b, c, d) evaluates as:
(((a, b) , c) , d)
The subexpression (a, b) combines two Value operands, to produce a
*temporary* object of type std::list<const Value*>.
You've defined an operator,() that takes a non-const reference to such a
list, but that parameter cannot be bound to a temporary. (Only const
references can be bound to temporaries.)
That means the second comma, at the top level of ((a,b),c), can only be
interpreted as the built-in operator,, the one that discards its left
argument and evaluates the right operand. The value of ((a,b),c) is just c.
Read the last two paragraphs again. They express the crux of the issue.
Then finally, the last comma invokes again your operator,() to combine two
Value operands into another temporary list, containing only pointers to c
and d. That temporary is used to initialize l.
I see three ways to make this work the way you expect. All are unattractive.
First method: redefine your second operator,() so it takes a *const*
reference to a list<const Value*> as its first parameter, then cast away
the constness so you can call push_back. That's ugly, because you can't
know that this operator will never be called on something that is truly
const. And besides, you're lying about what type of argument it takes, and
lying is bound to get you into trouble sooner or later.
Second method: as before, except that instead of casting away constness,
you copy the input parameter into a new non-const list, append to that, and
return it. Ugly because the cost to construct the list suddenly becomes
O(n^2), instead of O(n), in the number of elements.
Third method: rewrite the initialization so it uses a named list instead of
a temporary. That is, rewrite the declaration for l as:
std::list<const Value*> l; l, a, b, c, d; // how convenient!
This one isn't really so bad. The only drawback is that it isn't quite as
nice, notationally, as the original. On the other hand, it's probably
slightly more efficient (since it never constructs any temporary lists),
and generalizes nicely to the case where you have zero or one initial
element.
But if you do it this way, operator<< might be more readable than
operator,. Operator<< has already been "usurped" for another purpose than
its original, so the reader is a little bit better prepared for it to have
a non-standard meaning. The line in question would become:
std::list<const Value*> l; l << a << b << c << d;
which to my eyes looks a little better. That's purely subjective, of
course. YMMV
-Ron Hunsinger
Niklas Borson
news:<tgn15hm...@tresca.lufmech.RWTH-Aachen.DE>...
> Hi!
>
> I always feel a little uncomfortable when it comes to the
> lvalue/rvalue error messages, because it guess I have not
> understood this very well.
>
> Please try to explain why this toy code here has a problem.
> I was trying to "optimize" away some copy assignments
> between std::lists when I fell into this trap.
>
> For compilers this seems not to be a well-known error.
> gcc-3.0: not catched (wrong executable),
> KCC: warns - and creates code working the way I wanted (!)
> hp's aCC: no warning - again wrong executable.
I think this should be an error because a temporary
cannot be bound to a non-const reference.
Following are the relevant portions of your code,
with some cosmetic changes and comments added.
// I've added a typedef for convenience.
typedef std::list<const Value*> ValueList;
//
// comma operator for (Value,Value)
// Creates a list of Value pointers and returns it.
//
ValueList operator,(const Value& a, const Value& b)
{
ValueList tmp;
tmp.push_back(&a);
tmp.push_back(&b);
return tmp;
}
//
// Comma operator for (ValueList&,Value)
// Adds the given value to the given list and returns
// a reference to the list. (Note the list operand is
// a non-const reference.)
//
ValueList& operator,(ValueList& li, const Value& a)
{
li.push_back(&a);
return li;
}
//
// Example usage.
//
int main()
{
// Instantiate four values.
Value a(1.0), b(2.0), c(3.0), d(4.0);
// Stream object that takes a ValueList.
Output O;
// Here's the problem!
O << (a, b, c, d);
}
To see what's wrong with the last statement, let's break it
down into parts. The first subexpression to be evaluated is:
operator,(a,b)
Next you invoke the other comma operator, with the first
operand being the result of the first subexpression and the
second operand being c.
The problem is that result of the first expression is an
unnamed temporary of type ValueList, but the comma operator
in question takes a non-const reference to ValueList. A
temporary cannot be bound to a non-const reference. That's
what the lvalue/rvalue messages are trying to tell you.
The following rewrite should compile and work:
ValueList l;
l, a, b, c, d;
O << l;
Hope this helps,
--Nick
Markus Werle
> [...]
>
> I suspect that the first call is to your first comma operator (the only
> one that can match), which returns a temporary list, which in turn is
> not a valid match
Finally got it. Banging my head it was so simple to explain.
Thanx to Francis, too.
> for the list reference parameter in your second comma operator.
> [...]
> Pity. You should at least have learned that the best way to deal with a
> problem such as yours is to chop away from your code all the things that
> are not relevant to your problem. The smallest example of what your
> problem is is probably something like:
> [snipped]
I thought it was cooked down to a digestable format. Obviously it was not.
I promise future code examples in future postings will tend to be shorter.
I was blind for what really was the problem.
Anyway: I tested this with 3 big compilers (KCC, hp's aCC (march 2001) and
g++-3.0/3.1]) Except g++ they compiled it and even worse KCC only warned on
this big error while generating code that behaved as originally expected.
> [...]
> > Markus: I for myself have a really good one: comfort.
>
> A false sense of confort, more exactly.
> [...]
> It is a non obvious use of an obscure functionality of the language.
What is non-obvious or obscure in using operator comma to get stuff into a
list? Consider I return to my original code which used a call by value:
std::list<const Value*>
operator,(std::list<const Value*> li, const Value& a);
How can this become dangerous or choke unwanted side effects during usage
of the code?
> Personally, I almost never use the comma operator, let alone redefine
> it.
Would You prefer operator+ in this context?
> > > It's
> > > not even always obvious where operator, is actually invoked (not to us
> > > non-gurus, at least).
> >
> > operator, is invoked when a comma occurs between
> > objects of type Value. When else do You expect it to choke?
>
> Not necessarily, as that may be the separator between actual parameters
> in a function call.
Still I cannot see any problem here: function calls are not affected at
all. operator comma is only called by explicit request from the user.
Introducing functions like void Function(const Value& a, const Value& b) {}
does not break up the semantics of the existing code or vice versa.
The guy who writes Function((a,b)); knows what he does, right? (see below)
>
> > > Avoid using it at all, not to mention redefining
> > > it.
> >
> > Who gave this advice?
>
> Martin and I for starters, and I'm sure many others on the newsgroup
> would join us.
It would help me if You show up the reason for this advice and explain how
my use of operator comma makes room for code intrusion or disambiguities or
even unexpected behaviour. IMHO operator comma is not dangerous in this
context:
I act upon objects and a collection of pointers to objects and the whole
process is terminated before calling any member function, e.g. I do not
have the classical evaluation order problem that may (or may not) come in
with this operator. So how come I have to fear any side effects or
ambiguity for the user?
Anything changed since draft '96 here?
5.18 Comma operator [expr.comma]
[...]
2 In contexts where comma is given a special meaning, [Example: in lists
of arguments to functions (_expr.call_) and lists of initializers
(_dcl.init_) ] the comma operator as described in this clause can
appear only in parentheses. [Example:
f(a, (t=3, t+2), c);
has three arguments, the second of which has the value 5. ]
Markus
--
Is Pluto a planet? Yes, Pluto is a planet. [...] Other scientists [...] may
have different opinions [...], but while it may be trendy to declare that
Pluto is not a planet, it isn't accurate - the only official decision
belongs to the IAU.
Raoul Gough
> Martin Fabian <fabian...@s2.chalmers.se> writes:
[request to shorted code, etc..]
> // BIG problem! reference makes the head bang
> std::list<const Value*>&
> operator,(std::list<const Value*>& li, const Value& a) {
> li.push_back(&a);
> return li;
> }
>
>
> int main() {
> Value a(1.0), b(2.0), c(3.0), d(4.0);
>
> std::list<const Value*> l = (a, b, c, d); // how convenient !
If you had a member function instead of a free function, you wouldn't have
this problem. That is, if your *container* provided a comma-operator append
function, then this would work as you want.
[extract from code that works]
template<typename T>
class container
{
public:
container<T>& operator, (const T& t) { /*...*/ }
};
int main (void)
{
A a1(1), a2(2), a3(3), a4(4);
container<A> x = (a1, a2, a3, a4); // it works
}
Of course, you can't add a member function to std::list<>, but you could
possibly work around this with private inheritance or containment.
Regards,
Raoul Gough.
--
Don't bother trying the deja return address. You could try the same username
"at" knuut.de or via Hotmail if I've moved home address.
Tom
> Hi!
>
> I always feel a little uncomfortable when it comes to the
> lvalue/rvalue error messages, because it guess I have not
> understood this very well.
>
> Please try to explain why this toy code here has a problem.
> I was trying to "optimize" away some copy assignments
> between std::lists when I fell into this trap.
>
You've had the explanation, but how about this completely untested code:
template <class Cont>
class init_proxy
{
public:
operator Cont& {
return cont;
}
init_proxy& operator,(typename Cont::const_reference val){
cont.push_back(val);
return *this;
}
private:
Cont cont;
};
std::list<int> l(init_proxy<list<int> >(), 1, 2, 5, 9);
Tom