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

Jim Blandy
2004-04-29T23:32:31Z

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.