Am 21.09.2016 um 08:57 schrieb Olivier Goffart:
On Mittwoch, 21. September 2016 08:01:22 CEST Mathias Hasselmann wrote:
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!

Cool. Or rather not. Anyway: Thank you for looking this up!

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.

I fear balance has changed dramatically since we switched from 32 bit to 64 bit. In the old 32 bit days our structs tended to be densely packed with our enums and ints. Many structs were well defined as they contained no padding wholes. Memcmp was working. World was good.

These days with compilers often defaulting to sizeof(int) == 4 while using a memory alignment of 8 we usually end up with horrible inefficient, very sparse structs where often half of the bytes are undefined. Actually the compiler gurus' decision to use stay with 32 bits for ints feels slightly stupid now that I think about it: No improvement in memory efficiency, but lots of undefined behavior.

Maybe some clever use of type traits can tell us if your structs contain alignment wholes?

Ciao,
Mathias
_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development

Reply via email to