Re: [PATCH 00/12] Remove some ALL_* iteration macros

[Simon Marchi <simark@simark.ca>]

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