Re: [PATCH v3 5/6] qapi: Add support for aliases

[Markus Armbruster]

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