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

[Olivier Goffart]

On Freitag, 23. September 2016 19:45:18 CEST André Pönitz wrote:
> On Fri, Sep 23, 2016 at 11:22:08AM +0200, Olivier Goffart wrote:
> > > There is not much of a choice. An equivalence relation is reflexive,
> > > i.e. at least Foo(a) == Foo(a) must be true. Lacking the ability
> > > to compare Foos, treating them all equal at least doesn't break the
> > > relation.
> > 
> > Why do we want that kind of mathematical purity?
> 
> Because that's the only sane way to avoid ugly surprises.

An operator== returning true for two different object is a surprise to me.

In that respect I believe returning false is what will lead to less surprise.

> > This ensure we have the most  useless operator==.
> > 
> > Most of the code use it for stuff like that:
> >   void setFoo(const QVariant &foo) {
> >   
> >      if (foo != m_foo) {
> >      
> >         m_foo = foo;
> >         emit fooChanged();
> >      
> >      }
> >   
> >   }
> 
> That gives already "surprising" behaviour if fed with QChar('a') and
> QString("a") in a row.
> 
> And it is "surprising" to a degree that I'd call it buggy.

That's another problem. And yes, I agree that this looks buggy.

On the other hand, we also have have the same "42" == 42 in javascript, so it 
has some logic.


> > And suddenly returning always true if the variant has the same type will
> > break this code. And i think will break most use case for
> > QVariant::operator==
> Most of the uses of QVariant::operator==() ARE ALREADY BROKEN, and will
> not be fixable if people refuse to accept basic requirements.

What do you mean by broken? Do you mean the application misbehave. Or might 
misbehave?  Because I would think currently most usage of QVariant produce an 
application that behave properly.  (Even if admitively they may rely on the 
undefined behavior for memcmp)

And if you change to always return true, these application will misbehave.

Note: you can see all use of QVariant::operator== in Qt:
https://code.woboq.org/data/symbol.html?root=../qt5/&ref=_ZNK8QVarianteqERKS_

[...]
> > So let's be pragmatic here and do something usefull rather than something
> > methematicaly correct, but useless and that break lot of code.
> 
> This "pragmatism" has already led to the current situation where
> the number of items in a QMap with QVariant keys depend on the
> order of insertion and other crap.

That's just a bug in the implementation.
Of course Sune is right there. 
But that's again not the problem at hand.

> > Now that we have C++11 and we can use some expression SFINAE, we can do
> > 
> > something like:
> >   template<class T>
> >   auto registerEqualityOperator()
> >   
> >             -> decltype(std::declval<T>() == std::declval<T>())
> > 
> > Called from qRegisterMetaType, which automatically register the operator==
> > if it exists.
> > 
> > 
> > The QVariant::operator==  could return false if none was registered. And
> > possibly print a qWarning: "Attempting to compare two QVariant containing
> > type 'Foo' which has no equality operation registered".
> 
> That would be ok to, but does not invalidate my point.


I said "return false", when you said we should return true.