Discussion:
[Development] Behaviour change in QML in Qt 5.8 regarding null
Chris Adams
2016-09-19 01:14:51 UTC
Permalink
Hi,

Recently, a few unit test failures[1] in the (unreleased) QtPIM module
showed that the recent change[2] which changes the semantics of null
assignment (from JS) to a QVariant Q_PROPERTY can affect existing client
code.

Certainly, the cases which are affected are most likely edge-cases (that
is, specifically testing the type or value of the QVariant within C++ code
to detect "null" assignment), however it is probably worth raising for
discussion.

Why was the change made? The commit message tells us what was changed, but
not the reasoning behind the change. The unit tests were changed, so the
behaviour change was clearly intentional (and the old behaviour considered
problematic), and I assume that there must be very good reasons to make
such a change, but it wasn't discussed on the mailing list, so I don't know
what those reasons are.

Best regards,
Chris.

[1] https://codereview.qt-project.org/#/c/170491/
[2] https://codereview.qt-project.org/#/c/167062/


www.qinetic.com.au - Qt And QML User Experience Specialists
Jędrzej Nowacki
2016-09-26 07:02:58 UTC
Permalink
On mandag 19. september 2016 11.14.51 CEST Chris Adams wrote:
> Hi,
>
> Recently, a few unit test failures[1] in the (unreleased) QtPIM module
> showed that the recent change[2] which changes the semantics of null
> assignment (from JS) to a QVariant Q_PROPERTY can affect existing client
> code.
>
> Certainly, the cases which are affected are most likely edge-cases (that
> is, specifically testing the type or value of the QVariant within C++ code
> to detect "null" assignment), however it is probably worth raising for
> discussion.
>
> Why was the change made? The commit message tells us what was changed, but
> not the reasoning behind the change. The unit tests were changed, so the
> behaviour change was clearly intentional (and the old behaviour considered
> problematic), and I assume that there must be very good reasons to make
> such a change, but it wasn't discussed on the mailing list, so I don't know
> what those reasons are.
>
> Best regards,
> Chris.
>
> [1] https://codereview.qt-project.org/#/c/170491/
> [2] https://codereview.qt-project.org/#/c/167062/
>
>
> www.qinetic.com.au - Qt And QML User Experience Specialists

Hi,

There are many reasons, mostly related to C++11 (in more or less chronological
order):
- 5.8 depends on C++11 that has null defined sensibly so we want to use it to
mark null values.
- std::nullptr_t became registered type which is meant to be use for null
values
- QJson support got better null handling (the feature was waiting for
std::nullptr_t in metatype)
- using (void*)0 for null in QVariant was suboptimal as it could not detect
invalid states like for example (void*)1

I guess there are more from QML perspective.

Cheers,
Jędrek
Thiago Macieira
2016-09-26 07:37:07 UTC
Permalink
On segunda-feira, 26 de setembro de 2016 09:02:58 PDT Jędrzej Nowacki wrote:
> On mandag 19. september 2016 11.14.51 CEST Chris Adams wrote:
> > Hi,
> >
> > Recently, a few unit test failures[1] in the (unreleased) QtPIM module
> > showed that the recent change[2] which changes the semantics of null
> > assignment (from JS) to a QVariant Q_PROPERTY can affect existing client
> > code.
> >
> > Certainly, the cases which are affected are most likely edge-cases (that
> > is, specifically testing the type or value of the QVariant within C++ code
> > to detect "null" assignment), however it is probably worth raising for
> > discussion.
> >
> > Why was the change made? The commit message tells us what was changed,
> > but
> > not the reasoning behind the change. The unit tests were changed, so the
> > behaviour change was clearly intentional (and the old behaviour considered
> > problematic), and I assume that there must be very good reasons to make
> > such a change, but it wasn't discussed on the mailing list, so I don't
> > know
> > what those reasons are.
> >
> > Best regards,
> > Chris.
> >
> > [1] https://codereview.qt-project.org/#/c/170491/
> > [2] https://codereview.qt-project.org/#/c/167062/
> >
> >
> > www.qinetic.com.au - Qt And QML User Experience Specialists
>
> Hi,
>
> There are many reasons, mostly related to C++11 (in more or less
> chronological order):
> - 5.8 depends on C++11 that has null defined sensibly so we want to use it
> to mark null values.
> - std::nullptr_t became registered type which is meant to be use for null
> values
> - QJson support got better null handling (the feature was waiting for
> std::nullptr_t in metatype)
> - using (void*)0 for null in QVariant was suboptimal as it could not detect
> invalid states like for example (void*)1

Those are the reasons that enabled the change, not a justification for why it
is better (except the last one, which I don't agree; you can always check the
value).

Repeating Chris's question: is it worth the breakage on the user code?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Simon Hausmann
2016-09-26 09:31:34 UTC
Permalink
Hi,


I'm somewhat torn about the behavioral change. One the one hand it's the "correct" thing to do,

on the other hand it does have the potential of breaking existing code.


That said, this is not quite the same as changing the return type of a typed C++ function. QVariant is designed

to hold any type and if you receive a QVariant it has always been a little dangerous to rely on specific conversion

behavior (throughout Qt).



Simon

________________________________
From: Development <development-bounces+simon.hausmann=***@qt-project.org> on behalf of Thiago Macieira <***@intel.com>
Sent: Monday, September 26, 2016 9:37:07 AM
To: ***@qt-project.org
Subject: Re: [Development] Behaviour change in QML in Qt 5.8 regarding null

On segunda-feira, 26 de setembro de 2016 09:02:58 PDT Jêdrzej Nowacki wrote:
> On mandag 19. september 2016 11.14.51 CEST Chris Adams wrote:
> > Hi,
> >
> > Recently, a few unit test failures[1] in the (unreleased) QtPIM module
> > showed that the recent change[2] which changes the semantics of null
> > assignment (from JS) to a QVariant Q_PROPERTY can affect existing client
> > code.
> >
> > Certainly, the cases which are affected are most likely edge-cases (that
> > is, specifically testing the type or value of the QVariant within C++ code
> > to detect "null" assignment), however it is probably worth raising for
> > discussion.
> >
> > Why was the change made? The commit message tells us what was changed,
> > but
> > not the reasoning behind the change. The unit tests were changed, so the
> > behaviour change was clearly intentional (and the old behaviour considered
> > problematic), and I assume that there must be very good reasons to make
> > such a change, but it wasn't discussed on the mailing list, so I don't
> > know
> > what those reasons are.
> >
> > Best regards,
> > Chris.
> >
> > [1] https://codereview.qt-project.org/#/c/170491/
> > [2] https://codereview.qt-project.org/#/c/167062/
> >
> >
> > www.qinetic.com.au<http://www.qinetic.com.au> - Qt And QML User Experience Specialists
>
> Hi,
>
> There are many reasons, mostly related to C++11 (in more or less
> chronological order):
> - 5.8 depends on C++11 that has null defined sensibly so we want to use it
> to mark null values.
> - std::nullptr_t became registered type which is meant to be use for null
> values
> - QJson support got better null handling (the feature was waiting for
> std::nullptr_t in metatype)
> - using (void*)0 for null in QVariant was suboptimal as it could not detect
> invalid states like for example (void*)1

Those are the reasons that enabled the change, not a justification for why it
is better (except the last one, which I don't agree; you can always check the
value).

Repeating Chris's question: is it worth the breakage on the user code?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Thiago Macieira
2016-09-26 18:14:58 UTC
Permalink
On segunda-feira, 26 de setembro de 2016 09:31:34 PDT Simon Hausmann wrote:
> That said, this is not quite the same as changing the return type of a typed
> C++ function. QVariant is designed
> to hold any type and if you receive a QVariant it has always been a little
> dangerous to rely on specific conversion
> behavior (throughout Qt).

Only if you didn't make the type part of your API, which is what QML had done.

See the failures:
https://codereview.qt-project.org/170491

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Simon Hausmann
2016-09-27 11:25:49 UTC
Permalink
Hi,


That is exactly the part I'm referring to. Receiving a QVariant from the QML engine and relying on it

to contain a specific type.



Simon

________________________________
From: Thiago Macieira <***@intel.com>
Sent: Monday, September 26, 2016 8:14:58 PM
To: ***@qt-project.org
Cc: Simon Hausmann
Subject: Re: [Development] Behaviour change in QML in Qt 5.8 regarding null

On segunda-feira, 26 de setembro de 2016 09:31:34 PDT Simon Hausmann wrote:
> That said, this is not quite the same as changing the return type of a typed
> C++ function. QVariant is designed
> to hold any type and if you receive a QVariant it has always been a little
> dangerous to rely on specific conversion
> behavior (throughout Qt).

Only if you didn't make the type part of your API, which is what QML had done.

See the failures:
https://codereview.qt-project.org/170491

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Thiago Macieira
2016-09-27 15:04:56 UTC
Permalink
On terça-feira, 27 de setembro de 2016 11:25:49 PDT Simon Hausmann wrote:
> That is exactly the part I'm referring to. Receiving a QVariant from the QML
> engine and relying on it to contain a specific type.

Well, I would say it's acceptable that the QVariant contain different numeric
types as the engine changes. Maybe you originally only had double and now you
have double and int. That would be an acceptable behaviour change.

Changing from void* to nullptr is unexpected, but it does fall into the same
bucket. The problem is that you can't write code to adapt to it without
#ifdef, so there's no way to write forward compatibility.

Can we compromise and you delay this change for one release?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Simon Hausmann
2016-09-27 15:29:16 UTC
Permalink
Hi,


I'm personally fine with delaying this by one release. What do others think?


That said, I think it can be written without #ifdef perhaps by using QVariant::isNull() ?


(QVariant(nullptr) maps to isNull() as well, right? ;-)



Simon

________________________________
From: Development <development-bounces+simon.hausmann=***@qt-project.org> on behalf of Thiago Macieira <***@intel.com>
Sent: Tuesday, September 27, 2016 5:04:56 PM
To: ***@qt-project.org
Subject: Re: [Development] Behaviour change in QML in Qt 5.8 regarding null

On terça-feira, 27 de setembro de 2016 11:25:49 PDT Simon Hausmann wrote:
> That is exactly the part I'm referring to. Receiving a QVariant from the QML
> engine and relying on it to contain a specific type.

Well, I would say it's acceptable that the QVariant contain different numeric
types as the engine changes. Maybe you originally only had double and now you
have double and int. That would be an acceptable behaviour change.

Changing from void* to nullptr is unexpected, but it does fall into the same
bucket. The problem is that you can't write code to adapt to it without
#ifdef, so there's no way to write forward compatibility.

Can we compromise and you delay this change for one release?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Thiago Macieira
2016-09-27 16:31:14 UTC
Permalink
On terça-feira, 27 de setembro de 2016 15:29:16 PDT Simon Hausmann wrote:
> That said, I think it can be written without #ifdef perhaps by using
> QVariant::isNull() ?
>
>
> (QVariant(nullptr) maps to isNull() as well, right? ;-)

It should.

But QVariant(QString()) [after you bypass C++'s most vexing parse] also has
isNull() == true. Does the QML engine ever generate those?

Note that user code might.

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Thiago Macieira
2016-09-27 17:26:09 UTC
Permalink
On terça-feira, 27 de setembro de 2016 09:31:14 PDT Thiago Macieira wrote:
> On terça-feira, 27 de setembro de 2016 15:29:16 PDT Simon Hausmann wrote:
> > That said, I think it can be written without #ifdef perhaps by using
> > QVariant::isNull() ?
> >
> >
> > (QVariant(nullptr) maps to isNull() as well, right? ;-)
>
> It should.

Actually, I'm not sure it should.

QVariant's only pointer constructor is the one for const char*, which will be
null if the pointer is null too. That would support the proposition that
QVariant(nullptr).isNull() because QVariant((const char*)nullptr).isNull().

However, to create a VoidStar, you need to write:

QVariant::fromValue<void *>(nullptr);

which does

return QVariant(qMetaTypeId<T>(), &t, QTypeInfo<T>::isPointer);

That constructor always sets d.is_null = false. So the QVariant is not null,
as it contains a valid VoidStar value. It just happens that the VoidStar
itself is null. In other words, QVariant behaves like a non-null void**
pointing to a null void*.

What's more, it's very likely that your QML-created VoidStar also would report
isNull() == false, so you can't write that for current code either to detect a
null JSON value.

Unless you created it with:
QVariant(QMetaType::VoidStar);

is that the case?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Simon Hausmann
2016-09-27 18:22:34 UTC
Permalink
I'm fairly sure we used QVariant(QMetaType::VoidStar);


Simon

From: ***@intel.com
Sent: September 27, 2016 19:26
To: ***@qt-project.org
Subject: Re: [Development] Behaviour change in QML in Qt 5.8 regarding null


On terça-feira, 27 de setembro de 2016 09:31:14 PDT Thiago Macieira wrote:
> On terça-feira, 27 de setembro de 2016 15:29:16 PDT Simon Hausmann wrote:
> > That said, I think it can be written without #ifdef perhaps by using
> > QVariant::isNull() ?
> >
> >
> > (QVariant(nullptr) maps to isNull() as well, right? ;-)
>
> It should.

Actually, I'm not sure it should.

QVariant's only pointer constructor is the one for const char*, which will be
null if the pointer is null too. That would support the proposition that
QVariant(nullptr).isNull() because QVariant((const char*)nullptr).isNull().

However, to create a VoidStar, you need to write:

QVariant::fromValue<void *>(nullptr);

which does

return QVariant(qMetaTypeId<T>(), &t, QTypeInfo<T>::isPointer);

That constructor always sets d.is_null = false. So the QVariant is not null,
as it contains a valid VoidStar value. It just happens that the VoidStar
itself is null. In other words, QVariant behaves like a non-null void**
pointing to a null void*.

What's more, it's very likely that your QML-created VoidStar also would report
isNull() == false, so you can't write that for current code either to detect a
null JSON value.

Unless you created it with:
QVariant(QMetaType::VoidStar);

is that the case?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Thiago Macieira
2016-09-27 21:59:50 UTC
Permalink
On terça-feira, 27 de setembro de 2016 18:22:34 PDT Simon Hausmann wrote:
> I'm fairly sure we used QVariant(QMetaType::VoidStar);

Can you guarantee that the only time the QML engine generates null QVariants
is for null JS Values? That is, no null QStrings, null QVariantLists, null
QVariantMaps/Hahes, null doubles, etc.?

If that's the case, I'd recommend a doc update and unit tests. Chris can fix
his code for isNull().

Note:
QVariant::fromValue(nullptr).isNull() == false
QVariant(QMetaType::Nullptr).isNull() == true

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Allan Sandfeld Jensen
2016-09-27 23:22:33 UTC
Permalink
On Tuesday 27 September 2016, Thiago Macieira wrote:
> On terça-feira, 27 de setembro de 2016 18:22:34 PDT Simon Hausmann wrote:
> > I'm fairly sure we used QVariant(QMetaType::VoidStar);
>
> Can you guarantee that the only time the QML engine generates null
> QVariants is for null JS Values? That is, no null QStrings, null
> QVariantLists, null QVariantMaps/Hahes, null doubles, etc.?
>
> If that's the case, I'd recommend a doc update and unit tests. Chris can
> fix his code for isNull().
>
> Note:
> QVariant::fromValue(nullptr).isNull() == false
> QVariant(QMetaType::Nullptr).isNull() == true

And QVariant(nullptr) doesn't compile.

We should probably fix the fromValue constructor.

`Allan
Thiago Macieira
2016-09-28 05:29:08 UTC
Permalink
On quarta-feira, 28 de setembro de 2016 01:22:33 PDT Allan Sandfeld Jensen
wrote:
> > QVariant::fromValue(nullptr).isNull() == false
> > QVariant(QMetaType::Nullptr).isNull() == true
>
> And QVariant(nullptr) doesn't compile.
>
> We should probably fix the fromValue constructor.

I don't think so. This is also false:

QVariant::fromValue<void *>(nullptr).isNull()

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Allan Sandfeld Jensen
2016-09-28 08:39:42 UTC
Permalink
On Wednesday 28 September 2016, Thiago Macieira wrote:
> On quarta-feira, 28 de setembro de 2016 01:22:33 PDT Allan Sandfeld Jensen
>
> wrote:
> > > QVariant::fromValue(nullptr).isNull() == false
> > > QVariant(QMetaType::Nullptr).isNull() == true
> >
> > And QVariant(nullptr) doesn't compile.
> >
> > We should probably fix the fromValue constructor.
>
> I don't think so. This is also false:
>
> QVariant::fromValue<void *>(nullptr).isNull()

Is QVariant(QMetaType::VoidStar).isNull() true?

Since nullptr is new, I would prefer if we could avoid having it too quirky to
begin with.

`allan
Olivier Goffart
2016-09-28 10:41:16 UTC
Permalink
On Mittwoch, 28. September 2016 10:39:42 CEST Allan Sandfeld Jensen wrote:
> On Wednesday 28 September 2016, Thiago Macieira wrote:
> > On quarta-feira, 28 de setembro de 2016 01:22:33 PDT Allan Sandfeld Jensen
> >
> > wrote:
> > > > QVariant::fromValue(nullptr).isNull() == false
> > > > QVariant(QMetaType::Nullptr).isNull() == true
> > >
> > > And QVariant(nullptr) doesn't compile.
> > >
> > > We should probably fix the fromValue constructor.
> >
> > I don't think so. This is also false:
> > QVariant::fromValue<void *>(nullptr).isNull()
>
> Is QVariant(QMetaType::VoidStar).isNull() true?
>
> Since nullptr is new, I would prefer if we could avoid having it too quirky
> to begin with.
>
> `allan

int main() {
QVariant v1 = QVariant::fromValue<void*>(nullptr);
QVariant v2 = QVariant::fromValue(nullptr);
qDebug() << v1.isNull() << v2.isNull() << (v1 == v2) << (v2 == v1);
qDebug() << v2.canConvert<void*>() << v2.canConvert<QObject*>();
}
// output:
// "false false false false"
// "false false"

IMHO this is wrong!

First of all, I'm not even convinced it makes sens to have std::nullptr_t as a
builtin type.

I think any pointer type with nullptr in it should be isNull()
All we need a specialisation of QVariantIsNull::CallFilteredIsNull for T* and
for std::nullptr_t.
That would be a change of behaviour for T* tough. But not for nullptr_t since
it's new.

Also I think nullptr_t should be convertible to any pointer type. (nullptr_t
is converting top any pointer type in the c++ world)
And that should solve the equality problem.

--
Olivier

Woboq - Qt services and support - https://woboq.com - https://code.woboq.org
Allan Sandfeld Jensen
2016-09-28 12:53:01 UTC
Permalink
On Wednesday 28 September 2016, Olivier Goffart wrote:
>
> int main() {
> QVariant v1 = QVariant::fromValue<void*>(nullptr);
> QVariant v2 = QVariant::fromValue(nullptr);
> qDebug() << v1.isNull() << v2.isNull() << (v1 == v2) << (v2 == v1);
> qDebug() << v2.canConvert<void*>() << v2.canConvert<QObject*>();
> }
> // output:
> // "false false false false"
> // "false false"
>
> IMHO this is wrong!
>
> First of all, I'm not even convinced it makes sens to have std::nullptr_t
> as a builtin type.
>
> I think any pointer type with nullptr in it should be isNull()
> All we need a specialisation of QVariantIsNull::CallFilteredIsNull for T*
> and for std::nullptr_t.
> That would be a change of behaviour for T* tough. But not for nullptr_t
> since it's new.
>
> Also I think nullptr_t should be convertible to any pointer type.
> (nullptr_t is converting top any pointer type in the c++ world)
> And that should solve the equality problem.

I would agree, but after looking into the code, I think we have a major
problem. QVariant::isNull is used for two things, one it maps to isNull on any
complex classes, but during conversion anything null can't be converted and
null afterwards indicate a failed conversion. Convert should probably be using
QVariant::Invalid, but it currently doesn't.

In general we have not separated invalid and null well in the past, and trying
to fix that will cause issues.

`Allan
Simon Hausmann
2016-09-28 08:54:10 UTC
Permalink
Hi,


Ok, let me clarify: When the JavaScript engine wants to map a JS null value to a QVariant, it used to use


QVariant(QMetaType::VoidStar, (void *)0);


and now uses


QVariant::fromValue(nullptr);



If a string is to be converted to a QVariant, it will naturally use QVariant(thatString). I think if that

string happens to be a null QString, then QVariant isNull() will return true, right?


If that is unsufficient for the pim code here (or generally any other code), then my recommendation

would be to change the signature to take a QJSValue instead of a QVariant. The engine supports that

transparently and the QJSValue API allows distinguishing between a JavaScript null and a string, etc.


So that is another option.




Simon

________________________________
From: Development <development-bounces+simon.hausmann=***@qt-project.org> on behalf of Thiago Macieira <***@intel.com>
Sent: Tuesday, September 27, 2016 11:59:50 PM
To: ***@qt-project.org
Subject: Re: [Development] Behaviour change in QML in Qt 5.8 regarding null

On terça-feira, 27 de setembro de 2016 18:22:34 PDT Simon Hausmann wrote:
> I'm fairly sure we used QVariant(QMetaType::VoidStar);

Can you guarantee that the only time the QML engine generates null QVariants
is for null JS Values? That is, no null QStrings, null QVariantLists, null
QVariantMaps/Hahes, null doubles, etc.?

If that's the case, I'd recommend a doc update and unit tests. Chris can fix
his code for isNull().

Note:
QVariant::fromValue(nullptr).isNull() == false
QVariant(QMetaType::Nullptr).isNull() == true

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center

_______________________________________________
Development mailing list
***@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
Development Info Page - Qt<http://lists.qt-project.org/mailman/listinfo/development>
lists.qt-project.org
To see the collection of prior postings to the list, visit the Development Archives. Using Development: To post a message to all the list members ...
Thiago Macieira
2016-09-28 15:37:34 UTC
Permalink
On quarta-feira, 28 de setembro de 2016 08:54:10 PDT Simon Hausmann wrote:
> Hi,
>
> Ok, let me clarify: When the JavaScript engine wants to map a JS null value
> to a QVariant, it used to use
>
> QVariant(QMetaType::VoidStar, (void *)0);
>
> and now uses
>
> QVariant::fromValue(nullptr);

Neither is isNull() == true.

> If a string is to be converted to a QVariant, it will naturally use
> QVariant(thatString). I think if that
> string happens to be a null QString, then QVariant isNull() will return
> true, right?

Right. he question was whether QML strings were guaranteed to be non-null. If
you can't guarantee that, then we can't rely on QVariant::isNull().

> If that is unsufficient for the pim code here (or generally any other code),
> then my recommendation
> would be to change the signature to take a QJSValue instead of a QVariant.
> The engine supports that
> transparently and the QJSValue API allows distinguishing between a
> JavaScript null and a string, etc.

Are we deprecating the QVariant interface?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Simon Hausmann
2016-09-28 15:42:06 UTC
Permalink
Hi,

I don't think the QVariant interface is deprecated, but it just inherently unsuitable for JavaScript specific things such as distinguishing undefined from null or storing function closures. That is why the engine supports both, for different purposes.


Simon

From: ***@intel.com
Sent: September 28, 2016 17:37
To: ***@qt-project.org
Subject: Re: [Development] Behaviour change in QML in Qt 5.8 regarding null


On quarta-feira, 28 de setembro de 2016 08:54:10 PDT Simon Hausmann wrote:
> Hi,
>
> Ok, let me clarify: When the JavaScript engine wants to map a JS null value
> to a QVariant, it used to use
>
> QVariant(QMetaType::VoidStar, (void *)0);
>
> and now uses
>
> QVariant::fromValue(nullptr);

Neither is isNull() == true.

> If a string is to be converted to a QVariant, it will naturally use
> QVariant(thatString). I think if that
> string happens to be a null QString, then QVariant isNull() will return
> true, right?

Right. he question was whether QML strings were guaranteed to be non-null. If
you can't guarantee that, then we can't rely on QVariant::isNull().

> If that is unsufficient for the pim code here (or generally any other code),
> then my recommendation
> would be to change the signature to take a QJSValue instead of a QVariant.
> The engine supports that
> transparently and the QJSValue API allows distinguishing between a
> JavaScript null and a string, etc.

Are we deprecating the QVariant interface?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Thiago Macieira
2016-09-28 15:50:00 UTC
Permalink
On quarta-feira, 28 de setembro de 2016 15:42:06 PDT Simon Hausmann wrote:
> I don't think the QVariant interface is deprecated, but it just inherently
> unsuitable for JavaScript specific things such as distinguishing undefined
> from null or storing function closures. That is why the engine supports
> both, for different purposes.

Chris, is it possible to use the QJSValue interface instead? Why did the code
use QVariant?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Chris Adams
2016-09-29 01:13:29 UTC
Permalink
I'm certain that it's possible. I suspect the reason why the code used
QVariant is that when it was originally written (in Qt 4.7 days, IIRC),
QJSValue didn't exist, and it simply hasn't been ported to newer interfaces
since then. Without knowing too much about the QML bindings in QtPIM, I am
going to assume that there is at least some effort required to port all of
the usages and update all of the unit tests.

On Thu, Sep 29, 2016 at 1:50 AM, Thiago Macieira <***@intel.com>
wrote:

> On quarta-feira, 28 de setembro de 2016 15:42:06 PDT Simon Hausmann wrote:
> > I don't think the QVariant interface is deprecated, but it just
> inherently
> > unsuitable for JavaScript specific things such as distinguishing
> undefined
> > from null or storing function closures. That is why the engine supports
> > both, for different purposes.
>
> Chris, is it possible to use the QJSValue interface instead? Why did the
> code
> use QVariant?
>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
> Software Architect - Intel Open Source Technology Center
>
> _______________________________________________
> Development mailing list
> ***@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
Jocelyn Turcotte
2016-09-29 08:29:52 UTC
Permalink
One of the reasons to use QVariant is that it's usually what is needed to connect a C++ signal to a QML function or to use invokeMethod, I could only ever get this to work by passing all arguments as QVariants.
See for example: http://doc.qt.io/qt-5/qtqml-cppintegration-interactqmlfromcpp.html#invoking-qml-methods

Connecting QML signal to a C++ slot also recommends (requires?) using a QVariant for arguments: http://doc.qt.io/qt-5/qtqml-cppintegration-interactqmlfromcpp.html#connecting-to-qml-signals

Would there be a way that I'm not aware to use QJSValue in those situations too?
Sure it's the other way around, but for the sake of consistency I don't like to use QVariant for QML input functions while using QJSValue for arguments of output functions.

Jocelyn

> On 29 Sep 2016, at 03:13, Chris Adams <***@qinetic.com.au> wrote:
>
> I'm certain that it's possible. I suspect the reason why the code used QVariant is that when it was originally written (in Qt 4.7 days, IIRC), QJSValue didn't exist, and it simply hasn't been ported to newer interfaces since then. Without knowing too much about the QML bindings in QtPIM, I am going to assume that there is at least some effort required to port all of the usages and update all of the unit tests.
>
> On Thu, Sep 29, 2016 at 1:50 AM, Thiago Macieira <***@intel.com> wrote:
> On quarta-feira, 28 de setembro de 2016 15:42:06 PDT Simon Hausmann wrote:
> > I don't think the QVariant interface is deprecated, but it just inherently
> > unsuitable for JavaScript specific things such as distinguishing undefined
> > from null or storing function closures. That is why the engine supports
> > both, for different purposes.
>
> Chris, is it possible to use the QJSValue interface instead? Why did the code
> use QVariant?
>
> --
> Thiago Macieira - thiago.macieira (AT) intel.com
> Software Architect - Intel Open Source Technology Center
>
> _______________________________________________
> Development mailing list
> ***@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
> _______________________________________________
> Development mailing list
> ***@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
Loading...