| Summary: | Problems using Tp::SharedPtr as a key in QMap | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | Jonathon Jongsma <jonathon> |
| Component: | tp-qt | Assignee: | Olli Salli <ollisal> |
| Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | normal | ||
| Priority: | medium | ||
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
patch to add operator< and qHash
Fix the implicit bool conversion and make shared pointers usable in hash tables |
||
Created attachment 39966 [details] [review] Fix the implicit bool conversion and make shared pointers usable in hash tables I started working on this, but then I got switched to some other task. The patch adds tests for the expected behaviour wrt. bool conversion and hash tables and then fixes the related issues in the code. I would have liked to split this work in some nicely separated and clean patches, but I didn't have time before switching to some other tasks. Sorry for this. Fix will be in either 0.3.14 or 0.5.0, will look into it ASAP. Only feasible cleanly for 0.5.0. Fix merged to master. Will be in 0.5.0. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Created attachment 35238 [details] patch to add operator< and qHash When you try to use a SharedPtr in a QMap as follows: QMap<Tp::SharedPtr<T>, int> The compiler will happily allow you to do so. However, when running the application, you'll notice some strange behavior. note that QMap (like std::map) requires that the type used for a key have operator< defined in order to determine the order of th eelements in the map. tp-qt4 does not provide operator< for SharedPtrs, so the compiler looks for a suitable comparison operator and notices that SharedPtr can be implicitly converted to bool (due to the unfortunate definition of 'SharedPtr::operator bool()') and so it uses the following operator to determine element order: operator<(bool a, bool b) So when you insert a new element into the map, it will compare the new key with the old key by first converting them to booleans. The impact of this is that any non-null sharedptr will be interpreted as being the exact same key value. This means that the maximum number of elements in this map is 2: one non-null SharedPtr and one null SharedPtr. A user can define an appropriate operator< herslef, but it's not immediately obvious that this is necessary. It would be good if the library itself provided the comparison operator. I'm providing a patch that adds a few comparison operators and a qHash() implementation for SharedPtr. Note that this does not fix all of the unfortunate consequences of defining the 'operator bool()', since the following line will still happily compile despite it being quite obviously nonsense: int i = SharedPtr<T>(new T()); But that's a different bug report...