Search code examples
c++c++11setsmart-pointersweak-ptr

A set of weak_ptr


Here is the code:

struct lex_compare {
    bool operator() (const weak_ptr<int> &lhs, const weak_ptr<int> &rhs)const {
        return *lhs.lock() < *rhs.lock();
    }
};

int main(){
    set<weak_ptr<int>,lex_compare> intset;
    intset.insert(make_shared<int>(1));

    cout << "intset size:" << intset.size() << endl; //1
    cout << "Does 1 exist?"<< intset.count(make_shared<int>(1))<<endl; // failed

}

I want to know how to count/find the weak_ptr<int> stored in intset and if a better method is available to do the same work?


Solution

  • You cannot insert temporary shared_ptr to set of weak pointers because it is memory leak in the sense that this stored weak pointer points to already deleted memory.

    intset.insert(make_shared<int>(1)); 
    // after this instruction shared_ptr destructor frees the memory
    

    That is why you cannot find it in set - because *lhs.lock() is UB here.

    See weak_ptr::lock doc.

    You need to make your òrder operator in this way:

    struct lex_compare {
        bool operator() (const weak_ptr<int> &lhs, const weak_ptr<int> &rhs)const {
            auto lptr = lhs.lock(), rptr = rhs.lock();
            if (!rptr) return false; // nothing after expired pointer 
            if (!lptr) return true;  // every not expired after expired pointer
            return *lptr < *rptr;
        }
    };
    

    All that means - you need to have this shared_ptr sowmewhere to count it:

    int main(){
        set<weak_ptr<int>,lex_compare> intset;
        auto shared1 = make_shared<int>(1); 
        intset.insert(shared1);
    
        cout << "intset size:" << intset.size() << endl; //1
        cout << "Does 1 exist?"<< intset.count(make_shared<int>(1))<<endl; // failed
    }
    

    With the above - your count will work.

    Consider also to keep shared_ptr in set...

    [UPDATE]

    marko in comments pointed the valid issue. std::weak_ptr cannot be used as a key in a way you are using it at all. Only if you can ensure that pointed value will never change nor pointer itself will never expire. See this example:

        set<weak_ptr<int>,lex_compare> intset;
        auto shared1 = make_shared<int>(1); 
        intset.insert(shared1);
        cout << "Does 1 exist?"<< intset.count(make_shared<int>(1))<<endl; // works
        shared1.reset();
        cout << "Does 1 exist?"<< intset.count(make_shared<int>(1))<<endl; // failed
    

    And the other example:

        set<weak_ptr<int>,lex_compare> intset;
        auto shared1 = make_shared<int>(1); 
        intset.insert(shared1);
        cout << "Does 1 exist?"<< intset.count(make_shared<int>(1))<<endl; // works
        *shared1 = 2;
        cout << "Does 1 exist?"<< intset.count(make_shared<int>(1))<<endl; // failed
    

    You can keep std::shared_ptr which prevents from out-of-set expiration of pointer - and std::shared_ptr has operator < - but this operator compares pointers themselves - not the pointed values - so better is std::set<std::shared_ptr<int>> - but the best would be std::set<int>

    Or change std::set<...> --> std::vector<std::weak_ptr<int>> - and use count_if-- see:

    vector<weak_ptr<int>> intset;
    auto shared1 = make_shared<int>(1);
    intset.push_back(shared1);
    cout << "Does 1 exist?"<< count_if(begin(intset), end(intset), 
                                      [](auto&& elem) 
                                      { 
                                         auto ptr = elem.lock();
                                         return ptr && *ptr == 1; 
                                      }); 
    

    Or with std::set<std::shared_ptr<int>>:

    set<shared_ptr<int>> intset;
    auto shared1 = make_shared<int>(1);
    intset.insert(shared1);
    // if you can ensure shared1 value will not change:
    cout << "Does 1 exist?"<< intset.count(shared1);
    // if not  - use count_if - the slower than std::count
    cout << "Does 1 exist?"<< count_if(begin(intset), end(intset), 
                                      [](auto&& ptr) 
                                      { 
                                         return ptr && *ptr == 1; 
                                      });