From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14351 invoked by alias); 6 May 2016 12:19:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 14334 invoked by uid 89); 6 May 2016 12:19:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.0 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=inter, Yet, dispatch, tui X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 06 May 2016 12:19:22 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7FE72C000755; Fri, 6 May 2016 12:19:21 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u46CJKRZ014430; Fri, 6 May 2016 08:19:20 -0400 Subject: Re: [PATCH v2 05/25] Make the intepreters output to all UIs To: Yao Qi References: <1458573675-15478-1-git-send-email-palves@redhat.com> <1458573675-15478-6-git-send-email-palves@redhat.com> <86vb4evov5.fsf@gmail.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <433fc8ce-4eac-63e7-19db-5953fd845db6@redhat.com> Date: Fri, 06 May 2016 12:19:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <86vb4evov5.fsf@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-05/txt/msg00097.txt.bz2 On 03/22/2016 09:33 AM, Yao Qi wrote: > Pedro Alves writes: > > Hi Pedro, > We have three interpreters, cli, mi and tui. Each of them has a > ${inter}_on_normal_stop registered to normal_stop observer, as below, > > Why don't we have 'struct ui' have a method 'print_stop_event', so that > we can put the different handling there. That's what I tried in an earlier attempt as I tried to describe in the commit log: An earlier prototype of this series did the looping over struct UIs in common code, and then dispatched events to the interpreters through a matching interp_on_foo method for each observer. That turned out a lot more complicated than the present solution, as we'd end up with having to create a new interp method every time some interpreter wanted to listen to some observer notification, resulting in a lot of duplicated make-work and more coupling than desirable. I have an earlier version of the series on my github that goes all the way to make it possible to have multiple independent TUIs active. And then the TUI has even more random observers. It turned out to be a huge mess. So even though this particular observer could seemingly be handled like that, there's a larger pattern here that applies to all observers all interpreters use. I think that if we do switch to the mechanism of calling a "virtual" method directly, it'll be by calling into the method on the UI's top level interpreter directly. A wrinkle is that some interpreters install an observer than works with the current interpreter, while others install an observer that works with the top level interpreter. That should probably be changed to always dispatch through the top level interpreter. I tried that too, but, it was getting unwildly, and decided to back off. Yet another large-ish change for another day. I suspect it'll be easier in C++. But even that isn't clear to me is the best approach. The lose coupling between common code and the interpreters provided by the observers is quite convenient. I think of it as the common code posting an event, and then the interpreters consuming it. I thought of making it possible to associate a "data" pointer with each observer, thus have each UI instance register itself as an observer for a particular event, instead of registering an observer for the whole set of UI instances of a type of UI and then have each observer iterate over all instances. But then that would be painful to do with the current observers design, it'd much much more natural in C++11 with lambdas or some such... > In this way, we can only > register one to normal_stop observer, in interps.c for example, and the > code can be simplified, like this, > > static void > interp_on_normal_stop (struct bpstats *bs, int print_frame) > { > struct switch_thru_all_uis state; > > SWITCH_THRU_ALL_UIS (state) > { > current_ui->print_stop_event (bs, print_frame); > } > } > > we don't have to check the type of interpreter in the loop anymore. > I'd much prefer to go the direction of calling into the top interpreter, instead of adding virtual methods to the UI: SWITCH_THRU_ALL_UIS (state) { current_ui->top_level_interpreter->print_stop_event (bs, print_frame); } Because an UI has-a interpreter, not is-a one. But I've tried this, and ended up with too-ugly template-like macros and too much boilerplace, and lots of duplication in in UI's interface, and I never managed to make it regression free. Thanks, Pedro Alves