bug in boost::multi_index::sequenced splice (patch)
Hi, I guess there is a bug in boost/multi_index/sequenced_index.hpp. The following program fails to compile, because there is no matching operator== for class C. But this operator shouldn't be required for splice. #include "boost/multi_index/sequenced_index.hpp" #include "boost/multi_index_container.hpp" struct C { }; int main() { using namespace boost; using namespace boost::multi_index; typedef multi_index_container<C, indexed_by<sequenced<> > > Cs; Cs cs; cs.push_back(C()); cs.splice(cs.end(), cs, cs.begin()); // XXX: compile error } The following program compiles, but both assertions fail at runtime: #include "boost/multi_index/sequenced_index.hpp" #include "boost/multi_index_container.hpp" int main() { using namespace boost; using namespace boost::multi_index; typedef multi_index_container<int, indexed_by<sequenced<> > > ints; ints foo; foo.push_back(42); ints bar; bar.push_back(42); foo.splice(foo.end(), bar, bar.begin()); assert(foo.size() == 2); assert(bar.size() == 0); } I have made both tests with the boost library in Debian/unstable (version 1.33.1-7). I have also compared the source file sequenced_index.hpp with the CVS-MAIN version (1.12). I guess the following patch should fix this bug: --- sequenced_index.hpp.orig 2006-09-29 11:38:06.000000000 +0200 +++ sequenced_index.hpp 2006-09-29 11:39:43.000000000 +0200 @@ -355,7 +355,7 @@ BOOST_MULTI_INDEX_CHECK_DEREFERENCEABLE_ITERATOR(i); BOOST_MULTI_INDEX_CHECK_IS_OWNER(i,x); BOOST_MULTI_INDEX_SEQ_INDEX_CHECK_INVARIANT; - if(x==*this){ + if(&x==this){ if(position!=i)relink(position.get_node(),i.get_node()); } else{ @@ -388,7 +388,7 @@ BOOST_MULTI_INDEX_CHECK_IS_OWNER(last,x); BOOST_MULTI_INDEX_CHECK_VALID_RANGE(first,last); BOOST_MULTI_INDEX_SEQ_INDEX_CHECK_INVARIANT; - if(x==*this){ + if(&x==this){ BOOST_MULTI_INDEX_CHECK_OUTSIDE_RANGE(position,first,last); if(position!=last)relink( position.get_node(),first.get_node(),last.get_node()); regards, Hubert
----- Mensaje original ----- De: Hubert Schmid <kf1gmtvro69bnet5@expire.z42.de> Fecha: Viernes, Septiembre 29, 2006 4:48 pm Asunto: [Boost-users] bug in boost::multi_index::sequenced splice (patch) Para: boost-users@lists.boost.org CC: Hubert Schmid <kf1gmtvro69bnet5@expire.z42.de>
Hi,
I guess there is a bug in boost/multi_index/sequenced_index.hpp. The following program fails to compile, because there is no matching operator== for class C. But this operator shouldn't be required for splice. [...] The following program compiles, but both assertions fail at runtime: [...] I have made both tests with the boost library in Debian/unstable (version 1.33.1-7). I have also compared the source file sequenced_index.hpp with the CVS-MAIN version (1.12). I guess the following patch should fix this bug:
--- sequenced_index.hpp.orig 2006-09-29 11:38:06.000000000 +0200 +++ sequenced_index.hpp 2006-09-29 11:39:43.000000000 +0200 @@ -355,7 +355,7 @@ BOOST_MULTI_INDEX_CHECK_DEREFERENCEABLE_ITERATOR(i); BOOST_MULTI_INDEX_CHECK_IS_OWNER(i,x); BOOST_MULTI_INDEX_SEQ_INDEX_CHECK_INVARIANT; - if(x==*this){ + if(&x==this){ if(position!=i)relink(position.get_node(),i.get_node()); } else{ @@ -388,7 +388,7 @@ BOOST_MULTI_INDEX_CHECK_IS_OWNER(last,x); BOOST_MULTI_INDEX_CHECK_VALID_RANGE(first,last); BOOST_MULTI_INDEX_SEQ_INDEX_CHECK_INVARIANT; - if(x==*this){ + if(&x==this){ BOOST_MULTI_INDEX_CHECK_OUTSIDE_RANGE(position,first,last); if(position!=last)relink( position.get_node(),first.get_node(),last.get_node());
Hello Hubert, This is indeed a bug, and a gross one I must embarrassingly admit :) Also, the proposed patch is the way to go. Thank you very much for spotting this, I'll be committing the fixes next Monday (during weekends I'm CVS-disabled.) FYI, the same probem also shows at other places like random-access indices and some other ops like merge, yet the reference docs use the correct expression &x==this everywhere, it's so disturbing, I wonder how this could go unnoticed for so long. Best regards, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
JOAQUIN LOPEZ MU?Z wrote:
----- Mensaje original -----
... I guess there is a bug in boost/multi_index/sequenced_index.hpp. ...
This is indeed a bug, and a gross one I must embarrassingly admit :) Also, the proposed patch is the way to go. Thank you very much for spotting this, I'll be committing the fixes next Monday (during weekends I'm CVS-disabled.) FYI, the same problem also shows at other places like random-access indices and some other ops like merge, yet the reference docs use the correct expression &x==this everywhere, it's so disturbing, I wonder how this could go unnoticed for so long.
Well, ask yourself why your package's regression tests don't cover these cases -- and make sure they do, for all of whatever code changes you're about to make. This is critical for my own project; I was just about to commit to using multi_index, but now I'm not so sure of its reliability. I take it this would mean that, at a minimum, I would be unable to use the 1.32.0 release already installed with the OS, and I would need to pull directly from CVS after your changes go in, at least until 1.34.0 shows up. That makes me wonder as well: is there some way to post a warning on the Boost site that this package is broken in the current releases? Glenn
Glenn ha escrito:
JOAQUIN LOPEZ MU?Z wrote:
----- Mensaje original -----
... I guess there is a bug in boost/multi_index/sequenced_index.hpp. ...
This is indeed a bug, and a gross one I must embarrassingly admit :) Also, the proposed patch is the way to go. Thank you very much for spotting this, I'll be committing the fixes next Monday (during weekends I'm CVS-disabled.) FYI, the same problem also shows at other places like random-access indices and some other ops like merge, yet the reference docs use the correct expression &x==this everywhere, it's so disturbing, I wonder how this could go unnoticed for so long.
Well, ask yourself why your package's regression tests don't cover these cases -- and make sure they do, for all of whatever code changes you're about to make. This is critical for my own project; I was just about to commit to using multi_index, but now I'm not so sure of its reliability.
Well, if you think I can't meet the reliability standards that you mandate of the 3rd party SW you use, then by all means don't use the lib --luckily for you, this is *not* critical for your project since you haven't commited to using Boost.MultiIndex yet. Let me also state, however, that I find your reaction to this (fixed as of three days later) bug report quite bewildering; what kind of bug-free software are you using?
I take it this would mean that, at a minimum, I would be unable to use the 1.32.0 release already installed with the OS, and I would need to pull directly from CVS after your changes go in, at least until 1.34.0 shows up.
That's correct if you use or plan to use sequenced indices splice or merge. Otherwise, this particular problem does not affect you.
That makes me wonder as well: is there some way to post a warning on the Boost site that this package is broken in the current releases?
You'd have to convince one of the Boost developers with write access to the CVS that this issue is important enough that she enter the warning for you.
Glenn
Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
Joaquín Mª López Muñoz wrote:
Glenn ha escrito:
... it's so disturbing, I wonder how this could go unnoticed for so long. Well, ask yourself why your package's regression tests don't cover these cases -- and make sure they do, for all of whatever code changes you're about to make. This is critical for my own
JOAQUIN LOPEZ MU?Z wrote: project; I was just about to commit to using multi_index, but now I'm not so sure of its reliability.
Well, if you think I can't meet the reliability standards that you mandate of the 3rd party SW you use, then by all means don't use the lib --luckily for you, this is *not* critical for your project since you haven't commited to using Boost.MultiIndex yet. Let me also state, however, that I find your reaction to this (fixed as of three days later) bug report quite bewildering; what kind of bug-free software are you using?
Sorry the phrasing of my reaction came across so strongly. I had just learned of multi_index on Friday, and just two days later found it had this long-latent bug. I do very much appreciate the effort that you've put into this package and its documentation. Glenn
Hubert Schmid ha escrito:
Hi,
I guess there is a bug in boost/multi_index/sequenced_index.hpp. The following program fails to compile, because there is no matching operator== for class C. But this operator shouldn't be required for splice.
[...] Hello again, FYI, the issues are fixed both at the CVS trunk and at RC_1_34_0. You might want to confirm that a CVS checkout solves your problem. Thank you again for reporting the bug, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
participants (5)
-
"JOAQUIN LOPEZ MU?Z" -
Glenn -
Glenn Herteg
-
Hubert Schmid -
Joaquín Mª López Muñoz