This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]