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

[Olivier Goffart]

On Donnerstag, 22. September 2016 22:58:09 CEST André Pönitz wrote:
> On Thu, Sep 22, 2016 at 06:04:58AM +0200, Mathias Hasselmann wrote:
> > Am 22.09.2016 um 00:58 schrieb André Pönitz:
> > >On Wed, Sep 21, 2016 at 08:57:01AM +0200, Olivier Goffart wrote:
> > >>>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.
> > >
> > >Forbidding types without operator== in QVariants is not needed,
> > >not even if one wanted to use associative container with
> > >QVariants as keys.
> > >
> > >[Pseudocode]
> > >
> > >bool operator(QVariant(Foo) a, QVariant(Bar) b)
> > >{
> > >
> > >    if Foo != Bar:
> > >        return false
> > >    
> > >    if Foo has no operator==():
> > >        return true
> > >    
> > >    return (Foo)a == (Foo)b
> > >
> > >}
> > >
> > >establishes an equivalance relation by lumping all "uncomparable"
> > >objects into a single equivalency class.
> > 
> > I rather was considering to return false.
> 
> 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? 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();
     }
  }

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==


What use case did you have in mind where a reflexive relation is any usefull?
There is the case of the key of a QHash or in a QSet, but even then i'm not 
sure it is wise that all QVariant of the same type map to the "same" value.

So let's be pragmatic here and do something usefull rather than something 
methematicaly correct, but useless and that break lot of code.


> 
> > But indeed. Forcing all that types into a single equivalency class feels
> > that unnatural, that users should notice that issue much quicker, than if
> > we'd return false.
> > 
> > Would that be sufficient to warn Qt users, or would we also have to print
> > a warning?
> 
> I don't have an opinion on that.
> 



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".


-- 
Olivier

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