Short answer: User modify
instead, details in the accepted answer and also this answer
I'm attempting to use a boost::multi_index_container
holding a pointer type. It appears to me the replace
function is broken, and am wondering what I am doing wrong.
The following code demonstrates two cases: first a container holding a copy of the data (which works correctly) and secondly a container holding a pointer to the data (which fails).
using namespace boost::multi_index;
using boost::multi_index_container;
struct Data
{
int key1;
int key2;
};
using DataContainer =
multi_index_container<
Data,
indexed_by<
hashed_unique<member<Data, int, &Data::key1>>,
hashed_unique<member<Data, int, &Data::key2>>>>;
using DataPtrContainer =
multi_index_container<
Data*,
indexed_by<
hashed_unique<member<Data, int, &Data::key1>>,
hashed_unique<member<Data, int, &Data::key2>>>>;
TEST(DummyTest, Test1)
{
Data data{1,2};
DataContainer dataContainer;
dataContainer.insert(data);
EXPECT_EQ(1, dataContainer.get<0>().find(1)->key1);
EXPECT_EQ(2, dataContainer.get<0>().find(1)->key2);
auto iter = dataContainer.get<0>().find(1);
Data d = *iter;
d.key2 = 5;
dataContainer.replace(iter, d);
EXPECT_EQ(1, dataContainer.get<1>().find(5)->key1);
EXPECT_EQ(5, dataContainer.get<1>().find(5)->key2);
}
TEST(DummyTest, Test2)
{
Data* data = new Data{1,2};
DataPtrContainer dataContainer;
dataContainer.insert(data);
EXPECT_EQ(1, (*dataContainer.get<0>().find(1))->key1);
EXPECT_EQ(2, (*dataContainer.get<0>().find(1))->key2);
auto iter = dataContainer.get<0>().find(1);
Data* d = *iter;
d->key2 = 5;
dataContainer.replace(iter, d);
EXPECT_EQ(1, (*dataContainer.get<1>().find(5))->key1); // fail as the iterator not dereferencable
EXPECT_EQ(5, (*dataContainer.get<1>().find(5))->key2); // fail as the iterator not dereferencable
}
OK, this is not a bug in Boost.MultiIndex, but a subtle contract violation in your code. The pointerless version:
auto iter = dataContainer.get<0>().find(1);
Data d = *iter;
d.key2 = 5;
dataContainer.replace(iter, d);
is taking a copy of the contained value into d
, modifying it and then using it for replacement: so far so good. But the pointer version is breaking invariants along the way:
auto iter = dataContainer.get<0>().find(1);
Data* d = *iter;
d->key2 = 5; // #1: invariant breach here
dataContainer.replace(iter, d); // #2: unexpected behavior ensues
In #1, you're modifying an internal key of dataContainer
without its consent or knowledge: once you've done that, the touched element is incorrectly indexed. This is analogous to casting constness away in the pointerless version:
auto iter = dataContainer.get<0>().find(1);
const Data& d = *iter;
const_cast<Data&>(d).key2 = 5;
So, when #2 is executed, datacontainer
, which does not suspect you've changed its keys, simply verifies that your proposed replacement d
is equivalent to what it already has, and does nothing (no reindexing then).