From: Markus Armbruster <armbru@redhat.com>
Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> There are no real users of this case, and it's going to be invalid after
> merging of QFloat and QInt use the same QNum type in the following patch.
Invalid because our alternate code is insufficiently sophisticated. In
other words fixable. See "[PATCH 4/4] qapi: Reject alternates that
can't work with keyval_parse()" I just posted.
My patch's commit message describes two related issues, one fixable and
one unfixable. The fixable one already exists, but only in QGA. I
intend to fix it, but it's not a priority.
Fixing the issue you describe seems even less important. I considered
outlawing it in my series (patch appended for your reading pleasure),
but decided to leave it to yours. I don't expect you to replace your
patch. Perhaps my patch helps you with rebasing yours should that
become necessary.
Message-Id: <87tw4cn7sh.fsf@dusky.pond.sub.org>
---
scripts/qapi.py | 4 ++++
tests/test-keyval.c | 4 ++--
tests/test-qobject-input-visitor.c | 26 --------------------------
tests/qapi-schema/qapi-schema-test.json | 2 --
tests/qapi-schema/qapi-schema-test.out | 8 --------
5 files changed, 6 insertions(+), 38 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index b7a25e4759..06e583d8c3 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -826,6 +826,10 @@ def check_alternate(expr, info):
conflicting.add('QTYPE_QINT')
conflicting.add('QTYPE_QFLOAT')
conflicting.add('QTYPE_QBOOL')
+ elif qtype == 'QTYPE_QINT':
+ conflicting.add('QTYPE_QFLOAT')
+ elif qtype == 'QTYPE_QFLOAT':
+ conflicting.add('QTYPE_QINT')
if conflicting & set(types_seen):
raise QAPISemError(info, "Alternate '%s' member '%s' can't "
"be distinguished from member '%s'"
diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index c3be00524c..baf7e339ab 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -615,7 +615,7 @@ static void test_keyval_visit_alternate(void)
Visitor *v;
QDict *qdict;
AltStrObj *aso;
- AltNumInt *ani;
+ AltNumEnum *ane;
AltEnumBool *aeb;
/*
@@ -631,7 +631,7 @@ static void test_keyval_visit_alternate(void)
g_assert_cmpint(aso->type, ==, QTYPE_QSTRING);
g_assert_cmpstr(aso->u.s, ==, "1");
qapi_free_AltStrObj(aso);
- visit_type_AltNumInt(v, "b", &ani, &err);
+ visit_type_AltNumEnum(v, "b", &ane, &err);
error_free_or_abort(&err);
visit_type_AltEnumBool(v, "c", &aeb, &err);
error_free_or_abort(&err);
diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c
index 6b997a177d..83d663d11d 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -592,8 +592,6 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
AltEnumNum *aen;
AltNumEnum *ans;
AltEnumInt *asi;
- AltIntNum *ain;
- AltNumInt *ani;
/* Parsing an int */
@@ -620,18 +618,6 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
g_assert_cmpint(asi->u.i, ==, 42);
qapi_free_AltEnumInt(asi);
- v = visitor_input_test_init(data, "42");
- visit_type_AltIntNum(v, NULL, &ain, &error_abort);
- g_assert_cmpint(ain->type, ==, QTYPE_QINT);
- g_assert_cmpint(ain->u.i, ==, 42);
- qapi_free_AltIntNum(ain);
-
- v = visitor_input_test_init(data, "42");
- visit_type_AltNumInt(v, NULL, &ani, &error_abort);
- g_assert_cmpint(ani->type, ==, QTYPE_QINT);
- g_assert_cmpint(ani->u.i, ==, 42);
- qapi_free_AltNumInt(ani);
-
/* Parsing a double */
v = visitor_input_test_init(data, "42.5");
@@ -655,18 +641,6 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
visit_type_AltEnumInt(v, NULL, &asi, &err);
error_free_or_abort(&err);
qapi_free_AltEnumInt(asi);
-
- v = visitor_input_test_init(data, "42.5");
- visit_type_AltIntNum(v, NULL, &ain, &error_abort);
- g_assert_cmpint(ain->type, ==, QTYPE_QFLOAT);
- g_assert_cmpfloat(ain->u.n, ==, 42.5);
- qapi_free_AltIntNum(ain);
-
- v = visitor_input_test_init(data, "42.5");
- visit_type_AltNumInt(v, NULL, &ani, &error_abort);
- g_assert_cmpint(ani->type, ==, QTYPE_QFLOAT);
- g_assert_cmpfloat(ani->u.n, ==, 42.5);
- qapi_free_AltNumInt(ani);
}
static void test_native_list_integer_helper(TestInputVisitorData *data,
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 17649c6398..91ffb2648c 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -103,8 +103,6 @@
{ 'alternate': 'AltEnumNum', 'data': { 'e': 'EnumOne', 'n': 'number' } }
{ 'alternate': 'AltNumEnum', 'data': { 'n': 'number', 'e': 'EnumOne' } }
{ 'alternate': 'AltEnumInt', 'data': { 'e': 'EnumOne', 'i': 'int' } }
-{ 'alternate': 'AltIntNum', 'data': { 'i': 'int', 'n': 'number' } }
-{ 'alternate': 'AltNumInt', 'data': { 'n': 'number', 'i': 'int' } }
# for testing use of 'str' within alternates
{ 'alternate': 'AltStrObj', 'data': { 's': 'str', 'o': 'TestStruct' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 9f68610dc2..e727a5a84c 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -10,18 +10,10 @@ alternate AltEnumNum
tag type
case e: EnumOne
case n: number
-alternate AltIntNum
- tag type
- case i: int
- case n: number
alternate AltNumEnum
tag type
case n: number
case e: EnumOne
-alternate AltNumInt
- tag type
- case n: number
- case i: int
alternate AltStrObj
tag type
case s: str
--
2.13.0.91.g00982b8dd