Re: [Qemu-devel] [PATCH 04/21] qobject: add quint type

[Marc-André Lureau]

Hi

On Mon, Mar 13, 2017 at 5:29 PM Marc-André Lureau <address@hidden>
wrote:

Hi

----- Original Message -----
> On 03/13/2017 02:15 AM, Markus Armbruster wrote:
> > Eric Blake <address@hidden> writes:
> >
> >> On 03/11/2017 07:22 AM, Marc-André Lureau wrote:
> >>> The type is not used at all yet. Add some tests to exercice it.
> >>
> >> s/exercice/exercise/
> >>
> >> I wonder if we need this patch at all.
> >>
> >> I've been thinking about a possible alternative representation, such
> >> that a single QInt type can cover _both_ signed and unsigned types, by
> >> having QInt track a sign bit bool in addition to a uint64_t value.
> >>
>
> > You say you've been thinking about extending QInt's range to cover
> > uint64_t.  I've been thinking even more radically: replace both QInt and
> > QFloat by QNumber.  This is how JSON *actually* works.
> >
> > The new QNumber type provides constructors from double, int64_t and
> > uint64_t.  It also provides conversion functions to double, int64_t and
> > uint64_t.  The latter two can fail.
>
> Interesting - I like it, as it takes my idea and goes one step further.
> You'd want to track 64 bits of precision rather than just 53, when the
> input was integral, but the idea seems to have some merit (we have some
> special case in the testsuite for what happens in alternates with
> various combinations of 'number' vs. 'int' that may need tweaking when
> they are no longer distinguishable as QInt vs QFloat, but that's not too
> onerous).
>

I wonder the benefits from hiding the real type behind a QNumber
"superclass", then having to type check at a lower level. QType is not only
used for json, so I see some benefits from having a bit stricter type
declaration and compile-time check. But I don't have a very good idea of
what it would mean to have a generic QNumber type, I could try to implement
it to have a more informed opinion.


I have looked a bit more into implementing a QNumber type.

There is a bit more error handling to add everywhere for getting a value
(for get_int, get_uint), as some conversions will fail. The qdict getters
will  have to throw those errors too. Whereas the QObject type check or
dispatch is there, so less error handling to add.

In my proposal, conversion from int to uint is done for positive values,
with the qdict helper (is it the only way we access json parsed values?).
Do we want to cast a negative int to a uint? Do we have qmp clients using
this representation today, and can we break this?

The qdev-properties will be slightly less convenient to define, but that's
minor since it's behind macros, ex:
-            .qtype     = QTYPE_QUINT,                                   \
-            .defval.u  = (_type)_defval,                                \
+            .qtype     = QTYPE_QNUM,                                    \
+            .defval.type  = QNUM_U64,                                   \
+            .defval.u.u64  = (_type)_defval,                            \

The biggest issue I have is how to handle the alt visitors if all ints are
numbers, ex with AltIntNum:

    switch ((*obj)->type) {
    case QTYPE_QNUM:
        visit_type_int(v, name, &(*obj)->u.i, &err);
        break;
    case QTYPE_QNUM:
        visit_type_number(v, name, &(*obj)->u.n, &err);
        break;

Should the generator have special handling for QNUM and dispatch based on
underlying type?

Overall, it seems to me that using QUINT like I proposed is more
straightforward and equally convenient provided we have conversion helpers
where needed (mostly for qdict).

thanks for your comments
-- 
Marc-André Lureau