From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9D4C6C54E67 for ; Thu, 14 Mar 2024 13:33:37 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rklD1-0000Bl-BL; Thu, 14 Mar 2024 09:33:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rklCr-0000BS-Ku for qemu-devel@nongnu.org; Thu, 14 Mar 2024 09:33:13 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rklCo-00085U-Dl for qemu-devel@nongnu.org; Thu, 14 Mar 2024 09:33:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710423187; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Cq6mkbI92Tf7ls263D8mi8YyBge6GZSlUDN7Lq8Y2h4=; b=bxoYsihoHJ5UEFxvuo3TFPZqkXoTgkBIatautLcgd40cY7Mc99IqTH5+whcQq5G18mWKS6 bVinifmVBA/QixhFeYtS1Z8qpzw2GjFH/4zw7rUl9sxxBCdLlU+YSt3ieuAO/pYPRrTpnv v2SmV3CCkjLNFU8UWGs86XPv4U9Qr4Q= Received: from mail-pg1-f199.google.com (mail-pg1-f199.google.com [209.85.215.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-664-0QTpYXoSNpy50isqNSXh_g-1; Thu, 14 Mar 2024 09:33:05 -0400 X-MC-Unique: 0QTpYXoSNpy50isqNSXh_g-1 Received: by mail-pg1-f199.google.com with SMTP id 41be03b00d2f7-5d8dd488e09so869429a12.2 for ; Thu, 14 Mar 2024 06:33:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710423185; x=1711027985; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Cq6mkbI92Tf7ls263D8mi8YyBge6GZSlUDN7Lq8Y2h4=; b=IFZnNgBzXFGrHWc0NOzhL6NQZG2VrOS67RU6tVFfLyxS+aqNNEyNfYtkJAKKXymEK1 C22cAMzoeS9ijlwpBkUDm102XWtVU1kwH+3sS6hjGWr6IDhyjwzht+UIoJ9lzWmxo+1a ldvqW3MaiDR3Mbe7YeTnI6FWAUYPsvhwgfrMVwIydCjCpiXsAdgtD1r78spf5dN6uGz5 1HBvCTeAFyoAeOh4tCqPPexNtBQSx84W0ODk/PO9+BTQ5cvvU3I2TNfA5UPVnqzVt0DD U2pgOGaaAGy2dmIz5wwgpXQXiz6pYT53Cfx2jDc6/WhZG+rt2u8HhOylpcB530/krP53 TWnA== X-Gm-Message-State: AOJu0Yw7ac44tTc2aqiJ/zAsHy2jxRQzW4NxWwlB23IEMTNOo4w+3fzb +1b9LUJTl9PWWOXFf8uEXes/vKD5NssBdNcGmVnvqHRML7xK910n9dUadDsr5EHVDhJU+2g4arC vsu2MQjKV0a8J6TzWNGqheJ4dwdObDPLXKNNO2zGNKowp+TTxGDXEKmrkDtU2+N3xLqkqyI6Prl 7P8aPQKAmVN+lgR5b+mh06Krh50js= X-Received: by 2002:a05:6a20:54a6:b0:1a3:492d:a0da with SMTP id i38-20020a056a2054a600b001a3492da0damr41911pzk.43.1710423184895; Thu, 14 Mar 2024 06:33:04 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFTgNGdmMJ9sJrfK/LkM/YmIasDNqHqqUXLEdHgXwnu7ej+WkTVegXTpgOrIyBXvwd2p+M17NsU0u62SGdQTvA= X-Received: by 2002:a05:6a20:54a6:b0:1a3:492d:a0da with SMTP id i38-20020a056a2054a600b001a3492da0damr41859pzk.43.1710423184234; Thu, 14 Mar 2024 06:33:04 -0700 (PDT) MIME-Version: 1.0 References: <20240313044127.49089-1-jsnow@redhat.com> <20240313044127.49089-6-jsnow@redhat.com> <87o7bh9nyt.fsf@pond.sub.org> In-Reply-To: <87o7bh9nyt.fsf@pond.sub.org> From: John Snow Date: Thu, 14 Mar 2024 09:32:52 -0400 Message-ID: Subject: Re: [PATCH v4 05/23] qapi: create QAPISchemaDefinition To: Markus Armbruster Cc: qemu-devel , Peter Maydell , Michael Roth Content-Type: multipart/alternative; boundary="0000000000005005b406139eee64" Received-SPF: pass client-ip=170.10.133.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -30 X-Spam_score: -3.1 X-Spam_bar: --- X-Spam_report: (-3.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.987, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org --0000000000005005b406139eee64 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Mar 14, 2024, 5:12=E2=80=AFAM Markus Armbruster = wrote: > John Snow writes: > > > Include entities don't have names, but we generally expect "entities" t= o > > have names. Reclassify all entities with names as *definitions*, leavin= g > > the nameless include entities as QAPISchemaEntity instances. > > > > This is primarily to help simplify typing around expectations of what > > callers expect for properties of an "entity". > > > > Suggested-by: Markus Armbruster > > Signed-off-by: John Snow > > --- > > scripts/qapi/schema.py | 144 +++++++++++++++++++++++------------------ > > 1 file changed, 82 insertions(+), 62 deletions(-) > > > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > > index 117f0f78f0c..da273c1649d 100644 > > --- a/scripts/qapi/schema.py > > +++ b/scripts/qapi/schema.py > > [...] > > > @@ -107,6 +90,46 @@ def _set_module(self, schema, info): > > def set_module(self, schema): > > self._set_module(schema, self.info) > > > > + def visit(self, visitor): > > + # pylint: disable=3Dunused-argument > > + assert self._checked > > + > > + > > +class QAPISchemaDefinition(QAPISchemaEntity): > > + meta: Optional[str] =3D None > > + > > + def __init__(self, name: str, info, doc, ifcond=3DNone, > features=3DNone): > > + assert isinstance(name, str) > > + super().__init__(info) > > + for f in features or []: > > + assert isinstance(f, QAPISchemaFeature) > > + f.set_defined_in(name) > > + self.name =3D name > > + self.doc =3D doc > > + self._ifcond =3D ifcond or QAPISchemaIfCond() > > + self.features =3D features or [] > > + > > + def __repr__(self): > > + return "<%s:%s at 0x%x>" % (type(self).__name__, self.name, > > + id(self)) > > + > > + def c_name(self): > > + return c_name(self.name) > > + > > + def check(self, schema): > > + assert not self._checked > > + seen =3D {} > > + for f in self.features: > > + f.check_clash(self.info, seen) > > + super().check(schema) > > v3 called super().check() first. Why did you move it? I'm asking just > to make sure I'm not missing something subtle. > I don't believe you are, I think it was just ... something I didn't notice when rebasing, since I merged this patch by hand. I recall having to insert the super call manually, and I guess my preferences are nondeterministic. > > + > > + def connect_doc(self, doc=3DNone): > > + super().connect_doc(doc) > > + doc =3D doc or self.doc > > + if doc: > > + for f in self.features: > > + doc.connect_feature(f) > > + > > @property > > def ifcond(self): > > assert self._checked > > [...] > > --0000000000005005b406139eee64 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Thu, Mar 14, 2024, 5:12=E2=80=AFAM Markus Armbruste= r <armbru@redhat.com> wrote:=
John Snow <jsnow@redhat.com&g= t; writes:

> Include entities don't have names, but we generally expect "e= ntities" to
> have names. Reclassify all entities with names as *definitions*, leavi= ng
> the nameless include entities as QAPISchemaEntity instances.
>
> This is primarily to help simplify typing around expectations of what<= br> > callers expect for properties of an "entity".
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>=C2=A0 scripts/qapi/schema.py | 144 +++++++++++++++++++++++------------= ------
>=C2=A0 1 file changed, 82 insertions(+), 62 deletions(-)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 117f0f78f0c..da273c1649d 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py

[...]

> @@ -107,6 +90,46 @@ def _set_module(self, schema, info):
>=C2=A0 =C2=A0 =C2=A0 def set_module(self, schema):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._set_module(schema, self.info= )
>=C2=A0
> +=C2=A0 =C2=A0 def visit(self, visitor):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # pylint: disable=3Dunused-argument
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 assert self._checked
> +
> +
> +class QAPISchemaDefinition(QAPISchemaEntity):
> +=C2=A0 =C2=A0 meta: Optional[str] =3D None
> +
> +=C2=A0 =C2=A0 def __init__(self, name: str, info, doc, ifcond=3DNone,= features=3DNone):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 assert isinstance(name, str)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 super().__init__(info)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for f in features or []:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 assert isinstance(f, QAPISc= hemaFeature)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 f.set_defined_in(name)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.name =3D name
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.doc =3D doc
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._ifcond =3D ifcond or QAPISchemaIfCo= nd()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.features =3D features or []
> +
> +=C2=A0 =C2=A0 def __repr__(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "<%s:%s at 0x%x>" = % (type(self).__name__, self.name,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 id(self))
> +
> +=C2=A0 =C2=A0 def c_name(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return c_name(self.name)
> +
> +=C2=A0 =C2=A0 def check(self, schema):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 assert not self._checked
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 seen =3D {}
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 for f in self.features:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 f.check_clash(self.info, seen)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 super().check(schema)

v3 called super().check() first.=C2=A0 Why did you move it?=C2=A0 I'm a= sking just
to make sure I'm not missing something subtle.


--0000000000005005b406139eee64--