John Snow <js...@redhat.com> writes: > With the two major JSON-ish type hierarchies clarified for distinct > purposes; QAPIExpression for parsed expressions and JSONValue for
The comment you remove talks about _ExprValue, not QAPIExpression. > introspection data, remove this FIXME as no longer an action item. > > In theory, it may be possible to define a completely agnostic > one-size-fits-all JSON type hierarchy that any other user could borrow - > in practice, it's tough to wrangle the differences between invariant, > covariant and contravariant types: input and output parameters demand > different properties of such a structure. As such, it's simply more > trouble than it's worth. I think there's a weightier counter-argument struggling to get out. As I explained under v2's cover letter, the two types represent things that are only superficially similar. _ExprValue is the obvious stupid abstract syntax tree for the QAPI schema language, with str and bool leaves (QAPI doesn't support floating-point numbers), OrderedDict and list inner nodes. It is used for parser output. QAPIExpression augments _ExprValue, adding a QAPISourceInfo (identifying the expression's source) and a QAPIDoc (the expressions documentation). It is used to represent QAPI top-level expressions. JSONValue is an annotated JSON abstract syntax tree. QAPIExpression and _ExprValue are related to it only insofar as the QAPI schema language is (rather loosely) patterned after JSON. Moreover, the two ASTs serve different purposes. QAPIExpression and _ExprValue represent *input*: they are produced by a parser and consumed by a semantic analyzer. JSONValue represents *output*: it is produced within a backend so we can separate the JSON text formatting aspect. Consolidating these two ASTs makes no sense. Suggest to work this argument into your commit message. > So, declare this "done enough for now". Agree. > Signed-off-by: John Snow <js...@redhat.com> > --- > scripts/qapi/parser.py | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > index c165bd3912c..b5afdd703e7 100644 > --- a/scripts/qapi/parser.py > +++ b/scripts/qapi/parser.py > @@ -42,10 +42,6 @@ > _ExprValue = Union[List[object], Dict[str, object], str, bool] > > > -# FIXME: Consolidate and centralize definitions for _ExprValue and > -# JSONValue; currently scattered across several modules. > - > - > # 3.6 workaround: can be removed when Python 3.7+ is our required version. > if TYPE_CHECKING: > _UserDict = UserDict[str, object]
