Re: [PATCH v2 2/2] fortran: enable ptype/whatis for modules.

[Doug Evans <xdje42 at gmail dot com>]

On Wed, Dec 11, 2013 at 4:35 AM, Keven Boell <keven.boell@intel.com> wrote:
> Added new domain MODULE_DOMAIN for fortran modules to avoid
> issues with sharing namespaces (e.g. when a variable currently
> in scope has the same name as a module).
>
>         (gdb) ptype modname
>         old> No symbol "modname" in current context.
>         new> type = module modname
>
> This fixes PR 15209 and also addresses the issue
> with sharing namespaces:
> https://sourceware.org/ml/gdb-patches/2013-02/msg00643.html
>
> 2013-11-19  Keven Boell  <keven.boell@intel.com>
>             Sanimir Agovic  <sanimir.agovic@intel.com>
>
>         * cp-namespace.c (cp_lookup_nested_symbol): Enable
>         nested lookups for fortran modules.
>         * dwarf2read.c (read_module): Add fortran module to
>         the symbol table.
>         (add_partial_symbol, add_partial_module): Add fortran
>         module to the partial symbol table.
>         (new_symbol_full): Create full symbol for fortran module.
>         * f-exp.y (yylex): Add new module domain to be parsed.
>         * symtab.h: New domain for fortran modules.
>
> testsuite/
>
>         * gdb.fortran/module.exp: Completion matches fortran module
>         names as well. ptype/whatis on modules return a proper type.
>         Add new check for having the correct scope.
>
> Signed-off-by: Keven Boell <keven.boell@intel.com>
> ---
>  gdb/cp-namespace.c                   |    1 +
>  gdb/dwarf2read.c                     |   20 ++++++++++++++++++++
>  gdb/f-exp.y                          |    3 ++-
>  gdb/symtab.h                         |    4 ++++
>  gdb/testsuite/gdb.fortran/module.exp |   13 ++++++++++---
>  gdb/testsuite/gdb.fortran/module.f90 |   12 ++++++++++++
>  6 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
> index 36134c0..06d8c76 100644
> --- a/gdb/cp-namespace.c
> +++ b/gdb/cp-namespace.c
> @@ -784,6 +784,7 @@ cp_lookup_nested_symbol (struct type *parent_type,
>      case TYPE_CODE_STRUCT:
>      case TYPE_CODE_NAMESPACE:
>      case TYPE_CODE_UNION:
> +    case TYPE_CODE_MODULE:

My gut reaction is to shudder and hope we can assert fail for this
case instead. :-)
[having to handle non-c++ stuff in cp-anything.c seems odd at first glance]
The comment for this function says it's only called when parent_type
is a c++ class or namespace.
How do we get here for a module?
[And if we can legitimately get here, a comment explaining "why things
are the way they are" is required (i.e. explain why we have to handle
TYPE_CODE_MODULE here).
I say that a lot, I know.  But it really helps me when I'm in the code later.]

>        {
>         /* NOTE: carlton/2003-11-10: We don't treat C++ class members
>            of classes like, say, data or function members.  Instead,
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 1c7dfc5..8d07f86 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -6801,6 +6801,13 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu)
>                            &objfile->global_psymbols,
>                            0, (CORE_ADDR) 0, cu->language, objfile);
>        break;
> +    case DW_TAG_module:
> +      add_psymbol_to_list (actual_name, strlen (actual_name),
> +                          built_actual_name != NULL,
> +                          MODULE_DOMAIN, LOC_TYPEDEF,
> +                          &objfile->global_psymbols,
> +                          0, (CORE_ADDR) 0, cu->language, objfile);
> +      break;
>      case DW_TAG_class_type:
>      case DW_TAG_interface_type:
>      case DW_TAG_structure_type:
> @@ -6871,6 +6878,10 @@ static void
>  add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
>                     CORE_ADDR *highpc, int need_pc, struct dwarf2_cu *cu)
>  {
> +  /* Add a symbol for the namespace.  */
> +
> +  add_partial_symbol (pdi, cu);
> +
>    /* Now scan partial symbols in that module.  */
>
>    if (pdi->has_children)
> @@ -13660,6 +13671,10 @@ static void
>  read_module (struct die_info *die, struct dwarf2_cu *cu)
>  {
>    struct die_info *child_die = die->child;
> +  struct type *type;
> +
> +  type = read_type_die (die, cu);
> +  new_symbol (die, type, cu);
>
>    while (child_die && child_die->tag)
>      {
> @@ -17690,6 +17705,11 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
>           SYMBOL_ACLASS_INDEX (sym) = LOC_TYPEDEF;
>           list_to_add = &global_symbols;
>           break;
> +       case DW_TAG_module:
> +         SYMBOL_ACLASS_INDEX (sym) = LOC_TYPEDEF;
> +         SYMBOL_DOMAIN (sym) = MODULE_DOMAIN;
> +         list_to_add = &global_symbols;
> +         break;
>         case DW_TAG_common_block:
>           SYMBOL_ACLASS_INDEX (sym) = LOC_COMMON_BLOCK;
>           SYMBOL_DOMAIN (sym) = COMMON_BLOCK_DOMAIN;
> diff --git a/gdb/f-exp.y b/gdb/f-exp.y
> index a7e59df..efddf1f 100644
> --- a/gdb/f-exp.y
> +++ b/gdb/f-exp.y
> @@ -1175,7 +1175,8 @@ yylex (void)
>      char *tmp = copy_name (yylval.sval);
>      struct symbol *sym;
>      struct field_of_this_result is_a_field_of_this;
> -    enum domain_enum_tag lookup_domains[] = {STRUCT_DOMAIN, VAR_DOMAIN};
> +    enum domain_enum_tag lookup_domains[] = {STRUCT_DOMAIN, VAR_DOMAIN,
> +      MODULE_DOMAIN};

I'm sure there's a rule that says this needs to be formatted
differently, though I'm not sure of the details.
Maybe begin a new line at the {, and put } on a line by itself?

>      int i;
>      int hextype;
>
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 7cc6667..cca6a76 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -418,6 +418,10 @@ typedef enum domain_enum_tag
>
>    STRUCT_DOMAIN,
>
> +  /* MODULE_DOMAIN is used in Fortran to hold module type names.  */
> +
> +  MODULE_DOMAIN,
> +
>    /* LABEL_DOMAIN may be used for names of labels (for gotos).  */
>
>    LABEL_DOMAIN,
> diff --git a/gdb/testsuite/gdb.fortran/module.exp b/gdb/testsuite/gdb.fortran/module.exp
> index 6a2b87d..8f5f1b7 100644
> --- a/gdb/testsuite/gdb.fortran/module.exp
> +++ b/gdb/testsuite/gdb.fortran/module.exp
> @@ -43,6 +43,13 @@ gdb_breakpoint [gdb_get_line_number "i-is-2"]
>  gdb_continue_to_breakpoint "i-is-2" ".*i-is-2.*"
>  gdb_test "print var_i" " = 2" "print var_i value 2"
>
> +gdb_breakpoint [gdb_get_line_number "i-is-3"]
> +gdb_continue_to_breakpoint "i-is-3" ".*i-is-3.*"
> +# Ensure that the scope is correctly resolved.
> +gdb_test "p mod3" "Attempt to use a type name as an expression" "print mod3"
> +gdb_test "p mod2" " = 3" "print mod2"
> +gdb_test "p mod1" " = 3" "print mod1"
> +
>  gdb_breakpoint [gdb_get_line_number "a-b-c-d"]
>  gdb_continue_to_breakpoint "a-b-c-d" ".*a-b-c-d.*"
>  gdb_test "print var_a" "No symbol \"var_a\" in current context\\."
> @@ -54,7 +61,7 @@ gdb_test "print var_x" " = 30" "print var_x value 30"
>  gdb_test "print var_y" "No symbol \"var_y\" in current context\\."
>  gdb_test "print var_z" " = 31" "print var_x value 31"
>
> -gdb_test "ptype modmany" {No symbol "modmany" in current context.}
> +gdb_test "ptype modmany" "type = module modmany"
>
>  proc complete {expr list} {
>      set cmd "complete p $expr"
> @@ -62,8 +69,8 @@ proc complete {expr list} {
>      gdb_test $cmd $expect "complete $expr"
>  }
>  set modmany_list {modmany::var_a modmany::var_b modmany::var_c modmany::var_i}
> -complete "modm" $modmany_list
> -complete "modmany" $modmany_list
> +complete "modm" "modmany $modmany_list"
> +complete "modmany" "modmany $modmany_list"
>  complete "modmany::" $modmany_list
>  complete "modmany::var" $modmany_list
>
> diff --git a/gdb/testsuite/gdb.fortran/module.f90 b/gdb/testsuite/gdb.fortran/module.f90
> index ada7262..d10b1fb 100644
> --- a/gdb/testsuite/gdb.fortran/module.f90
> +++ b/gdb/testsuite/gdb.fortran/module.f90
> @@ -23,6 +23,12 @@ module mod2
>          integer :: var_i = 2
>  end module mod2
>
> +module mod3
> +        integer :: mod2 = 3
> +        integer :: mod1 = 3
> +        integer :: var_i = 3
> +end module mod3
> +
>  module modmany
>          integer :: var_a = 10, var_b = 11, var_c = 12, var_i = 14
>  end module modmany
> @@ -43,6 +49,11 @@ end module moduse
>          var_i = var_i                         ! i-is-2
>          end
>
> +        subroutine sub3
> +        USE mod3
> +        var_i = var_i                         ! i-is-3
> +        END
> +
>          program module
>
>          use modmany, only: var_b, var_d => var_c, var_i
> @@ -50,6 +61,7 @@ end module moduse
>
>          call sub1
>          call sub2
> +        call sub3
>
>          if (var_b .ne. 11) call abort
>          if (var_d .ne. 12) call abort
> --
> 1.7.9.5
>


The rest seems ok to me.