From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 123960 invoked by alias); 27 Dec 2018 01:52:12 -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 123671 invoked by uid 89); 27 Dec 2018 01:52:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=mine, adapters, ugly, am 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, 27 Dec 2018 01:52:10 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id EFF321E4B1; Wed, 26 Dec 2018 20:52:08 -0500 (EST) Subject: Re: [PATCH 00/12] Remove some ALL_* iteration macros To: Tom Tromey , gdb-patches@sourceware.org References: <20181125165439.13773-1-tom@tromey.com> From: Simon Marchi Message-ID: <4406ff6a-975d-0db7-747c-27c7edda8bdb@simark.ca> Date: Thu, 27 Dec 2018 01:52:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.3 MIME-Version: 1.0 In-Reply-To: <20181125165439.13773-1-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-12/txt/msg00348.txt.bz2 On 2018-11-25 11:54 a.m., Tom Tromey wrote: > This series removes various ALL_* iteration macros, in favor of C++ > iterators, range adapters, and ranged for loops. I've been wanting > this for a while, because it helps a little bit with various > experiments of mine that involve changing objfile lifetime management; > Pedro's thread iterator patch prompted me to finally do this. > > The main downside of removing these macros is that it involves some > reindentation; and expanding some macros to two nested loops means a > couple somewhat ugly reformattings. > > On the plus side, though, this tightens the scope of iteration > variables, which is good. And, it removes some hairy code, > particularly the ALL_OBJSECTIONS patch. > > There are still a few more such macros that could be converted. And, > I think inf_threads_iterator could be converted to use next_iterator. > I can do some of this if there's interest. > > Regression tested by the buildbot. > > Tom I gave that a quick look. I was wondering if you had thought about replacing, for example ALL_COMPUNITS (objfile, s) with an equivalent like this for (compunit_symtab *s : all_compunits (current_program_space)) in order to avoid nested loops like for (objfile *objfile : all_objfiles (current_program_space)) for (compunit_symtab *s : objfile_compunits (objfile)) { ... ... } In most cases, the objfile variable is not needed for anything else than iterating on the compunit_symtabs. For cases where the outer iteration variable is needed, then we can use the nested loops. I am not sure which one I like best. The flat version reduces indentation, but the nested version makes it clear and explicit how the data is represented, so it might help readers who are less familiar with the code. Also, in theory, according to the coding style, we should write for (...) { for (...) { ... ... } } Simon