From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 48880 invoked by alias); 3 Jan 2019 22:45:23 -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 21666 invoked by uid 89); 3 Jan 2019 22:45:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,KAM_SHORT,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=familiar, lean X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 03 Jan 2019 22:45:03 +0000 Received: by simark.ca (Postfix, from userid 112) id 4FEA41E7AF; Thu, 3 Jan 2019 17:45:01 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 722F71E4C0; Thu, 3 Jan 2019 17:45:00 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 03 Jan 2019 22:45:00 -0000 From: Simon Marchi To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 00/12] Remove some ALL_* iteration macros In-Reply-To: <87y381v2iu.fsf@tromey.com> References: <20181125165439.13773-1-tom@tromey.com> <4406ff6a-975d-0db7-747c-27c7edda8bdb@simark.ca> <87y381v2iu.fsf@tromey.com> Message-ID: X-Sender: simark@simark.ca User-Agent: Roundcube Webmail/1.3.6 X-SW-Source: 2019-01/txt/msg00069.txt.bz2 On 2019-01-03 16:45, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi writes: > > Simon> I was wondering if you had thought about replacing, for example > Simon> ALL_COMPUNITS (objfile, s) > Simon> with an equivalent like this > Simon> for (compunit_symtab *s : all_compunits > (current_program_space)) > Simon> in order to avoid nested loops like > [...] > > Yeah, I don't think I really considered it. > > Simon> I am not sure which one I like best. The flat version reduces > indentation, but > Simon> the nested version makes it clear and explicit how the data is > represented, so > Simon> it might help readers who are less familiar with the code. > > Same for me. Maybe I lean a bit toward the explicit form but that > might > only be because I already have the patch in hand. > > Simon> Also, in theory, according to the coding style, we should write > Simon> for (...) > Simon> { > Simon> for (...) > Simon> { > Simon> ... > Simon> ... > Simon> } > Simon> } > > I thought it was ok to leave a single statement unbraced, though I > personally never do this for something like: > > for (...) > if ... > else... > > ...since I think that's less readable than the braced version. > > If the braces are needed then that probably argues for a smarter > iterator, to avoid excessive indentation. They are needed if we want to strictly follow the GNU coding style: https://www.gnu.org/prep/standards/standards.html#Clean-Use-of-C-Constructs I think what you did is easy to read, since it's pretty straightforward. We could always make an exception for these constructs, but it would probably end up being confusing to understand and explain when you can omit the braces and when you can't. If we end up using "smarter iterators" (for the lack of a better name) they could be overloads: /* Iterate on all compunits of an objfile. */ ... all_compunits (objfile *); / Iterate on all compunits of a program space. */ ... all_compunits (program_space *); And let's say that all_compunits (program_space *) returns tuples of , we'll be able to use structured bindings when we switch to C++17 :). Something like: for (const auto &[objfile, compunit] : all_compunits (pspace)) Simon