From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120608 invoked by alias); 16 Jan 2019 11:19:46 -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 120594 invoked by uid 89); 16 Jan 2019 11:19:45 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=canceled, Hx-spam-relays-external:209.85.221.68, H*RU:209.85.221.68, waste X-HELO: mail-wr1-f68.google.com Received: from mail-wr1-f68.google.com (HELO mail-wr1-f68.google.com) (209.85.221.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 16 Jan 2019 11:19:43 +0000 Received: by mail-wr1-f68.google.com with SMTP id z5so6421153wrt.11 for ; Wed, 16 Jan 2019 03:19:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=4p4MDpWCqI9juFNIh7sS3viS10UHYspBUBnlnmK8ESI=; b=ZjOXv/H6ds1A9a3In9o7jO43i+xn2hdjx9kac97+BAp3OKVP+dqYEyto8RjZZmQQ8K OYkovGT6dMxCJQLTySU1j5eqWBchaBqyxIF/YqfZONCCTFoVSMIiIBILmrtjREIbWCxz Bt4fqiU2fKg+SFh7VvCzHNq1C6Rec8h/7qKlrLD84ZKQgQPFoZJ8MlZpOFYTQOzWByA8 VVgmoPpA1jc7YMc9hn/SWCkmYcgyXZdkdWarcUxWLmH3Gh+ymDxX+HibS45J06EjHWeF haUB2NuVVUgrsx7Y81X27XN2iUrkr84F0TrVDl6XOWVx8dVCKflYSrFMADge0ImlLD/H 4tNw== Return-Path: Received: from localhost (host86-148-235-91.range86-148.btcentralplus.com. [86.148.235.91]) by smtp.gmail.com with ESMTPSA id b7sm71720235wrs.47.2019.01.16.03.19.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 16 Jan 2019 03:19:40 -0800 (PST) Date: Wed, 16 Jan 2019 11:19:00 -0000 From: Andrew Burgess To: Pedro Alves Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH 00/12] remove some cleanups using a cleanup function Message-ID: <20190116111939.GT3456@embecosm.com> References: <20190109033426.16062-1-tom@tromey.com> <961d9501-23e6-9adb-a11b-da41702c4fa0@redhat.com> <20190115094157.GP3456@embecosm.com> <87ef9dttfl.fsf@tromey.com> <1014fa7f-bbce-cbea-f54f-480373299809@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1014fa7f-bbce-cbea-f54f-480373299809@redhat.com> X-Fortune: Who needs friends when you can sit alone in your room and drink? X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-01/txt/msg00370.txt.bz2 * Pedro Alves [2019-01-15 23:43:07 +0000]: > On 01/15/2019 11:03 PM, Tom Tromey wrote: > >>>>>> "Andrew" == Andrew Burgess writes: > > > > Andrew> Maybe there's some other reason why scoped_finish_thread_state is > > Andrew> different, in which case I apologise for wasting everyone's time, but > > Andrew> right now it appears to me that scoped_finish_thread_state is no > > Andrew> different to cleanup_function, it's just used more. > > > > FWIW I don't think it's a waste of time at all. There's no particular > > rush for these patches and I think it's more valuable for us to agree on > > what we'd like the result to look like than it is to land them quickly. > > Definitely agreed. Not a waste at all! > > I've been playing with this today, and I have a different > implementation of Andrew's class that allows writing: > > using delete_longjmp_breakpoint_cleanup > = forward_cleanup_function delete_longjmp_breakpoint>; > > Or, with a macro to eliminate redundancy: > > using delete_longjmp_breakpoint_cleanup > = FORWARD_CLEANUP_FUNCTION (delete_longjmp_breakpoint); > > Naming up in the air, I just picked that as straw man. > > > > > Andrew> I think if we're going to put in a generic solution (which I think is > > Andrew> a good thing) then we should either, make sure we understand why > > Andrew> scoped_finish_thread_state is different (and what the rules are for > > Andrew> when to use the generic, and when to create a class), or, make sure > > Andrew> the generic is suitable to replace scoped_finish_thread_state. > > > > Andrew> (I'm not trying to pick on scoped_finish_thread_state, it was just the > > Andrew> first example I found when originally replying to Tom.) > > > > Maybe I was just making too big a deal out of it, but my thinking was > > that writing the finish_thread_state call at each spot would be bad, > > since it would be multiple copies of the same thing. But, maybe it is > > actually no big deal? > > > > Using a template, as Pedro suggested, would remove some of the ugliness > > from the series. Stuff like this: > > > > + auto do_invalidate > > + = [=] () > > + { > > + this->invalidate (regnum); > > + }; > > + cleanup_function invalidator (do_invalidate); > > > > Could instead just be: > > > > SCOPE_EXIT { this->invalidate (regnum); } > > > > ... assuming we like SCOPE_EXIT (to me it seems reasonable enough). > > > > Anyway, I tend to think we should simply copy the scope_exit paper. If > > it's accepted into C++ then someday we can just remove the gdb variant. > > > > Let me know if you agree; if so I can implement this. > > > I've also played with the template idea, basically implemented > scope_exit / make_scope_exit. Seems to work nicely. > > I hadn't done the SCOPE_EXIT macro though, not sure it is worth > it to have yet another way to write these things (thinking about > newcomers' cognitive load, having to learn all the different > things) -- of all the make_scope_exit calls I have, most either > take the form: > > auto cleanup = make_scope_exit (function); > > i.e., are passed a function pointer, can do without a > lambda, and/or need access to the scope_exit object to > cancel it. But I can give it a try for sure. It may > be clearer code to standardize writing: > > auto cleanup = make_scope_exit (function); > cleanup.release (); > > when the cleanup may need to be canceled and > > SCOPE_EXIT { function (); } > > when it doesn't? > > I won't be able to finish this today (I'd like to clean up a couple hacks > here and there), but I'll post something tomorrow so we can all see > and decide a way forward. I'm happy with either approach so long as it opens the door for removing cleanup classes that are just calling a global function. Thanks for all the work on this. Thanks, Andrew