[Development] Should QVariant be doing fuzzy comparisons on doubles?

[Olivier Goffart]

On Mittwoch, 21. September 2016 08:01:22 CEST Mathias Hasselmann wrote:
> Am 20.09.2016 um 12:44 schrieb Olivier Goffart:
> > On Dienstag, 20. September 2016 12:21:11 CEST Mathias Hasselmann wrote:
> >> Am 19.09.2016 um 23:27 schrieb Olivier Goffart:
> >>> We really cannot have a qHash for QVariant anyway, because that would
> >>> imply
> >>> that ALL QVariant can be hashed, and compared. Which is not the case.
> >>> Most
> >>> custom types don't even register comparator function.
> >> 
> >> Which actually is quite a serious issue, as we do a simply memcmp() on
> >> such custom types, which simply won't work if your custom data structure
> >> contains uninitialized memory from alignment padding.
> > 
> > We don't do that. We just return false in that case.
> 
> Valgrind is telling a different story. It blames uninitialized memory
> access via memcpy() when comparing custom types without registered
> compare operator. Those types are gadgets, maybe that's making the
> difference.
> 
> As much as I'd like to debug this code now to prove me right, I sadly
> have deadlines to meet this week. So I have to behave myself to not dig
> up the code right now. Maybe later. Or someone else.

Turns out you are right. If there is no registered comparison function, we do 
a memcmp in 'customCompare' in qvariant.cpp:
https://code.woboq.org/qt5/qtbase/src/corelib/kernel/qvariant.cpp.html#1079

That is indeed an undefined behavior!

> 
> >> It is easy to forget registering comparator functions and currently Qt
> >> doesn't help in debugging such issues. So I wonder if QVariant should
> >> forbid comparison of custom types without having a comparator function
> >> registered.
> > 
> > That's a source incompatible change.
> 
> No, it's not. It's changing undefined behavior into defined behavior.

But in many case, we want to put something in a QVariant, and we never compare 
this variant. 
Forbidding types that do not have an operator== to be in a QVariant might be 
to strict.


-- 
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org