c++boostboost-multi-index

Why can't `boost::multi_index replace` be used for pointer types?


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

}

Solution

  • 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).