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

[Simon Marchi <simark@simark.ca>]

On 2019-01-06 15:10, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> I think what you did is easy to read, since it's pretty
> Simon> straightforward. We could always make an exception for these
> Simon> constructs, but it would probably end up being confusing to 
> understand
> Simon> and explain when you can omit the braces and when you can't.
> 
> [...]
> 
> Simon> And let's say that all_compunits (program_space *) returns 
> tuples of
> Simon> <objfile *, compunit_symtab *>, we'll be able to use structured
> Simon> bindings when we switch to C++17 :).  Something like:
> 
> Simon> for (const auto &[objfile, compunit] : all_compunits (pspace))
> 
> I gave this a brief try.  I wrote a "nested" iterator to make this
> generic and then converted all_compunits.
> 
> With the nested iterator, one needs to write:
> 
>   for (auto blah : all_compunits ())
>     {
>       compunit_symtab *s = blah.second;
> 
> This looked ugly to me.

In that case it would always return a specialized struct

   struct {
     objfile *objfile;
     compunit_symtab *compunit;
   };

> Alternatively, we'd have to write a "second-selecting" wrapper 
> iterator.
> I didn't implement it but I suppose it would look like:
> 
>   for (compunit_symtab *s : second_adapter<all_compunits> ())
>      ...
> 
> This seemed a bit obscure to me, though.
> 
> In the end I think I prefer the explicit route.  It does need more
> indentation (I will add the missing braces), but the explicitness seems
> good.  To me, there doesn't seem to be a strong reason to prefer the
> shorter formulations; and the new iterators and adapters that would be
> needed are just a bunch more code to try to track through.
> 
> Let me know what you think about this.  If you like the second_adapter
> approach, I can implement that.

Yeah, I think the second_adapter approach just makes things more 
obscure.

Really, I am fine with what you did.

Simon