Differentiate between "actively in the process of checking" and
"checking has completed". This allows us to clean up the types of some
internal fields such as QAPISchemaObjectType's members field which
currently uses "None" as a canary for determining if check has
completed.
This simplifies the typing from a cumbersome Optional[List[T]] to merely
a List[T], which is more pythonic: it is safe to iterate over an empty
list with "for x in []" whereas with an Optional[List[T]] you have to
rely on the more cumbersome "if L: for x in L: ..."
RFC: are we guaranteed to have members here? can we just use "if
members" without adding the new field?
Signed-off-by: John Snow <js...@redhat.com>
---
scripts/qapi/schema.py | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 164d86c4064..200bc0730d6 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -18,7 +18,7 @@
from collections import OrderedDict
import os
import re
-from typing import List, Optional
+from typing import List, Optional, cast
from .common import (
POINTER_SUFFIX,
@@ -447,22 +447,24 @@ def __init__(self, name, info, doc, ifcond, features,
self.base = None
self.local_members = local_members
self.variants = variants
- self.members = None
+ self.members: List[QAPISchemaObjectTypeMember] = []
+ self._checking = False
def check(self, schema):
# This calls another type T's .check() exactly when the C
# struct emitted by gen_object() contains that T's C struct
# (pointers don't count).
- if self.members is not None:
- # A previous .check() completed: nothing to do
- return
- if self._checked:
+ if self._checking:
# Recursed: C struct contains itself
raise QAPISemError(self.info,
"object %s contains itself" % self.name)
+ if self._checked:
+ # A previous .check() completed: nothing to do
+ return
+ self._checking = True
super().check(schema)
- assert self._checked and self.members is None
+ assert self._checked and not self.members
seen = OrderedDict()
if self._base_name:
@@ -479,13 +481,17 @@ def check(self, schema):
for m in self.local_members:
m.check(schema)
m.check_clash(self.info, seen)
- members = seen.values()
+
+ # check_clash is abstract, but local_members is asserted to be
+ # List[QAPISchemaObjectTypeMember]. Cast to the narrower type.
+ members = cast(List[QAPISchemaObjectTypeMember], list(seen.values()))
if self.variants:
self.variants.check(schema, seen)
self.variants.check_clash(self.info, seen)
- self.members = members # mark completed
+ self.members = members
+ self._checking = False # mark completed
# Check that the members of this type do not cause duplicate JSON members,
# and update seen to track the members seen so far. Report any errors
--
2.41.0
