From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103144 invoked by alias); 11 Jan 2019 06:56:50 -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 103130 invoked by uid 89); 11 Jan 2019 06:56:50 -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=situations, bespoke X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 Jan 2019 06:56:47 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 205235600C; Fri, 11 Jan 2019 01:56:46 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id fFPO-2r6gYlg; Fri, 11 Jan 2019 01:56:46 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id AA4F05600B; Fri, 11 Jan 2019 01:56:45 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 5C87F867B6; Fri, 11 Jan 2019 10:56:41 +0400 (+04) Date: Fri, 11 Jan 2019 06:56:00 -0000 From: Joel Brobecker To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 00/12] remove some cleanups using a cleanup function Message-ID: <20190111065641.GA22922@adacore.com> References: <20190109033426.16062-1-tom@tromey.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190109033426.16062-1-tom@tromey.com> User-Agent: Mutt/1.9.4 (2018-02-28) X-SW-Source: 2019-01/txt/msg00236.txt.bz2 > This is the series to introduce a new cleanup_function class, that can > be used in situations where a simple function call is needed to do > some cleanup. > > As I mentioned in the other thread, I was (and still am) uncertain as > to whether this is a good idea. My worry is just that this will be > used in ways leading to unclear code. (You can judge for yourself > whether this series has already entered that territory...) > > A couple of rules I used when deciding when to use the new class: > > * There should not be too many uses of the existing cleanup (one or > two). If there are more than several it should probably be a > bespoke class. > > * Any lambdas should capture only a small number of things (one or > maybe two), and only capture by value should be used. > > Hopefully I actually followed these. > > I wonder if the new class should take a std::function rather than a > gdb::function_view. Some of the changes needs a bit of extra code to > deal with the latter. > > Also I wonder if it should be possible to default-construct a > cleanup_function, and then set it later. This would eliminate some > uses of gdb::optional; and seemed maybe reasonable given that the > class already has to maintain a (sort of-) validity flag. > > This series includes a couple of cleanup-related comment fixes for > things I happened to notice along the way. I don't know C++ well enough to have an opinion on the general questions asked here, but FWIW, the patch series in itself seems like a nice improvement already (compared to before). -- Joel