From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 21B433836018 for ; Sat, 24 Jul 2021 02:05:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 21B433836018 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 16O24NOl000842 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 23 Jul 2021 22:04:29 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 16O24NOl000842 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 954901E4A3; Fri, 23 Jul 2021 22:04:23 -0400 (EDT) Subject: Re: [PATCH 15/16] gdb: make cmd_list_element var an optional union To: Lancelot SIX Cc: gdb-patches@sourceware.org References: <20210714120851.3pfew5pgcdp6ezn6@ubuntu.lan> <20210714171238.vzccwpurh2izbkps@ubuntu.lan> <20210714232112.wsn7pits6uuz3nf5@ubuntu.lan> <20210720230335.dcpfxbol2uwjre3b@Plymouth> <20210723204613.ud6kkhnt46dz4bq3@ubuntu.lan> <20210723225536.vldk332x4be7czts@ubuntu.lan> From: Simon Marchi Message-ID: <902d2087-7ff6-2f4c-cb00-03230c4ca68f@polymtl.ca> Date: Fri, 23 Jul 2021 22:04:22 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20210723225536.vldk332x4be7czts@ubuntu.lan> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sat, 24 Jul 2021 02:04:23 +0000 X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 24 Jul 2021 02:05:34 -0000 > Yes, the usage seems to be quite close (from the command writer point of > view). The major diferences at the momment is: > - Your setter returns a bool to let know the caller that the setting has > been updated where mine does not. I should change that. Returning a bool is necessary to be able to tell whether we should emit an MI =param-changed notification or not. I think that's the only use. > Other than that, you should be able to fit your set_inferior_tty_command > and get_inferior_tty (among others) in my framework quite easily, in a > very similar way. > > Your add_setshow_*_cmd are changed from something like > > commands.set->function.set_cmd.set_func.string = set_func; > commands.show->function.show_cmd.get_func.string = get_func; > > to something like > > commands.set->var.set_accessor (set_func, get_func); > commands.show->var.set_accessor (set_func, get_func); Great! > (I should probably not set both accessors with one function, only one > will ever be used). Just for the sake of discussion: Conceptually, I find it a little bit wrong that settings are "part" of the set and show commands. Ideally, I think a setting (or parameter, the two terms are used almost interchangeably) should be decoupled from the set/show commands. There are various ways to read setting values, show commands are just one of them. So, every time some part of GDB needs to look up a setting, it actually looks up the corresponding show command. I think that settings should be kept in their own registry, indexed by name. You would have one setting instance for "non-stop", for example, with a getter and a setter function. The "set non-stop" and "show non-stop" cmd_list_element objects would both point to this instance and use the setting's setter or getter function. Other parts of GDB that need to access settings, like the gdb.parameter Python function, would look up the settings registry. For example, gdb.parameter('non-stop') would look up 'non-stop' in that registry, and not look up the 'show non-stop' command as it does today. >> Also, could you provide a git branch with your latest patch? It will be >> easier than figuring out on what to apply it. > > I have pushed my current work (still a work in progress) to the > following branch on sourceware: users/lsix/refactor-typesafe-var. Thanks! Simon