[Development] QString behavior change

[Julien Blanc]

Le mardi 11 août 2015 à 10:10 -0700, Thiago Macieira a écrit : 
> On Tuesday 11 August 2015 18:45:10 Julien Blanc wrote:
> > Le mardi 11 août 2015 à 08:46 -0700, Thiago Macieira a écrit :
> > > On Tuesday 11 August 2015 10:38:27 Julien Blanc wrote:
> > > > That point is certainly valid, but i would like to raise the point that
> > > > string nullness is a *required* feature for QtSQL (a null QString is
> > > > converted to a NULL SQL, whereas a non-null empty QString is converted
> > > 
> > > That's a misfeature. QtSql should have been using something like
> > > QOptional<QString> for a nullable string. In addition, it usually uses
> > > QVariant, which has its own null setting -- it allows for a null int,
> > > different from a value of zero.
> > 
> > What are the chances that such a change can be accepted ? (i mean, i can
> > submit such a patch, but that would mean breaking a *lot* of code).
> 
> QOptional does not exist yet and for very good reasons.
> 
> QVariant is already in use, so I don't think you'd be sending a patch to use 
> QVariant.
> 
> What are you proposing to send?

Changing QtSql so that :
- null QString are binded to SQL '' (same as empty string)
- to bind to NULL you thus require a null QVariant (same as for null
int), that require no change

I don’t expect such a change to be accepted before at least Qt6, but i’d
be happy to hear i’m wrong.

> There are just too many places where strings are mutated, so we cannot offer to 
> keep the nullness stable for everything.

I will not put on the same level QString API with other mutators (like
QUrl(QString()).toString() ). In an ideal world, I would expect the
first one (QString API) to keep the nullness stable, not the second one
(imho building a QUrl with a null QString is a contract violation,
undefined behaviour is fine).

However, i got your point that this is not going to happen for
performance reasons.

> We should remove it, indeed, but only after std::optional can be used properly 
> in API that needs it (like after refactoring QtSql). I don't think it will 
> happen in time for Qt 6.

Like you said, QVariant offers an already existing (yet inferior)
alternative to std::optional in many places. It looks like there’s a
chicken and egg problem :
- null QString is used in some modules
- as long as null QString exists, modules will rely on it (QtSql is
probably not the only one)
- as long as modules rely on it, null QString will stay

I can work on the QtSql part, but like I said, that’s a code-breaking
change (acceptable for Qt6 ?).

> I won't do this change, but I will accept a patch from someone who
> does. I will accept it only because it's a performance improvement,
> not because it keeps the nullness.

Thanks, i’ll submit a patch.

Regards,

Julien