Re: [patch] Flat DW_TAG_module support (PR fortran/9806)

[Joel Brobecker <brobecker@adacore.com>]

> 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