[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 5/6] qapi: Add support for aliases
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 5/6] qapi: Add support for aliases |
Date: |
Tue, 14 Sep 2021 13:00:33 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Markus Armbruster <armbru@redhat.com> writes:
> Kevin Wolf <kwolf@redhat.com> writes:
>
>> Am 06.09.2021 um 17:24 hat Markus Armbruster geschrieben:
>>> Kevin Wolf <kwolf@redhat.com> writes:
>>>
>>> > Introduce alias definitions for object types (structs and unions). This
>>> > allows using the same QAPI type and visitor for many syntax variations
>>> > that exist in the external representation, like between QMP and the
>>> > command line. It also provides a new tool for evolving the schema while
>>> > maintaining backwards compatibility during a deprecation period.
>>> >
>>> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
[...]
>> It don't remember the details of why I needed the list(), but
>> a.check_clash() is (a wrapper around) QAPISchemaMember.check_clash(), so
>> yes, it does change @seen. Specifically, it adds alias names to it.
>
> That's why you need it: seen.values() is a dictionary view object, but
> you need something writable.
>
> The silent change from list to view we got with Python 3 is kind of
> iffy: we store the view in self.members (visible right below), which
> keeps @seen alive.
>
> Would you mind reverting this silent change in a separate one-liner
> patch?
Appending one for your convenience.
[...]
>From 4eee60a6a02e425d25167761c47e858e240fe3f8 Mon Sep 17 00:00:00 2001
From: Markus Armbruster <armbru@redhat.com>
Date: Tue, 14 Sep 2021 12:25:09 +0200
Subject: [PATCH] qapi: Revert an accidental change from list to view object
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
A long time ago, commit 23a4b2c6f1 "qapi: Eliminate
QAPISchemaObjectType.check() variable members" replaced the manual
building of the list of members by seen.values(), where @seen is an
OrderedDict mapping names to members. The list is then stored in
self.members.
With Python 2, this is an innocent change: seen.values() returns "a
copy of the dictionary’s list of values".
With Python 3, it returns a dictionary view object instad. These
"provide a dynamic view on the dictionary’s entries, which means that
when the dictionary changes, the view reflects these changes."
Commit 23a4b2c6f1 predates the first mention of Python 3 in
scripts/qapi/ by years. If we had wanted a view object then, we'd
have used seen.viewvalues().
The accidental change of self.members from list to view object keeps
@seen alive longer. Not wanted, but harmless enough. I believe
that's all.
However, the change is in the next commit's way, which wants to mess
with self.members. Revert it.
All other uses of .values() in scripts/qapi/ are of the form
for ... in dict.values():
where the change to view object is just fine. Same for .keys() and
.items().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi/schema.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index d1d27ff7ee..f313dbea27 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -413,7 +413,7 @@ def check(self, schema):
for m in self.local_members:
m.check(schema)
m.check_clash(self.info, seen)
- members = seen.values()
+ members = list(seen.values())
if self.variants:
self.variants.check(schema, seen)
--
2.31.1