Using QExplicitlySharedPointer as a key to QMap causes reference issues.



  • When using a QExplicitlySharedDataPointer as the Key in a QMap, the qMapLessThanKey for QExplicitlySharedDataPointer is using the operator bool of the QExplicitlySharedDataPointer; Hence if you have two valid objects the map always identifies the two objects as identical and doesn't insert the second. Can QExplicitlySharedDataPointer have an implementation with a lessthan operator?

    Example code:

    #include <stdio>
    #include <QtCore/QSharedData>
    #include <QtCore/QMap>
    class Spoon : public QSharedData
    {  
        using Pointer = QExplicitlySharedDataPointer<Spoon>;
        int I; 
    }
    int main()
    {
        QMap< Spoon::Pointer, int > map;
        Spoon::Pointer a1(new Spoon);
        Spoon::Pointer a2(new Spoon);
        map.insert(a1, 12);
        std::cout << map.size() << std::endl;
        map.insert(a2, 2512);
        std::cout << map.size() << std::endl;
        return;
    }
    

    Expected output:

    1
    2

    Actual output:

    1
    1


  • Moderators

    Hi, welcome to devnet.

    I don't think QExplicitlySharedDataPointer was ever intended to be used that way.
    But QExplicitlySharedDataPointer<Spoon> is a regular user type so you can roll your own operator< for it.
    One possible implementation:

    class Spoon : public QSharedData
    {
    public:
        using Pointer = QExplicitlySharedDataPointer<Spoon>;
        int I;
    };
    bool operator<(const Spoon::Pointer& p1, const Spoon::Pointer& p2)
    {
        return p1.data() < p2.data();
    }
    

    Of course you can make any comparison criterion you want. Comparing data pointers is just a simple example.


  • Qt Champions 2016

    @Chris-Kawa

    I don't think QExplicitlySharedDataPointer was ever intended to be used that way.

    Neither do I.

    @Phillip
    From what I can seen in your code, you want to use the QSharedPointer class, not QExplicitlySharedDataPointer. Moreover if you wish to use some class as a key for the map, it's supposed to have a comparison operator, i.e. operator < (for QHash it's the equivalence operator - operator ==). QMap orders elements by it's keys, while QHash is unordered. However, I wouldn't even hazard to guess why exactly your second element is not added to the container, only thinking about the possibilities makes my head hurt.

    Kind regards.


  • Moderators

    @kshegunov said:

    However, I wouldn't even hazard to guess why exactly your second element is not added to the container

    It's not that hard actually. @Phillip already hinted at what's going on. QMap is using qMapLessThanKey function to order the elements, which happens to simply call operator< for any compared pairs. Since QExplicitlySharedDataPointer does not have an operator< defined this would be an error, except... c++ defines allowed implicit casts. It's a long read, but the punch line is that the compiler is basically allowed to make a single type conversion to satisfy function parameters in case there are no direct matches.

    That's actually the same rule that let's us write stuff like QUrl("foobar.org") even though there's no QUrl(const char*) constructor. Preventing stuff like this is also one of the reasons for the explicit keyword that we see in front of many single param Qt constructors.

    Anyway... there's no exact match for operator<(const Spoon::Pointer& p1, const Spoon::Pointer& p2) so the compiler heads out and checks what it can cast p1 and p2 to, so that the comparison operator can be called. It so happens that QExplicitlySharedDataPointer defines operator bool() and there's very well defined operator<(bool, bool) so the compiler is happy with that and casts the parameters. Since the cast will result in a true boolean for any non-nullptr, the comparisons will also always result in true. p1 < p2 = true and p2 < p1 -> true means p1 == p2 so the keys are matching and the value is overwritten instead of new one being added.
    This also means that in such map only two values can be stored at any given time - one null pointer and one non-null pointer, because these are the only ones that differ when cast to bool.

    To overcome all these shenanigans all you have to do is properly define operator< for your type so that the compiler doesn't get sneaky and cast stuff behind your back (what I proposed in my first response).

    Whew... I guess it is a little hard after all ;)


  • Qt Champions 2016

    @Chris-Kawa
    Thanks Chris for the elaborate explanation, I appreciate it!
    I already knew all that you've written, except for the QExplicitlySharedDataPointer definition of a boolean cast operator. I always thought it only provided operator T *, which wouldn't have generated this particular behavior, and which is not in fact defined at all.
    Seeing the typecasting and boolean expressions you've calculated, I'd say you're quite able to work as a compiler yourself. ;)



  • @Chris-Kawa thanks for the feedback.

    To give you an update our current solution is to use a QHash instead of a QMap.

    It was just an awkward bug to find as we were modifying legacy code and updating it when it occurred. The suggestion to modify QExplicitlySharedDataPointer is purely to prevent others experiencing difficult to track bugs thanks to this casting to bool fun.


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.