Menu
Is the following code causing undefined behavior?
std::map<int, vector<int>> foo()
{
return ...
}
BOOST_FOREACH(const int& i, foo()[42])
{
std::cout << i << std::endl;
}
If undefined, What is the good way to fix it? What if I use c++11 range-for loop instead of BOOST_FOREACH?
606
This is, unfortunately, most probably undefined behavior.
The problem is that you have two levels here:
std::map<...> is an r-value, its lifetime will be expanded until the end of the full-expressionstd::vector<int>& is an l-value reference (into an object), its lifetime is that of the object.The problem arises because the code (roughly) expands to something like:
// from
for (<init>: <expr>) {
<body>
}
// to
auto&& __container = <expr>;
for (auto __it = begin(container), __e = end(container); __it != __e; ++__it)
{
<init> = *__it;
<body>
}
The issue here is in the initialization of __container:
auto&& __container = foo()[42];
If it where just foo(), this would work, because the lifetime of std::map<...> would be extended to match that of __container, however in this case we get:
// non-standard gcc extension, very handy to model temporaries:
std::vector<int>& __container = { std::map<...> m = foo(); m[42] };
And thus __container ends up pointing into the nether.
68
The return value exists until the end of the full expression
which creates it. So it all depends on how BOOST_FOREACH
expands; if it creates a scope outside of the for loop, and
copies the return value to a variable in it (or uses it to
initialize a reference), then you're safe. If it doesn't,
you're not.
The C++11 range-for loop basically has the semantics of binding to a reference in a scope outside of a classic for-loop, so it should be safe.
EDIT:
This would apply if you were capturing the return value of
foo. As Benjamin Lindley points out, you aren't. You're
capturing the return value of operator[] on a map. And this
is not a temporary; it is a reference. So no extension of
lifetime occurs, neither in BOOST_FOREACH nor in range-for.
Which means that the map itself will be destructed at the end of
the full expression which contains the function call, and that
undefined behavior occurs. (Boost could, I suppose, make a deep
copy of the map, so you'd be safe. But somehow, I doubt that it
does.)
END OF EDIT:
Never the less, I would question the wisdom of returning an
std::map when all you want is a single entry in it. If the
map actually exists outside the function (is not on the heap),
then I'd return a reference to it. Otherwise, I'd find some
what that it did.
23
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With
