Search code examples
c++listuniqueduplicatesforward-list

In C++, is it allowed to delete objects in list<Pointer>::unique


We have got legacy code which returns huge sized lists of raw pointers to heap allocated objects (we can't use smart pointers), and we'll remove duplicates from the list and also delete them from heap.

For now, as it advised by gurus, I would like to try std::list::unique (or forward_list::unique) instead of algorithm std::unique.

I've read in http://en.cppreference.com/w/cpp/container/list/unique that within 'unique' predicate we shouldn't change objects, so is it safe by the term of standard to delete the “going to be removed” objects in list::unique?

And if so, what object in list::unique should be considered as a duplicate? In gnu implementation the 'b' will be removed, but in http://www.cplusplus.com/reference/list/list/unique/ is written that in pred(i, i-1), i item will be removed, so does this behaviour specified by standard ?

Is this (working in gcc) code correct in term of standard or is UB?

List.sort( [] (const Val *a, const Val *b) { 
    return *a < *b;
});

List.unique([] (const Val *a, const Val *b) {
    if (*a == *b) {
        delete b;  // (1) working in gcc 4.6
        // or (2) delete a (elsewhere)? 
        return true;
    }

    return false;
}) ;

Update #1

Mike's explanation was the most helpful one, but for now we're using such solution:

struct RawPtrEq {
    bool operator()(const Val a, const Val b) { return *a == *b;}
};

auto it = adjacent_find( begin(List), end(List), RawPtrEq() );

while(it != end(li)) {
    delete *it;
    it = List.erase(it);

    it = adjacent_find( it, end(List), RawPtrEq() );
}

Solution

  • No, there's no guarantee that this is well defined. unique is not specified completely enough to guarantee that this is the last time that b is passed to the predicate, so it's possible that the deleted pointer might be used again later.

    I'm surprised that it works for you, since the specification is for b to be the first of the two elements, which is the one that would be kept if the either were removed.

    I'd suggest storing either the objects themselves, or unique_ptr<Val> if you really need them to be dynamic, so that they are always destroyed automatically on removal. I've no idea why you say "we can't use smart pointers"; they would make much more sense than jumping through hoops to leave your legacy code unchanged.