Discussion:
[Development] RFC: unified data model API in QtCore => thin wrapper proposal
Arnaud Clère
2018-06-15 11:31:04 UTC
Permalink
-----Original Message-----
From: Thiago Macieira <***@intel.com>
Sent: jeudi 14 juin 2018 02:08

> This email is to start that discussion and answer these questions:
> a) should we have this API?
> b) if so, what would this API look like?
> c) if not, should we unify at least JSON and CBOR?
> c) either way, should remove QCborValue until we have it?
...
> This API would also be the replacement of the JSON and CBOR parsers, for which
> we'd have a unified API, something like:
> QFoo json = QJson::fromJson(jcontent);
> QFoo cbor = QCbor::fromCbor(ccontent);
> qDebug() << QCbor::toDiagnosticFormat(json); // yes, json

Hi all,

As I was saying during QtCS "QDebug and others" session, structured traces need
a way to serialize in-memory data structures in a number of formats for
interoperability reasons. This requires a generic way to traverse data structures,
not necessarily a generic data structure.

A common data structure for Cbor and Json makes sense since they share so much.
But even these 2 formats have peculiarities that may need distinct APIs like Cbor
tags. This is even more true for Xml. I think that Cbor found a sweet spot between
generality and efficiency, so the data structure behind QCborValue will be good
for many use cases. But like as a generic data structure, it will not suit all use
cases. Look at Python: although it has general purpose list, dict, and so on, its
Json decoder provides hooks to bind data to specialized types.

So, I think it is perfectly Ok to have a common data structure for Cbor and Json
but it does not preclude the need for specific APIs. Also, specific documentation
is usually easier to understand than generic one since you can refer to Json and Cbor
standards and examples. I think it is also Ok to have QCborValue in 5.12 because
we can always add a more generic API as a thin layer on top of specialized data
structures, especially in Qt6 if we take advantage of C++17.

Since the title of the discussion is so general, please let me sketch here what
such API could be. That may help find a definitive answer to your questions.

The basic existing API for reading/writing is streams. One problem with streams
is that the structure of the data being traversed is lost. So, a reader must know
the structure to read the data. And in some cases, ambiguities may even prevent
from reading back the data:

cout << 1.0 << 1;
cin >> myFloat >> myInt; // may well read myFloat==1.1, myInt==0

QDebug avoids most problems by inserting spaces between << by default but does not
allow reading. Also, a user-defined type T must write slightly different code for writing
in QDebug, and other formats, and for reading the resulting text...

The approach we took in the MODMED project originates in functional Zippers which
are generalized iterators for arbitrary data structures, not just sequences. It
makes the data structure apparent in the traversal. Also, the traversal can be adapted
to the task at end. For instance, a user-defined type may ignore some Json data it
does not understand while reading. Thus, the approach, allows to bind "any data
with a common structure" such as a generic QCborValue and a user-defined type or
a QByteArray containing Cbor data or utf8 encoded Json.

Let me dream what this approach could look like in Qt, by first using the approach
to directly write some usual data types in Cbor or Json:

QVector<double> vector = {1.,2.};
QByteArray buffer;
QCborWriter cborw(&buffer);
cborw.sequence().bind("val").bind(true).bind(vector);
// buffer = 0x9F6376616...

Note: A generic encoder would use Cbor indefinite length arrays and few or no Cbor tags
so a specialized encoder would still be needed for some use cases.

buffer.clear();
QJsonWriter jsonw(&buffer);
jsonw.sequence().bind("val").bind(true).bind(vector); // same code as above
// buffer = ["val",true,[1.0,2.0]]

In our approach, "bind" handles Read and Write the same way, so it is possible to do:

QString val; bool b;
QJsonReader jsonr(&buffer);
jsonr.sequence().bind(val).bind(b).bind(vector); // same code with lvalues
// val = "val", b = true, ...

This can work with any in-memory data type like QMap, QVector or QCborValue.
It just requires a bind method or QBind<TResult,T> functor definition. Let me show
you the default templated QBind definition:

template<class TResult, typename T>
struct QBind {
static TResult bind(Val<TResult> value, T t) {
return t.bind(value); // In case of error, define a T::bind(Val<TResult>) method or an external QBind<TResult,T>::bind(Val<TResult>,T) functor
}
};

Most user-defined bind methods would be very simple and the type system would
guarantee that data is well-formed (no sequence or record left open...):

struct Person {
QString m_firstName, m_lastName;
int m_age;

template<TResult>
TResult bind(Val<TResult> value) { return value
.record()
.sequence("name")
.bind(m_firstName)
.bind(m_lastName)
.out()
.bind("age" , m_age); // automagically closes opened record
}
};

One advantage of the approach is that such boiler-plate code would have to be written
once for any TResult (be it a QJsonReader, QCborWriter, etc.), so the above code
would be enough to allow:

QByteArray json;
QJsonWriter(&json) jsonw; jsonw.bind(Person {"John","Doe",42}); // json = {"name":["John","Doe"],"age":42}
Person p;
QJsonReader(&json) jsonr; jsonr.bind(p); // p = Person {"John","Doe",42}
QByteArray cbor;
QCborWriter(&cbor) cborw; cborw.bind(p); // cbor = 0xBF646E616D659F64...

Note: Dynamic data structures' bind methods need to handle Write and Read differently
but user-defined types are rarely dynamically-sized.

The approach even works with QIODevice and no intermediate in-memory data,
so it is possible to do:

QIODevice in, out;
// open appropriately
QJsonReader(&in ) jsonr;
QCborWriter(&out) cborw;
if (cborw.bind(jsonr)) cout << "Done.";
// transforms any Json to Cbor without loading everything in memory

To sum up:
* this approach can use QCborValue which is a nice balance between generality
and efficiency that provides in-place editing
* QBind could provide QCborValue (or any other data type) generic read/write to a
number of formats, but...
* specific writers/readers may always be necessary
* QCborValue may differ from QJsonValue at one time to handle Cbor tags and other peculiarities

To move on, we have a working structured traces library implementing this approach.
However, its write performance is 10 times that of boiler-plate code using QDebug.
Based on our previous work and using modern C++ compilers, it seems possible to
implement the approach with more reasonable write performance. So, I will try to
submit a proof of concept in the following days.

In the meanwhile, I've put a few details on our approach and links to related
discussions on the "QDebug" session wiki page:
https://wiki.qt.io/QDebug_and_other_tracing_facilities

Hope it helps,
Arnaud
Tor Arne Vestbø
2018-06-15 13:46:43 UTC
Permalink
> On 15 Jun 2018, at 13:31, Arnaud Clère <***@minmaxmedical.com> wrote:
>
[snip]

> Most user-defined bind methods would be very simple and the type system would
> guarantee that data is well-formed (no sequence or record left open...):
>
> struct Person {
> QString m_firstName, m_lastName;
> int m_age;
>
> template<TResult>
> TResult bind(Val<TResult> value) { return value
> .record()
> .sequence("name")
> .bind(m_firstName)
> .bind(m_lastName)
> .out()
> .bind("age" , m_age); // automagically closes opened record
> }
> };

Not commenting on this API, but I’d like to note that the API I brought up as “missing” from my use-case was a more “light weight” in-place edit API, where the user does not have to define bindings to data structures like above. Similar to e.g. python:

>>> data = json.loads('{ "foo": [1, 2, 3] }')
>>> data
{u'foo': [1, 2, 3]}
>>> data["foo"][0] = 42
>>> data
{u'foo': [42, 2, 3]}

With our existing APIs I’d expect:

QJsonDocument doc;
doc[“foo”][0] = 42;

Tor Arne
Jason H
2018-06-18 11:33:21 UTC
Permalink
+1 this.

I'd prefer Arne's over the bind below. I do want value updates to be emitted so that we only have to change the data to keep dependent structures updated. I've suggested having some kind of xpath watcher facility...

Having done some web UIs powered by a Ajax, (and qt will undoubtedly power some web backends) you need a facility to keep 2 trees in sync - the server data and the web UI model. However it should be the same for QML or widget UIs... And in my dream world, a proper QML web UI. But no matter how it works, you need to be able to broadcast minimal data changes to the tree to listeners (clients).




> Sent: Friday, June 15, 2018 at 3:46 PM
> From: "Tor Arne Vestbø" <***@qt.io>
> To: "Arnaud Clère" <***@minmaxmedical.com>
> Cc: "Thiago Macieira" <***@intel.com>, "***@qt-project.org" <***@qt-project.org>
> Subject: Re: [Development] RFC: unified data model API in QtCore => thin wrapper proposal
>
>
>
> > On 15 Jun 2018, at 13:31, Arnaud Clère <***@minmaxmedical.com> wrote:
> >
> [snip]
>
> > Most user-defined bind methods would be very simple and the type system would
> > guarantee that data is well-formed (no sequence or record left open...):
> >
> > struct Person {
> > QString m_firstName, m_lastName;
> > int m_age;
> >
> > template<TResult>
> > TResult bind(Val<TResult> value) { return value
> > .record()
> > .sequence("name")
> > .bind(m_firstName)
> > .bind(m_lastName)
> > .out()
> > .bind("age" , m_age); // automagically closes opened record
> > }
> > };
>
> Not commenting on this API, but I’d like to note that the API I brought up as “missing” from my use-case was a more “light weight” in-place edit API, where the user does not have to define bindings to data structures like above. Similar to e.g. python:
>
> >>> data = json.loads('{ "foo": [1, 2, 3] }')
> >>> data
> {u'foo': [1, 2, 3]}
> >>> data["foo"][0] = 42
> >>> data
> {u'foo': [42, 2, 3]}
>
> With our existing APIs I’d expect:
>
> QJsonDocument doc;
> doc[“foo”][0] = 42;
>
> Tor Arne
> _______________________________________________
> Development mailing list
> ***@qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
Dmitry Volosnykh
2018-06-18 11:40:02 UTC
Permalink
FYI, there's QTBUG-29095 and QTBUG-25723 that feel like somewhat related to
the problem under discussion.

On Mon, Jun 18, 2018 at 2:33 PM Jason H <***@gmx.com> wrote:

> +1 this.
>
> I'd prefer Arne's over the bind below. I do want value updates to be
> emitted so that we only have to change the data to keep dependent
> structures updated. I've suggested having some kind of xpath watcher
> facility...
>
> Having done some web UIs powered by a Ajax, (and qt will undoubtedly power
> some web backends) you need a facility to keep 2 trees in sync - the server
> data and the web UI model. However it should be the same for QML or widget
> UIs... And in my dream world, a proper QML web UI. But no matter how it
> works, you need to be able to broadcast minimal data changes to the tree to
> listeners (clients).
>
>
>
>
> > Sent: Friday, June 15, 2018 at 3:46 PM
> > From: "Tor Arne VestbÞ" <***@qt.io>
> > To: "Arnaud ClÚre" <***@minmaxmedical.com>
> > Cc: "Thiago Macieira" <***@intel.com>, "
> ***@qt-project.org" <***@qt-project.org>
> > Subject: Re: [Development] RFC: unified data model API in QtCore => thin
> wrapper proposal
> >
> >
> >
> > > On 15 Jun 2018, at 13:31, Arnaud ClÚre <***@minmaxmedical.com>
> wrote:
> > >
> > [snip]
> >
> > > Most user-defined bind methods would be very simple and the type
> system would
> > > guarantee that data is well-formed (no sequence or record left
> open...):
> > >
> > > struct Person {
> > > QString m_firstName, m_lastName;
> > > int m_age;
> > >
> > > template<TResult>
> > > TResult bind(Val<TResult> value) { return value
> > > .record()
> > > .sequence("name")
> > > .bind(m_firstName)
> > > .bind(m_lastName)
> > > .out()
> > > .bind("age" , m_age); // automagically closes opened
> record
> > > }
> > > };
> >
> > Not commenting on this API, but I’d like to note that the API I brought
> up as “missing” from my use-case was a more “light weight” in-place edit
> API, where the user does not have to define bindings to data structures
> like above. Similar to e.g. python:
> >
> > >>> data = json.loads('{ "foo": [1, 2, 3] }')
> > >>> data
> > {u'foo': [1, 2, 3]}
> > >>> data["foo"][0] = 42
> > >>> data
> > {u'foo': [42, 2, 3]}
> >
> > With our existing APIs I’d expect:
> >
> > QJsonDocument doc;
> > doc[“foo”][0] = 42;
> >
> > Tor Arne
> > _______________________________________________
> > 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
>
Arnaud Clère
2018-06-18 16:35:43 UTC
Permalink
> From: Dmitry Volosnykh <***@gmail.com>
> Sent: lundi 18 juin 2018 13:40
>
> FYI, there's QTBUG-29095 and QTBUG-25723 that feel like somewhat related to the problem under discussion.

I guess QTBUG-25723 is the kind of problem that Thiago solved for QCborValue

For the use case described in QTBUG-29095, I think my QBind proposal could be used instead of QJsonDocument. For instance, it can iterate over a possibly infinite JSON sequence, binding one item at a time to an appropriate in-memory data structure. So you would have to worry about accidental detaches and could use the in-memory data structure to filter or modify each item.

MyItem myItem;
auto i = QJsonReader(&file).begin().sequence().item();
while (i) {
i = i.bind(myItem);
// do whatever you want to myItem
}

> On Mon, Jun 18, 2018 at 2:33 PM Jason H <***@gmx.com> wrote:
> +1 this.
> I'd prefer Arne's over the bind below. I do want value updates to be emitted so that we only have to change the data to keep dependent structures updated. I've suggested having some kind of xpath watcher facility...
> Having done some web UIs powered by a Ajax, (and qt will undoubtedly power some web backends) you need a facility to keep 2 trees in sync - the server data and the web UI model. However it should be the same for QML or widget UIs... And in my dream world, a proper QML web UI. But no matter how it works, you need to be able to broadcast minimal data changes to the tree to listeners (clients).

Not sure I understand you correctly. If you miss a generic data structure with in-place editing, QCborValue or QJsonValue (sharing the same backend) may be a candidate, as are QVariant* or your own classes. QBind is not a data structure but a kind of generic iterator for data structures.

However, since QBind can traverse any data structure in a generic way, it can be used to serialize/deserialize you own specific data structures without having to first convert them to a generic data structure such as QCborValue. Converting to QCborValue would only be required to use specific Cbor features such as tags.

> > Sent: Friday, June 15, 2018 at 3:46 PM
> > From: "Tor Arne VestbÞ" <***@qt.io>
> >
> > Not commenting on this API, but I’d like to note that the API I brought up as “missing” from my use-case was a more “light weight” in-place edit API, where the user does not have to define bindings to data structures like above. Similar to e.g. python:

I guess the same answer as above applies.

BTW, I have committed a QBind POC on our gitlab to explain what I am talking about:
https://gricad-gitlab.univ-grenoble-alpes.fr/modmed/modmedLog/tree/f6b257d806ee50db2c5c86d90757b54ec898391b/tests/QBind

Again, this POC does not provide any in-memory data structure, it just demonstrates
how to traverse data structures in a way that is generic enough to be serialized
in different ways.

The POC is voluntarily "condensed" in a single main.cpp file + an alternate QCborWriter.hpp implementation.

The QJsonWriter now demonstrates same or better performance than hand-written serialization using QDebug, and QCborWriter is 2 to 3 times more efficient than QDebug. As opposed to QDebug, it guarantees well-formedness of the data, so it can always be re-read.
Arnaud Clère
2018-06-20 13:05:35 UTC
Permalink
Hi,
Thiago, did you decide on something regarding QCborValue in Qt5.12?

For structured traces, we have to deal with any user-defined data types so, we can do with QCborValue, QJsonValue or QFoo too. Now, the value of exposing the common backend of QJson and QCbor as a QFoo binary format is not clear to me, and the "lowest common denominator" approach is inherently limited.

In my latest QBind prototype, we can write any user-defined type in various formats, and we can even take a copy of the user-defined type to serialize it later. So, QFoo is not required as an intermediate storage and would probably be slower. The latest benchmark shows the QBind approach can be *very* efficient:

(usecs) |QDebug|Json|Cbor|Writable|Writable>Cbor
MSVC 17
<<atoms__| 8| 6| 4| 0| 3
<<doubles| 15| 19| 4| 0| 5
GCC 5.3
<<atoms__| 4| 3| 2| 0| 4
<<doubles| 29| 28| 4| 0| 4

You can test for yourself, it is 2 source files and a .pro
https://gricad-gitlab.univ-grenoble-alpes.fr/modmed/modmedLog/tree/master/tests/QBind

If you are afraid of template code bloat in the QBind approach, I think a disciplined use can mitigate that, i.e. use QBind to just write to a single format. Actually, I think the compiler has more opportunities to factor out similar code in our QBind approach than with hand-written serialization code. It removes so much boiler plate code with anecdotal differences...

Please let me know what you think.
Arnaud

-----Original Message-----
From: Thiago Macieira <***@intel.com>
Sent: jeudi 14 juin 2018 02:08

> This email is to start that discussion and answer these questions:
> a) should we have this API?
> b) if so, what would this API look like?
> c) if not, should we unify at least JSON and CBOR?
> c) either way, should remove QCborValue until we have it?
...
> This API would also be the replacement of the JSON and CBOR parsers,
> for which we'd have a unified API, something like:
> QFoo json = QJson::fromJson(jcontent);
> QFoo cbor = QCbor::fromCbor(ccontent);
> qDebug() << QCbor::toDiagnosticFormat(json); // yes, json
Thiago Macieira
2018-08-16 20:11:05 UTC
Permalink
On Wednesday, 20 June 2018 06:05:35 PDT Arnaud Clère wrote:
> Hi,
> Thiago, did you decide on something regarding QCborValue in Qt5.12?

Hello Arnaud, all

No, I have not. I have not had the time to read your email yet and nor have I
had time to even test if the API I designed does what we discussed in Oslo.

I will not have time to do any of that before the 5.12 feature freeze.

So we need a summary decision:
a) yank it out
b) leave it as is and damn the torpedoes

Unless someone can volunteer to test. I *think* my design is slightly better
than QJsonValue, so the following should work:

value[1]["hello"][32] = false;

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Tor Arne Vestbø
2018-08-17 09:50:32 UTC
Permalink
> On 16 Aug 2018, at 22:11, Thiago Macieira <***@intel.com> wrote:
>
> On Wednesday, 20 June 2018 06:05:35 PDT Arnaud Clère wrote:
>> Hi,
>> Thiago, did you decide on something regarding QCborValue in Qt5.12?
>
> Hello Arnaud, all
>
> No, I have not. I have not had the time to read your email yet and nor have I
> had time to even test if the API I designed does what we discussed in Oslo.
>
> I will not have time to do any of that before the 5.12 feature freeze.
>
> So we need a summary decision:
> a) yank it out
> b) leave it as is and damn the torpedoes
>
> Unless someone can volunteer to test. I *think* my design is slightly better
> than QJsonValue, so the following should work:
>
> value[1]["hello"][32] = false;

That’s great news. I assume it’s also easy to convert from and to JSON, so that one could use QCbor as an intermediate data format if one needs to modify JSON In place and write it out again?

Tor Arne
Thiago Macieira
2018-08-17 15:08:52 UTC
Permalink
On Friday, 17 August 2018 02:50:32 PDT Tor Arne Vestbø wrote:
> > Unless someone can volunteer to test. I *think* my design is slightly
> > better than QJsonValue, so the following should work:
> >
> > value[1]["hello"][32] = false;
>
> That’s great news. I assume it’s also easy to convert from and to JSON, so
> that one could use QCbor as an intermediate data format if one needs to
> modify JSON In place and write it out again?

Please note I said that I *think* it should work, not that it does work. Could
you give it a try and see if it makes your life easier?

Now, looking at the code, I don't think it does work. I thought that
QCborValue::operator[] returned QCborValueRefs, but it doesn't. Adding a set
of non-const overloads returning QCborValueRef might be the trick. And it's a
trick we can add to QJsonValue too.

Converting from JSON is lossless and converting that content back to JSON is
lossless too. It's just not particularly efficient today.

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Tor Arne Vestbø
2018-08-17 15:13:21 UTC
Permalink
> On 17 Aug 2018, at 17:08, Thiago Macieira <***@intel.com> wrote:
>
> On Friday, 17 August 2018 02:50:32 PDT Tor Arne Vestbø wrote:
>>> Unless someone can volunteer to test. I *think* my design is slightly
>>> better than QJsonValue, so the following should work:
>>>
>>> value[1]["hello"][32] = false;
>>
>> That’s great news. I assume it’s also easy to convert from and to JSON, so
>> that one could use QCbor as an intermediate data format if one needs to
>> modify JSON In place and write it out again?
>
> Please note I said that I *think* it should work, not that it does work. Could
> you give it a try and see if it makes your life easier?
>
> Now, looking at the code, I don't think it does work. I thought that
> QCborValue::operator[] returned QCborValueRefs, but it doesn't. Adding a set
> of non-const overloads returning QCborValueRef might be the trick.

That would be great to have as part of the API yes

> And it's a trick we can add to QJsonValue too.

That would be great, but I think it was kind of tricky with the current design of QJsonValue.

Tor Arne
Thiago Macieira
2018-08-17 17:48:54 UTC
Permalink
On Friday, 17 August 2018 08:13:21 PDT Tor Arne Vestbø wrote:
> > Now, looking at the code, I don't think it does work. I thought that
> > QCborValue::operator[] returned QCborValueRefs, but it doesn't. Adding a
> > set of non-const overloads returning QCborValueRef might be the trick.
> That would be great to have as part of the API yes

I won't have time to experiment with it before feature freeze. Technically,
the API freeze doesn't happen until Beta, so we have a few more weeks, but I
wouldn't hold my hopes up that I will have time to trial this.

Any chance you can give it a go? Or someone else?

> > And it's a trick we can add to QJsonValue too.
>
> That would be great, but I think it was kind of tricky with the current
> design of QJsonValue.

Fair enough. My long-term goal is to replace the QJsonDocument internals with
QCborContainerPrivate. That way, the conversion from JSON to CBOR is O(1) and
the conversion backwards is just a validation.

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Sze Howe Koh
2018-09-09 10:25:54 UTC
Permalink
Hi Thiago,

On Sat, 18 Aug 2018 at 01:51, Thiago Macieira <***@intel.com> wrote:
>
> On Friday, 17 August 2018 08:13:21 PDT Tor Arne Vestbø wrote:
> > > Now, looking at the code, I don't think it does work. I thought that
> > > QCborValue::operator[] returned QCborValueRefs, but it doesn't. Adding a
> > > set of non-const overloads returning QCborValueRef might be the trick.
> > That would be great to have as part of the API yes
>
> I won't have time to experiment with it before feature freeze. Technically,
> the API freeze doesn't happen until Beta, so we have a few more weeks, but I
> wouldn't hold my hopes up that I will have time to trial this.
>
> Any chance you can give it a go? Or someone else?

I've installed the Qt 5.12 alpha for MSVC 2015 and started playing
with the CBOR API.

map["hello"] = foo; // ERROR C2593: Operator '[' is ambiguous
map[QString("hello")] = foo; // OK

The ambiguity disappears if I remove either operator[](const QString&)
OR operator[](const QCborValue&). Oddly, operator[](QLatin1String)
doesn't matter. Why's that?

Also, chained operator[] currently doesn't work because
QCborValueRef::operator[](...) doesn't exist:

qDebug() << array[1]; // OK
qDebug() << array[1][2]; // ERROR C2676: 'QCborValueRef' does not
define this operator or a conversion to a type acceptable to the
predefined operator


Regards,
Sze-Howe
Tor Arne Vestbø
2018-09-09 11:16:24 UTC
Permalink
> On 9 Sep 2018, at 12:25, Sze Howe Koh <***@gmail.com> wrote:
>
> Hi Thiago,
>
> On Sat, 18 Aug 2018 at 01:51, Thiago Macieira <***@intel.com> wrote:
>>
>> On Friday, 17 August 2018 08:13:21 PDT Tor Arne Vestbø wrote:
>>>> Now, looking at the code, I don't think it does work. I thought that
>>>> QCborValue::operator[] returned QCborValueRefs, but it doesn't. Adding a
>>>> set of non-const overloads returning QCborValueRef might be the trick.
>>> That would be great to have as part of the API yes
>>
>> I won't have time to experiment with it before feature freeze. Technically,
>> the API freeze doesn't happen until Beta, so we have a few more weeks, but I
>> wouldn't hold my hopes up that I will have time to trial this.
>>
>> Any chance you can give it a go? Or someone else?
>
> I've installed the Qt 5.12 alpha for MSVC 2015 and started playing
> with the CBOR API.
>
> map["hello"] = foo; // ERROR C2593: Operator '[' is ambiguous
> map[QString("hello")] = foo; // OK
>
> The ambiguity disappears if I remove either operator[](const QString&)
> OR operator[](const QCborValue&). Oddly, operator[](QLatin1String)
> doesn't matter. Why's that?
>
> Also, chained operator[] currently doesn't work because
> QCborValueRef::operator[](...) doesn't exist:
>
> qDebug() << array[1]; // OK
> qDebug() << array[1][2]; // ERROR C2676: 'QCborValueRef' does not
> define this operator or a conversion to a type acceptable to the
> predefined operator

This is exactly the kind of stuff I brought up at the contributors summit. We should strive to at least be on par with QJson, but I’m hoping we can also have a nice API for writing (something QJson doesn’t easily facilitate). Any chance the read-only stuff can be added at least Thiago?

Tor Arne
Thiago Macieira
2018-09-09 16:36:28 UTC
Permalink
On Sunday, 9 September 2018 04:16:24 PDT Tor Arne Vestbø wrote:
> This is exactly the kind of stuff I brought up at the contributors summit.
> We should strive to at least be on par with QJson, but I’m hoping we can
> also have a nice API for writing (something QJson doesn’t easily
> facilitate). Any chance the read-only stuff can be added at least Thiago?

It can be, but we're already past feature freeze and I don't have time to do
it right now. I'm completely swamped at work and I'm dedicating any free
minutes I have for Qt to do code reviews for others.

The issue with map["hello"] can be an API review issue, though.

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Edward Welbourne
2018-10-05 11:25:05 UTC
Permalink
On Sunday, 9 September 2018 04:16:24 PDT Tor Arne Vestbø wrote:
>> This is exactly the kind of stuff I brought up at the contributors
>> summit. We should strive to at least be on par with QJson, but I’m
>> hoping we can also have a nice API for writing (something QJson
>> doesn’t easily facilitate). Any chance the read-only stuff can be
>> added at least Thiago?

Thiago Macieira (9 September 2018 18:36) wrote:
> It can be, but we're already past feature freeze and I don't have time
> to do it right now. I'm completely swamped at work and I'm dedicating
> any free minutes I have for Qt to do code reviews for others.
>
> The issue with map["hello"] can be an API review issue, though.

For the sake of openness (we took most of this discussion off-line), the
end result of that was:

5.12:
QCbor{Map,Array}::operator[] grow over-loads for string literals and all
read-only variants return const QCborValue:
https://codereview.qt-project.org/240046

QCborArray::operator[] and insert() allow indices past the end of the
array, padding the array with invalid entries:
https://codereview.qt-project.org/240537

dev (5.13):
QCborValue{Ref,}::operator[] added where missing
https://codereview.qt-project.org/240042

The first's const-ing and the second's padding are needed in 5.12 to let
the last be backwards-compatible, when it lands. Once the first two land,
we can finally close the qtbase API review.

Eddy.
Thiago Macieira
2018-10-05 15:35:10 UTC
Permalink
On Friday, 5 October 2018 04:25:05 PDT Edward Welbourne wrote:
> dev (5.13):
> QCborValue{Ref,}::operator[] added where missing
> https://codereview.qt-project.org/240042
>
> The first's const-ing and the second's padding are needed in 5.12 to let
> the last be backwards-compatible, when it lands. Once the first two land,
> we can finally close the qtbase API review.

May as well bring this question to the list as a whole now:

QCborValue, Array and Map have methods like:

const QCborValue operator[](qint64 index) const;

We're trying to figure out if the returned type should be const.

Pros:
1) once QCborValue can be modified by the APi above, you can't accidentally
use it in a const object
2) you can't write
array[n] = newValue;

Cons:
Suppresses move construction as in
QCborValue v = array[n];
this still compiles, but passes through the copy constructor, not the move
one. We cana add an extra move constructor for const QCborValue && if
necessary.

Eddy: what happens in the new API if you write:
const QCborArray array = { QCborArray{ 1 } };
QCborValue v = array[0];
v[0] = 2;

Does that modify array?


--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Edward Welbourne
2018-10-05 16:26:45 UTC
Permalink
Thiago Macieira (5 October 2018 17:35) asked me:
> Eddy: what happens in the new API if you write:
> const QCborArray array = { QCborArray{ 1 } };
> QCborValue v = array[0];
> v[0] = 2;
>
> Does that modify array?

No. It should only modify what v[0] reports, or a v.toArray()[0]
reports, after the assignment. My addition to the end of
tst_QCborValue::arrayDefaultInitialization() verifies exactly this with
the QVERIFY(a2.isEmpty()); that it adds, after such an assignment.

Eddy.
Thiago Macieira
2018-10-05 22:47:42 UTC
Permalink
On Friday, 5 October 2018 08:35:10 PDT Thiago Macieira wrote:
> Cons:
> Suppresses move construction as in
> QCborValue v = array[n];
> this still compiles, but passes through the copy constructor, not the move
> one. We cana add an extra move constructor for const QCborValue && if
> necessary.

Never mind, you can't move from the const rvalue reference. It needs to be
modifiable.

So, again: should we have the const in the return types at the expense of
losing the move semantic?

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Lars Knoll
2018-10-07 08:53:04 UTC
Permalink
> On 6 Oct 2018, at 00:47, Thiago Macieira <***@intel.com> wrote:
>
> On Friday, 5 October 2018 08:35:10 PDT Thiago Macieira wrote:
>> Cons:
>> Suppresses move construction as in
>> QCborValue v = array[n];
>> this still compiles, but passes through the copy constructor, not the move
>> one. We cana add an extra move constructor for const QCborValue && if
>> necessary.
>
> Never mind, you can't move from the const rvalue reference. It needs to be
> modifiable.
>
> So, again: should we have the const in the return types at the expense of
> losing the move semantic?

I’d go for returning a const object. It’s a shame to loose the move semantics, but the other case would lead to easy programming errors.

Cheers,
Lars
Edward Welbourne
2018-10-08 08:14:41 UTC
Permalink
> On Friday, 5 October 2018 08:35:10 PDT Thiago Macieira wrote:
>>> Cons:
>>> Suppresses move construction as in
>>> QCborValue v = array[n];
>>> this still compiles, but passes through the copy constructor, not
>>> the move one. We cana add an extra move constructor for const
>>> QCborValue && if necessary.

On 6 Oct 2018, at 00:47, Thiago Macieira <***@intel.com> wrote:
>> Never mind, you can't move from the const rvalue reference. It needs
>> to be modifiable.
>>
>> So, again: should we have the const in the return types at the
>> expense of losing the move semantic?

Lars Knoll (7 October 2018 10:53)
> I’d go for returning a const object. It’s a shame to loose the move
> semantics, but the other case would lead to easy programming errors.

The change currently staged for integration [0] only adds const to the
QCborValue return types of the read-only operator[] on QCbor{Array,Map};
as noted in the review comments there, there are several other methods
returning QCborValue, e.g. at(index) and value(key), that potentially
need to also return it as const. My rationale for limiting this to
operator[] was that a['b']['c']['d'] = 12 might seem like a reasonable
thing to do, while a.at('b')['c']['d'] = 12 seems more obviously like a
senseless thing to attempt.

[0] https://codereview.qt-project.org/240046

So the question arises: which other QCbor{Map,Array} methods should
const-qualify their QCborValue returns ?

Eddy.
Thiago Macieira
2018-09-09 16:34:51 UTC
Permalink
On Sunday, 9 September 2018 03:25:54 PDT Sze Howe Koh wrote:
> Hi Thiago,
> > Any chance you can give it a go? Or someone else?
>
> I've installed the Qt 5.12 alpha for MSVC 2015 and started playing
> with the CBOR API.
>
> map["hello"] = foo; // ERROR C2593: Operator '[' is ambiguous

That's what you get for trying to make a modern API that uses QLatin1String
and QStringView... But then I changed to QString.

> map[QString("hello")] = foo; // OK
>
> The ambiguity disappears if I remove either operator[](const QString&)
> OR operator[](const QCborValue&). Oddly, operator[](QLatin1String)
> doesn't matter. Why's that?

Needs investigation. I had QStringView originally, but had to change for some
reason.

> Also, chained operator[] currently doesn't work because
> QCborValueRef::operator[](...) doesn't exist:
>
> qDebug() << array[1]; // OK
> qDebug() << array[1][2]; // ERROR C2676: 'QCborValueRef' does not
> define this operator or a conversion to a type acceptable to the
> predefined operator

That's what I said in the previous email. When I asked for someone to "give it
a go", I meant adding the necessary API to make it work.

--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
Tor Arne Vestbø
2018-08-22 14:46:39 UTC
Permalink
On 17 Aug 2018, at 17:08, Thiago Macieira <***@intel.com> wrote:
>
> On Friday, 17 August 2018 02:50:32 PDT Tor Arne Vestbø wrote:
>>> Unless someone can volunteer to test. I *think* my design is slightly
>>> better than QJsonValue, so the following should work:
>>>
>>> value[1]["hello"][32] = false;
>>
>> That’s great news. I assume it’s also easy to convert from and to JSON, so
>> that one could use QCbor as an intermediate data format if one needs to
>> modify JSON In place and write it out again?
>
> Please note I said that I *think* it should work, not that it does work. Could
> you give it a try and see if it makes your life easier?
>
> Now, looking at the code, I don't think it does work. I thought that
> QCborValue::operator[] returned QCborValueRefs, but it doesn't. Adding a set
> of non-const overloads returning QCborValueRef might be the trick. And it's a
> trick we can add to QJsonValue too.
>
> Converting from JSON is lossless and converting that content back to JSON is
> lossless too. It's just not particularly efficient today.

This library looks really nice:

https://cocoapods.org/pods/nlohmann_json

Tor Arne

>
> --
> 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
Mikhail Svetkin
2018-08-22 14:53:46 UTC
Permalink
Yes, this library is very cool and requires only c++11.

Best regards,
Mikhail


On Wed, 22 Aug 2018 at 16:47, Tor Arne VestbÞ <***@qt.io> wrote:

> On 17 Aug 2018, at 17:08, Thiago Macieira <***@intel.com>
> wrote:
> >
> > On Friday, 17 August 2018 02:50:32 PDT Tor Arne VestbÞ wrote:
> >>> Unless someone can volunteer to test. I *think* my design is slightly
> >>> better than QJsonValue, so the following should work:
> >>>
> >>> value[1]["hello"][32] = false;
> >>
> >> That’s great news. I assume it’s also easy to convert from and to JSON,
> so
> >> that one could use QCbor as an intermediate data format if one needs to
> >> modify JSON In place and write it out again?
> >
> > Please note I said that I *think* it should work, not that it does work.
> Could
> > you give it a try and see if it makes your life easier?
> >
> > Now, looking at the code, I don't think it does work. I thought that
> > QCborValue::operator[] returned QCborValueRefs, but it doesn't. Adding a
> set
> > of non-const overloads returning QCborValueRef might be the trick. And
> it's a
> > trick we can add to QJsonValue too.
> >
> > Converting from JSON is lossless and converting that content back to
> JSON is
> > lossless too. It's just not particularly efficient today.
>
> This library looks really nice:
>
> https://cocoapods.org/pods/nlohmann_json
>
> Tor Arne
>
> >
> > --
> > 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
>
Arnaud Clère
2018-08-20 08:42:51 UTC
Permalink
Hi,

Unfortunately I do not think I can help with the QCborValue development and Json/Cbor refactoring.
Just, from my experience, Json and Cbor share so much that the refactoring makes a lot of sense and having an efficient, editable, in-memory representation of the underlying data is a nice goal for many use cases (was not it the idea of a "JsonDb"?).

Now, regarding the "structured traces" use case, converting the user-defined data types (and Qt types) to such representation at the tracepoint would only make sense performance-wise if the in-memory representation:
- require one or very few heap allocations per tracepoint
- can be copied to the log as a single chunk
Does the new design would allow that?

Even in this case, it would probably be better performance-wise to use the kind of thin-wrapper "QBind" API that I am pushing since we managed to implement it without heap allocation and with only 20% CPU overhead compared to hand-written serialisation (be it QDebug or QDataStream, etc.).

Finally, as a user, I can cope with reasonable API changes but new features suck if they do not work, so I would probably stay away from the "torpedoes" unless I am sure they are targeting the right thing 😊

Cheers,
Arnaud

-----Original Message-----
From: Thiago Macieira <***@intel.com>
Sent: jeudi 16 août 2018 22:11
To: ***@qt-project.org
Subject: Re: [Development] unified data model API in QtCore => thin wrapper proposal

On Wednesday, 20 June 2018 06:05:35 PDT Arnaud Clère wrote:
> Hi,
> Thiago, did you decide on something regarding QCborValue in Qt5.12?

Hello Arnaud, all

No, I have not. I have not had the time to read your email yet and nor have I had time to even test if the API I designed does what we discussed in Oslo.

I will not have time to do any of that before the 5.12 feature freeze.

So we need a summary decision:
a) yank it out
b) leave it as is and damn the torpedoes

Unless someone can volunteer to test. I *think* my design is slightly better than QJsonValue, so the following should work:

value[1]["hello"][32] = false;

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