This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Flat DW_TAG_module support (PR fortran/9806)
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 11 Feb 2009 11:17:59 -0800
- Subject: Re: [patch] Flat DW_TAG_module support (PR fortran/9806)
- References: <20090207143504.GA28253@host0.dyn.jankratochvil.net>
> 2009-02-07 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> PR fortran/9806
> * dwarf2read.c (scan_partial_symbols <DW_TAG_module)
> (process_die <DW_TAG_module>): New.
Just a few minor requests...
> gdb/testsuite/
> 2009-02-07 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> PR fortran/9806
> * gdb.fortran/module.exp, gdb.fortran/module.f90: New.
This part looks OK to me, although I do have one suggestion.
> + case DW_TAG_module:
> + /* FIXME: Support the separate Fortran module namespaces. */
> + if (pdi->has_children)
> + scan_partial_symbols (pdi->die_child, lowpc, highpc, cu);
> + break;
To be consistent with the current approach, would you mind putting
this code into its own routine (add_partial_module), and then call
this routine from here. I know it's a little bit overkill given
the current limited implementation, but it'll set things up nicely
for when we do want to implement modules support.
> + case DW_TAG_module:
> + /* FIXME: Support the separate Fortran module namespaces. */
> + if (die->child != NULL)
> + {
> + struct die_info *child_die = die->child;
> +
> + while (child_die && child_die->tag)
> + {
> + process_die (child_die, cu);
> + child_die = sibling_die (child_die);
> + }
> + }
Same here: Can we put this in its own subprogram (read_module)?
> +gdb_test "print i" "\\$\[0-9\]+ = 42"
Here, your expected output is not incorrect. But you can also drop
the complex regexp that matches the "$NUM" part of the output if
you like:
gdb_test "print i" " = 42"
This is a common practice in our testsuite.
--
Joel