From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2348 invoked by alias); 10 Feb 2018 00:17:45 -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 2335 invoked by uid 89); 10 Feb 2018 00:17:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-8.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_1,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=observer, observers X-HELO: gateway24.websitewelcome.com Received: from gateway24.websitewelcome.com (HELO gateway24.websitewelcome.com) (192.185.50.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 10 Feb 2018 00:17:42 +0000 Received: from cm12.websitewelcome.com (cm12.websitewelcome.com [100.42.49.8]) by gateway24.websitewelcome.com (Postfix) with ESMTP id CBD69C727 for ; Fri, 9 Feb 2018 18:17:40 -0600 (CST) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id kIrMeHRp4zzFjkIrMeuXM2; Fri, 09 Feb 2018 18:17:40 -0600 Received: from 174-29-33-254.hlrn.qwest.net ([174.29.33.254]:37048 helo=bapiya) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1ekIrM-002YnK-ID; Fri, 09 Feb 2018 18:17:40 -0600 From: Tom Tromey To: Pedro Alves Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [RFA] Convert observers to C++ References: <20171105013039.9135-1-tom@tromey.com> <20f70b2b-bf93-a6b6-f2a4-82e14c807e05@redhat.com> Date: Sat, 10 Feb 2018 00:17:00 -0000 In-Reply-To: <20f70b2b-bf93-a6b6-f2a4-82e14c807e05@redhat.com> (Pedro Alves's message of "Tue, 7 Nov 2017 15:48:15 +0000") Message-ID: <87zi4haeak.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.90 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-BWhitelist: no X-Source-L: No X-Exim-ID: 1ekIrM-002YnK-ID X-Source-Sender: 174-29-33-254.hlrn.qwest.net (bapiya) [174.29.33.254]:37048 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-SW-Source: 2018-02/txt/msg00158.txt.bz2 >>>>> "Pedro" == Pedro Alves writes: Responding to some old-ish mail... >> This converts observers from using a special source-generating script >> to be plain C++. [...] >> -Note that the @code{normal_stop} notification is not emitted when >> -the execution stops due to a breakpoint, and this breakpoint has >> -a condition that is not met. If the breakpoint has any associated >> -commands list, the commands are executed after the notification >> -is emitted. Pedro> Is all the documentation in this file preserved? I couldn't find Pedro> where this paragraph was moved to, for example. I've now moved this paragraph in particular into a comment. The rest is either preserved or not relevant (IMO). I've added some comments to the public methods of the class. Pedro> And then, you could even do: Pedro> #define DEFINE_OBSERVER(OBSERVER_NAME) \ Pedro> decltype (OBSERVER_NAME) OBSERVER_NAME (#OBSERVER_NAME) Pedro> DEFINE_OBSERVER (normal_stop); I did this. Pedro> It'd be nicer if the self tests were under "namespace selftests", Pedro> IMHO. Done. Pedro> OOC, is there some guideline you were following for preferring Pedro> "namespace gdb_observers" over, say, "namespace gdb::observers" ? Pedro> Just curiosity, don't feel the need to change anything. Nope. I went ahead and moved it to gdb::observers, since it was easy. Pedro> You're going to hate me, but it'd be nice IMO if you Pedro> switched to use /**/ consistently throughout, per GCC/GDB Pedro> convention. I did this. This was actually easy too. >> + m_observers.erase (m_observers.begin () + f); Pedro> Hmm, this looks incorrect. "attach" returns an index Pedro> as token. And then detach uses "std::vector::erase()", Pedro> which removes the element at the index and then shifts Pedro> the rest of the elements left to fill in the space Pedro> left over by the removed element(s). So that invalidates Pedro> all token after the element erased. Yes, thanks for catching this. I figure that observers aren't very space-sensitive, so I just put a counter into the observer and then put the value into the vector, like: std::vector> m_observers; Pedro> This could be constexpr and then defined in-class, right? Did it. Pedro> (I wonder about splitting core/lib from uses of the core. Pedro> I.e., defining the struct observer core mechanism in one Pedro> header, and then define the actual observers in a separate header. Pedro> I'm basically pondering about e.g., using observers under common/ Pedro> or in gdbserver/. Like e.g., "common/observer.h" and then Pedro> "gdb/observers.h" and "gdbserver/observers.h", etc. But we can cross Pedro> that bridge when we get to it, too.) I did this. Also I realized that calling the class "observer" is somewhat wrong, as the callback is the observer -- so I made common/observable.h and I renamed the class to "class observable". I'm running this through the buildbot again and when it is working I will re-submit it. Tom