Re: [RFC/dwarf-2] Add support for included files

[Jim Blandy <jimb@redhat.com>]

Joel Brobecker <brobecker@gnat.com> writes:
> Here is an updated version of the previous patch, adapted to fit in
> the current dwarf2read.c after Daniel's changes, and with comments
> added. It's actually slightly smaller thanks to Daniel's changes
> :-).

Great!

I actually have still more comments:

- Rather than passing 'line_offset' by reference to read_partial_die,
  could you add a line_offset field to 'struct partial_die_info, and have
  read_partial_die set that?  That would seem more consistent with the
  rest of what read_partial_die does.  Then
  'dwarf2_build_include_psymtabs' should take a pointer to the CU die.

  That's assuming Daniel doesn't see any problem enlarging
  partial_die_info.  If he does, maybe we could add the fields to
  'struct dwarf2_cu' instead: it's a little odd to have
  'read_partial_die' set the cu's fields directly, but not horribly
  so, as long as the die's tag is DW_TAG_compile_unit.

- Is it really okay to pass NULL to dwarf_decode_lines for comp_dir?
  Won't the filenames of the partial symbol tables be different from
  those of the symbol tables?  To fix this, read_partial_die would
  have to check for DW_AT_comp_dir attributes, too.

- It seems to me the file_is_included array should be in struct
  line_header, with add_file_name managing its allocation /
  initialization in parallel with file_names.  It's already the case
  that that structure is mutated by the act of processing the line
  number program, so I don't see that it's a serious change to the
  usage patterns of that structure type.

- I'd rather see the call to dwarf2_build_include_psymtabs before the
  assignment to info_ptr, not after.  Not that it matters much.

- Thanks for fixing the comments on dwarf_decode_lines --- especially
  the references to arguments that had been missing for some time now.