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 X-Spam-Level: X-Spam-Status: No, score=-5.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 28AF4C433DB for ; Wed, 20 Jan 2021 16:05:41 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id AEF5A2339E for ; Wed, 20 Jan 2021 16:05:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AEF5A2339E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:39230 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1l2FzH-0006dZ-NN for qemu-devel@archiver.kernel.org; Wed, 20 Jan 2021 11:05:39 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:55218) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l2Fwp-0004Sg-5Y for qemu-devel@nongnu.org; Wed, 20 Jan 2021 11:03:07 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:44108) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1l2Fwm-0005md-RD for qemu-devel@nongnu.org; Wed, 20 Jan 2021 11:03:06 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1611158584; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7iR1FW5/gOGfAjBJNG12avOdSwQeE6siFdz/NyONr1Q=; b=e2qOw9CEWnPwQ9DbX2lStttpbQGoi0PxVXhK4SoeAGvk7yReggQQdunfVuBNXLLSlpi27E C2HCnkqSn7bo9wySZZew74dBRbO7o+yEsCa5J3a6ggoVZcGO5MLHZ0/nMHYimDlrZjHNNT m4Y++kDewEI3HuoakBCPMk3WZduiLHs= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-487-JzRoQpBDNSmYvtWpqETCBQ-1; Wed, 20 Jan 2021 11:03:00 -0500 X-MC-Unique: JzRoQpBDNSmYvtWpqETCBQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id EA5DC107ACE4 for ; Wed, 20 Jan 2021 16:02:59 +0000 (UTC) Received: from [10.3.113.116] (ovpn-113-116.phx2.redhat.com [10.3.113.116]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A0D151724C; Wed, 20 Jan 2021 16:02:56 +0000 (UTC) Subject: Re: [PATCH v3 05/17] qapi: pass QAPISchemaModule to visit_module instead of str To: Markus Armbruster , John Snow References: <20210119180242.1570753-1-jsnow@redhat.com> <20210119180242.1570753-6-jsnow@redhat.com> <87eeifu805.fsf@dusky.pond.sub.org> From: Eric Blake Organization: Red Hat, Inc. Message-ID: <3e43fe6d-b718-af82-598e-e277f9104cbc@redhat.com> Date: Wed, 20 Jan 2021 10:02:55 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <87eeifu805.fsf@dusky.pond.sub.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eblake@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=216.205.24.124; envelope-from=eblake@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.167, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-0.094, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , Cleber Rosa , qemu-devel@nongnu.org, Eduardo Habkost Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 1/20/21 6:07 AM, Markus Armbruster wrote: > John Snow writes: > >> Modify visit_module to pass the module itself instead of just its >> name. This allows for future patches to centralize some >> module-interrogation behavior within the QAPISchemaModule class itself, >> cutting down on duplication between gen.py and schema.py. > > We've been tempted to make similar changes before (don't worry, I'm not > building a case for "no" here). > > When I wrote the initial version of QAPISchemaVisitor (commit 3f7dc21be, > 2015), I aimed for a loose coupling of backends and the internal > representation. Instead of > > def visit_foo(self, foo): > pass > > where @foo is a QAPISchemaFooBar, I wrote > > def visit_foo_bar(self, name, info, [curated attributes of @foo]): > pass > > In theory, this is nice: the information exposed to the backends is > obvious, and the backends can't accidentally mutate @foo. > > In practice, it kind of failed right then and there: > > def visit_object_type(self, name, info, base, members, variants): > pass > > We avoid passing the QAPISchemaObjectType (loose coupling, cool!), only > to pass member information as List[QAPISchemaObjectTypeMember]. > > Morever, passing "curated atttibutes" has led to visit_commands() taking > a dozen arguments. Meh. > > This had made Eric and me wonder whether we should write off the > decoupling idea as misguided, and just pass the object instead of > "curated attributes", always. Thoughts? I'm open to the idea of passing just the larger object instead of the curated list of relevant attributes. It's a bit more coupling, but I don't see any of our qapi code being reused outside its current scope where the extra coupling will bite us. But I'm not volunteering for the refactoring work, because I'm not an expert on python typing hints. If consolidating parameters into the larger object makes for fewer parameters and easier typing hints, I'm assuming the work can be done as part of static typing; but if leaving things as they currently are is manageable, that's also fine by me. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org