On Thu, Nov 19, 2020 at 11:39:14AM +0100, Markus Armbruster wrote: > Marc-André Lureau <marcandre.lur...@gmail.com> writes: > > > On Tue, Nov 17, 2020 at 2:48 AM Eduardo Habkost <ehabk...@redhat.com> wrote: > > > >> Use QNumValue to represent QNums, so we can also support uint64_t > >> and double QNum values. Add new QLIT_QNUM_(INT|UINT|DOUBLE) > >> macros for each case. > >> > >> The QLIT_QNUM() macro is being kept for compatibility with > >> existing code, but becomes just a wrapper for QLIT_QNUM_INT(). > >> > > > > I am not sure it's worth to keep. (furthermore, it's only used in tests > > afaics) > > Seconded.
Understood, I will remove it. I thought the QAPI code generator was using it. [...] > >> diff --git a/qobject/qlit.c b/qobject/qlit.c > >> index be8332136c..b23cdc4532 100644 > >> --- a/qobject/qlit.c > >> +++ b/qobject/qlit.c > >> @@ -71,7 +71,8 @@ bool qlit_equal_qobject(const QLitObject *lhs, const > >> QObject *rhs) > >> case QTYPE_QBOOL: > >> return lhs->value.qbool == qbool_get_bool(qobject_to(QBool, rhs)); > >> case QTYPE_QNUM: > >> - return lhs->value.qnum == qnum_get_int(qobject_to(QNum, rhs)); > >> + return qnum_value_is_equal(&lhs->value.qnum, > >> + qnum_get_value(qobject_to(QNum, rhs))); > > Before the patch, we crash when @rhs can't be represented as int64_t. I thought it was expected behavior? QLit never supported QNUM_U64 or QNUM_DOUBLE as input. > > Afterwards, we return false (I think). > > Please note this in the commit message. A separate fix preceding this > patch would be even better, but may not be worth the trouble. Up to > you. The fix would be 3 or 4 extra lines of code that would be immediately deleted. I'll just mention it as a side effect of the new feature. > > >> case QTYPE_QSTRING: > >> return (strcmp(lhs->value.qstr, > >> qstring_get_str(qobject_to(QString, rhs))) == 0); > >> @@ -94,7 +95,7 @@ QObject *qobject_from_qlit(const QLitObject *qlit) > >> case QTYPE_QNULL: > >> return QOBJECT(qnull()); > >> case QTYPE_QNUM: > >> - return QOBJECT(qnum_from_int(qlit->value.qnum)); > >> + return QOBJECT(qnum_from_value(qlit->value.qnum)); > >> case QTYPE_QSTRING: > >> return QOBJECT(qstring_from_str(qlit->value.qstr)); > >> case QTYPE_QDICT: { > >> diff --git a/tests/check-qjson.c b/tests/check-qjson.c > >> index 07a773e653..711030cffd 100644 > >> --- a/tests/check-qjson.c > >> +++ b/tests/check-qjson.c > >> @@ -796,20 +796,23 @@ static void simple_number(void) > >> int i; > >> struct { > >> const char *encoded; > >> + QLitObject qlit; > >> int64_t decoded; > >> int skip; > >> } test_cases[] = { > >> - { "0", 0 }, > >> - { "1234", 1234 }, > >> - { "1", 1 }, > >> - { "-32", -32 }, > >> - { "-0", 0, .skip = 1 }, > >> + { "0", QLIT_QNUM(0), 0, }, > >> + { "1234", QLIT_QNUM(1234), 1234, }, > >> + { "1", QLIT_QNUM(1), 1, }, > >> + { "-32", QLIT_QNUM(-32), -32, }, > >> + { "-0", QLIT_QNUM(0), 0, .skip = 1 }, > > Note .qlit is always QLIT_QNUM(.decoded). Would doing without .qlit > result in a simpler patch? Good point. When I wrote this, I mistakenly thought we would end up having different types of qlits in the array. I still want to test multiple types of QNums here, not just QNUM_I64. I will try to get something that is simple but also gets us more coverage. Maybe as a separate test function and/or a separate patch. > > >> { }, > >> }; > >> > >> for (i = 0; test_cases[i].encoded; i++) { > >> QNum *qnum; > >> int64_t val; > >> + QNum *qlit_num; > >> + int64_t qlit_val; > >> > >> qnum = qobject_to(QNum, > >> qobject_from_json(test_cases[i].encoded, > >> @@ -817,6 +820,7 @@ static void simple_number(void) > >> g_assert(qnum); > >> g_assert(qnum_get_try_int(qnum, &val)); > >> g_assert_cmpint(val, ==, test_cases[i].decoded); > >> + > >> if (test_cases[i].skip == 0) { > >> QString *str; > >> > >> @@ -826,9 +830,66 @@ static void simple_number(void) > >> } > >> > >> qobject_unref(qnum); > >> + > >> + qlit_num = qobject_to(QNum, > >> + qobject_from_qlit(&test_cases[i].qlit)); > >> + g_assert(qlit_num); > >> + g_assert(qnum_get_try_int(qlit_num, &qlit_val)); > >> + g_assert_cmpint(qlit_val, ==, test_cases[i].decoded); > >> + > >> + qobject_unref(qlit_num); > >> } > >> } > >> [...] -- Eduardo