[RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[Petr Sorfa]

[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]

Hi,

The patch below provides support for DW_TAG_module which is used to
represent FORTRAN modules, and possibly other language constructs, such
as C++ namespaces. The patch basically provides support for a module as
a C++ class with static members and functions.

2002-07-16  Petr Sorfa <petrs@caldera.com>

        * dwarf2read.c (dwarf2_update_pc_bounds): New function
        that updates a die's low and high pc bound. Used to
        remedy possible inconsistencies between ELF and DWARF
        information.
        (dwarf2_get_child_pc_bounds): New function that
        calculates the scope bounds of a die's children.
        (dwarf2_add_member_fn): Added a new argument that holds
        a parent die's tag type. Used to determine whether
        a child die's parent is a module and treat the child
        die as a static.
        (scan_partial_symbol): Support the DW_TAG_module tag.
        (add_partial_symbol): Support the DW_TAG_module tag.
        (process_die): Supports the DW_TAG_module tag by
        treating it as a structure scope.
        (read_file_scope): Moved  and enhanced some of the code
         to create the new dwarf2_get_child_pc_bounds()
        function.
        (read_structure_scope): Now supports the DW_TAG_module
        tag as a class and handles module variables and
        functions/subroutines as static members.
        (new_symbol): Now supports the DW_TAG_module tag and
        adds a module symbol to the global symbol list.
        (read_die_type): Now supports the DW_TAG_module tag
        as a structure scope.

[-- Attachment #2: fortran_module.patch --]
[-- Type: text/plain, Size: 11030 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.62
diff -c -p -r1.62 dwarf2read.c
*** dwarf2read.c	12 Jul 2002 19:55:10 -0000	1.62
--- dwarf2read.c	16 Jul 2002 20:19:30 -0000
*************** static void read_func_scope (struct die_
*** 793,801 ****
--- 793,809 ----
  static void read_lexical_block_scope (struct die_info *, struct objfile *,
  				      const struct comp_unit_head *);
  
+ static void dwarf2_update_pc_bounds (struct die_info *,
+                                      CORE_ADDR, CORE_ADDR,
+                                      const struct comp_unit_head *);
+ 
  static int dwarf2_get_pc_bounds (struct die_info *,
  				 CORE_ADDR *, CORE_ADDR *, struct objfile *);
  
+ static void dwarf2_get_child_pc_bounds (struct die_info *, CORE_ADDR *,
+ 			                CORE_ADDR *, struct objfile *,
+                                         const struct comp_unit_head *);
+ 
  static void dwarf2_add_field (struct field_info *, struct die_info *,
  			      struct objfile *, const struct comp_unit_head *);
  
*************** static void dwarf2_attach_fields_to_type
*** 803,809 ****
  					  struct type *, struct objfile *);
  
  static void dwarf2_add_member_fn (struct field_info *,
! 				  struct die_info *, struct objfile *objfile,
  				  const struct comp_unit_head *);
  
  static void dwarf2_attach_fn_fields_to_type (struct field_info *,
--- 811,818 ----
  					  struct type *, struct objfile *);
  
  static void dwarf2_add_member_fn (struct field_info *,
! 				  struct die_info *, unsigned int,
!                                   struct objfile *objfile,
  				  const struct comp_unit_head *);
  
  static void dwarf2_attach_fn_fields_to_type (struct field_info *,
*************** scan_partial_symbols (char *info_ptr, st
*** 1325,1330 ****
--- 1334,1342 ----
  		    }
  		}
  	      break;
+             case DW_TAG_module:
+ 		  add_partial_symbol (&pdi, objfile, cu_header);
+               break;
  	    case DW_TAG_variable:
  	    case DW_TAG_typedef:
  	    case DW_TAG_class_type:
*************** add_partial_symbol (struct partial_die_i
*** 1454,1459 ****
--- 1466,1472 ----
  			   &objfile->static_psymbols,
  			   0, (CORE_ADDR) 0, cu_language, objfile);
        break;
+     case DW_TAG_module:
      case DW_TAG_class_type:
      case DW_TAG_structure_type:
      case DW_TAG_union_type:
*************** add_partial_symbol (struct partial_die_i
*** 1467,1473 ****
  			   &objfile->static_psymbols,
  			   0, (CORE_ADDR) 0, cu_language, objfile);
  
!       if (cu_language == language_cplus)
  	{
  	  /* For C++, these implicitly act as typedefs as well. */
  	  add_psymbol_to_list (pdi->name, strlen (pdi->name),
--- 1480,1486 ----
  			   &objfile->static_psymbols,
  			   0, (CORE_ADDR) 0, cu_language, objfile);
  
!       if (cu_language == language_cplus || cu_language == language_fortran)
  	{
  	  /* For C++, these implicitly act as typedefs as well. */
  	  add_psymbol_to_list (pdi->name, strlen (pdi->name),
*************** process_die (struct die_info *die, struc
*** 1616,1621 ****
--- 1629,1637 ----
      {
      case DW_TAG_padding:
        break;
+     case DW_TAG_module:
+       read_structure_scope (die, objfile, cu_header);
+       break;
      case DW_TAG_compile_unit:
        read_file_scope (die, objfile, cu_header);
        break;
*************** read_file_scope (struct die_info *die, s
*** 1698,1721 ****
  
    if (!dwarf2_get_pc_bounds (die, &lowpc, &highpc, objfile))
      {
!       if (die->has_children)
! 	{
! 	  child_die = die->next;
! 	  while (child_die && child_die->tag)
! 	    {
! 	      if (child_die->tag == DW_TAG_subprogram)
! 		{
! 		  CORE_ADDR low, high;
! 
! 		  if (dwarf2_get_pc_bounds (child_die, &low, &high, objfile))
! 		    {
! 		      lowpc = min (lowpc, low);
! 		      highpc = max (highpc, high);
! 		    }
! 		}
! 	      child_die = sibling_die (child_die);
! 	    }
! 	}
      }
  
    /* If we didn't find a lowpc, set it to highpc to avoid complaints
--- 1714,1720 ----
  
    if (!dwarf2_get_pc_bounds (die, &lowpc, &highpc, objfile))
      {
!       dwarf2_get_child_pc_bounds (die, &lowpc, &highpc, objfile, cu_header);
      }
  
    /* If we didn't find a lowpc, set it to highpc to avoid complaints
*************** read_lexical_block_scope (struct die_inf
*** 1965,1970 ****
--- 1964,2003 ----
    local_symbols = new->locals;
  }
  
+ /* Update the dwarf attribute low and high pc bounds for a die.
+    This is usually used to resolve any conflict between
+    ELF and DWARF information. */
+ static void
+ dwarf2_update_pc_bounds (struct die_info *die, CORE_ADDR lowpc,
+ 			CORE_ADDR highpc,
+                         const struct comp_unit_head *cu_header)
+ {
+   struct attribute *attr;
+   CORE_ADDR low;
+   CORE_ADDR high;
+ 
+   attr = dwarf_attr (die, DW_AT_low_pc);
+   if (attr)
+     {
+       low = DW_ADDR (attr);
+ 
+       if (low > lowpc)
+         {
+           DW_ADDR (attr) = lowpc;
+         }
+     }
+   attr = dwarf_attr (die, DW_AT_high_pc);
+   if (attr)
+     {
+       high = DW_ADDR (attr);
+ 
+       if (high < highpc)
+         {
+           DW_ADDR (attr) = highpc;
+         }
+     }
+ }
+ 
  /* Get low and high pc attributes from a die.
     Return 1 if the attributes are present and valid, otherwise, return 0.  */
  
*************** dwarf2_get_pc_bounds (struct die_info *d
*** 2006,2011 ****
--- 2039,2099 ----
    return 1;
  }
  
+ /* Calculate the pc bounds of the scope of all the die's children */
+ static void
+ dwarf2_get_child_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
+ 			    CORE_ADDR *highpc, struct objfile *objfile,
+                             const struct comp_unit_head *cu_header)
+ {
+   struct die_info *child_die;
+ 
+   if (die->has_children)
+     {
+       child_die = die->next;
+       while (child_die && child_die->tag)
+         {
+ 
+           if (child_die->tag == DW_TAG_subprogram)
+             {
+               CORE_ADDR low, high;
+ 
+               if (dwarf2_get_pc_bounds (child_die, &low, &high, objfile))
+                 {
+                   *lowpc = min (*lowpc, low);
+                   *highpc = max (*highpc, high);
+                 }
+ 
+               /* Now check its children as well. This is for cases in
+                  FORTRAN 95 where a CONTAINS scope is a child of the
+                  main subprogram, but its low and high pc are outside
+                  of the main subprogram's low and high pc. */
+               dwarf2_get_child_pc_bounds (child_die, &low, &high, objfile,
+                                           cu_header);
+ 
+               *lowpc = min (*lowpc, low);
+               *highpc = max (*highpc, high);
+ 
+               /* Need to update the pc bounds for the enclosing
+                  subprogram to prevent gdb from moaning about
+                  inner blocks being outside the scope of the enclosing
+                  blocks. FIXME? FIX compiler? */
+               dwarf2_update_pc_bounds (child_die, low, high, cu_header);
+             }
+           else if (child_die->tag == DW_TAG_module)
+             {
+               CORE_ADDR low, high;
+ 
+               dwarf2_get_child_pc_bounds (child_die, &low, &high, objfile,
+                                           cu_header);
+ 
+               *lowpc = min (*lowpc, low);
+               *highpc = max (*highpc, high);
+             }
+           child_die = sibling_die (child_die);
+         }
+     }
+ }
+ 
  /* Add an aggregate field to the field list.  */
  
  static void
*************** dwarf2_attach_fields_to_type (struct fie
*** 2255,2260 ****
--- 2343,2349 ----
  
  static void
  dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
+ 		      unsigned int parent_tag,
  		      struct objfile *objfile,
  		      const struct comp_unit_head *cu_header)
  {
*************** dwarf2_add_member_fn (struct field_info 
*** 2337,2342 ****
--- 2426,2435 ----
           from read_subroutine_type via TYPE_FIELD_ARTIFICIAL.  */
        if (nparams == 0 || TYPE_FIELD_ARTIFICIAL (die->type, 0) == 0)
  	fnp->voffset = VOFFSET_STATIC;
+       
+       /* If the parent tag is a module, treat all functions as static */
+       if (parent_tag == DW_TAG_module)
+         fnp->voffset = VOFFSET_STATIC;
      }
    else
      complain (&dwarf2_missing_member_fn_type_complaint, physname);
*************** read_structure_scope (struct die_info *d
*** 2465,2470 ****
--- 2558,2570 ----
      {
        TYPE_CODE (type) = TYPE_CODE_UNION;
      }
+   else if (die->tag == DW_TAG_module)
+     {
+       /* Handle the FORTRAN module as a C++ class with static entries */
+       TYPE_CODE (type) = TYPE_CODE_CLASS;
+       ALLOCATE_CPLUS_STRUCT_TYPE (type);
+       TYPE_DECLARED_TYPE (type) = DECLARED_TYPE_CLASS;
+     }
    else
      {
        /* FIXME: TYPE_CODE_CLASS is currently defined to TYPE_CODE_STRUCT
*************** read_structure_scope (struct die_info *d
*** 2505,2518 ****
  	    }
  	  else if (child_die->tag == DW_TAG_variable)
  	    {
! 	      /* C++ static member.  */
  	      dwarf2_add_field (&fi, child_die, objfile, cu_header);
  	    }
  	  else if (child_die->tag == DW_TAG_subprogram)
  	    {
! 	      /* C++ member function. */
  	      process_die (child_die, objfile, cu_header);
! 	      dwarf2_add_member_fn (&fi, child_die, objfile, cu_header);
  	    }
  	  else if (child_die->tag == DW_TAG_inheritance)
  	    {
--- 2605,2623 ----
  	    }
  	  else if (child_die->tag == DW_TAG_variable)
  	    {
! 	      /* C++ static member or FORTRAN 95 module variable.  */
!               if (die->tag == DW_TAG_module)
!                 {
!                   /* Adding fortran module variable */
! 	          child_die->tag = DW_TAG_variable;
!                 }
  	      dwarf2_add_field (&fi, child_die, objfile, cu_header);
  	    }
  	  else if (child_die->tag == DW_TAG_subprogram)
  	    {
! 	      /* C++ member / FORTRAN 95 module function. */
  	      process_die (child_die, objfile, cu_header);
! 	      dwarf2_add_member_fn (&fi, child_die, die->tag, objfile, cu_header);
  	    }
  	  else if (child_die->tag == DW_TAG_inheritance)
  	    {
*************** new_symbol (struct die_info *die, struct
*** 4815,4820 ****
--- 4920,4929 ----
  	     interest in this information, so just ignore it for now.
  	     (FIXME?) */
  	  break;
+ 	case DW_TAG_module:
+ 	  SYMBOL_CLASS (sym) = LOC_STATIC;
+ 	  add_symbol_to_list (sym, &global_symbols);
+           break;
  	case DW_TAG_class_type:
  	case DW_TAG_structure_type:
  	case DW_TAG_union_type:
*************** read_type_die (struct die_info *die, str
*** 5075,5080 ****
--- 5184,5193 ----
  {
    switch (die->tag)
      {
+     case DW_TAG_module:
+       /* Read the module scope a structure. */
+       read_structure_scope (die, objfile, cu_header);
+       break;
      case DW_TAG_class_type:
      case DW_TAG_structure_type:
      case DW_TAG_union_type:

Re: [RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[Jim Blandy]

If I understand this patch correctly, it stores a Fortran module in
GDB's symbol table as if it were a C++ structure type, full of static
data and function members.  A Fortran module gets stored in GDB's
symbol table as a LOC_TYPEDEF symbol.  Is that right?

I'd like to hear other folks' opinions on this.

It seems to me like it would work, since a class establishes a scope
that behaves very much like a module.  Isn't Class::member, where
member is a static thingy, very much like Module::member (or whatever
the Fortran syntax is)?  We already have logic for resolving class
scopes; I don't immediately see why module scopes need to behave any
differently.

But I think a module should be represented by something that calls
itself a module, not a typedef.  How will people feel reading a
comment explaining that a LOC_TYPEDEF for a type with TYPE_CODE_CLASS
is how we represent Fortran modules?  I'm not sure that counts as good
maintenance.

Perhaps we should introduce LOC_MODULE, or TYPE_CODE_MODULE.

Or maybe this is okay for now.  When we provide better support for C++
namespaces, Fortran modules can become a variant of that, which feels
like a better fit.

But without a better sense of how people feel about this issue, 
I don't think I can comment specifically on this patch.


Petr Sorfa <petrs@caldera.com> writes:

> Hi,
> 
> The patch below provides support for DW_TAG_module which is used to
> represent FORTRAN modules, and possibly other language constructs, such
> as C++ namespaces. The patch basically provides support for a module as
> a C++ class with static members and functions.
> 
> 2002-07-16  Petr Sorfa <petrs@caldera.com>
> 
>         * dwarf2read.c (dwarf2_update_pc_bounds): New function
>         that updates a die's low and high pc bound. Used to
>         remedy possible inconsistencies between ELF and DWARF
>         information.
>         (dwarf2_get_child_pc_bounds): New function that
>         calculates the scope bounds of a die's children.
>         (dwarf2_add_member_fn): Added a new argument that holds
>         a parent die's tag type. Used to determine whether
>         a child die's parent is a module and treat the child
>         die as a static.
>         (scan_partial_symbol): Support the DW_TAG_module tag.
>         (add_partial_symbol): Support the DW_TAG_module tag.
>         (process_die): Supports the DW_TAG_module tag by
>         treating it as a structure scope.
>         (read_file_scope): Moved  and enhanced some of the code
>          to create the new dwarf2_get_child_pc_bounds()
>         function.
>         (read_structure_scope): Now supports the DW_TAG_module
>         tag as a class and handles module variables and
>         functions/subroutines as static members.
>         (new_symbol): Now supports the DW_TAG_module tag and
>         adds a module symbol to the global symbol list.
>         (read_die_type): Now supports the DW_TAG_module tag
>         as a structure scope.Index: dwarf2read.c

Re: [RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[Daniel Jacobowitz]

On Thu, Aug 15, 2002 at 04:31:15PM -0500, Jim Blandy wrote:
> 
> If I understand this patch correctly, it stores a Fortran module in
> GDB's symbol table as if it were a C++ structure type, full of static
> data and function members.  A Fortran module gets stored in GDB's
> symbol table as a LOC_TYPEDEF symbol.  Is that right?
> 
> I'd like to hear other folks' opinions on this.
> 
> It seems to me like it would work, since a class establishes a scope
> that behaves very much like a module.  Isn't Class::member, where
> member is a static thingy, very much like Module::member (or whatever
> the Fortran syntax is)?  We already have logic for resolving class
> scopes; I don't immediately see why module scopes need to behave any
> differently.
> 
> But I think a module should be represented by something that calls
> itself a module, not a typedef.  How will people feel reading a
> comment explaining that a LOC_TYPEDEF for a type with TYPE_CODE_CLASS
> is how we represent Fortran modules?  I'm not sure that counts as good
> maintenance.
> 
> Perhaps we should introduce LOC_MODULE, or TYPE_CODE_MODULE.
> 
> Or maybe this is okay for now.  When we provide better support for C++
> namespaces, Fortran modules can become a variant of that, which feels
> like a better fit.

That's my opinion.  Fortran modules are obvious candidates for a real
scope; when we have real scopes they can do that, and for now we can
handle them like classes.  I like the principle of the patch (though I
haven't looked at the patch itself).

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[Petr Sorfa]

Hi Jim,

> If I understand this patch correctly, it stores a Fortran module in
> GDB's symbol table as if it were a C++ structure type, full of static
> data and function members.  A Fortran module gets stored in GDB's
> symbol table as a LOC_TYPEDEF symbol.  Is that right?
Yes. The reason for a LOC_TYPEDEF is to resolve some evaluation issues.
The module in FORTRAN is not really an instance of anything since
everything is static and there is only one occurance of it (i.e. it is
not really equivalent to a C++ class), but rather a type. I think a
FORTRAN module should rather be thought of as a C++ namespace rather
than a C++ class. In fact this patch puts in ground work for namespace
handling.

> I'd like to hear other folks' opinions on this.
> 
> It seems to me like it would work, since a class establishes a scope
> that behaves very much like a module.  Isn't Class::member, where
> member is a static thingy, very much like Module::member (or whatever
> the Fortran syntax is)?  We already have logic for resolving class
> scopes; I don't immediately see why module scopes need to behave any
> differently.
I thought it did this. Can you point out where I'm going wrong?

> But I think a module should be represented by something that calls
> itself a module, not a typedef.  How will people feel reading a
> comment explaining that a LOC_TYPEDEF for a type with TYPE_CODE_CLASS
> is how we represent Fortran modules?  I'm not sure that counts as good
> maintenance.
I originally introduced a TYPE_CODE_MODULE that was basically equivalent
to a TYPE_CODE_CLASS, as much as TYPE_CODE_CLASS is really
TYPE_CODE_STRUCT. I think I must have pulled it out for the patch. I can
put it back in and make it equivalent to TYPE_CODE_CLASS.

> Perhaps we should introduce LOC_MODULE, or TYPE_CODE_MODULE.
TYPE_CODE_MODULE explained above. However, LOC_MODULE would be
equivalent in this case to LOC_TYPEDEF. If you look at the comment for
LOC_TYPEDEF concerning namespace (below this sentence) I am using a
similar angle.

   /* Value not used; definition in SYMBOL_TYPE.  Symbols in the
namespace
       STRUCT_NAMESPACE all have this class.  */ 

> Or maybe this is okay for now.  When we provide better support for C++
> namespaces, Fortran modules can become a variant of that, which feels
> like a better fit.
Yes. *Cough* maybe this patch can provide support for namespaces? ;o)
 
> But without a better sense of how people feel about this issue,
> I don't think I can comment specifically on this patch.
Ok, one step at a time. One I get your feedback on this, we can start
making a plan on how I should do this.

Petr
> 
> Petr Sorfa <petrs@caldera.com> writes:
> 
> > Hi,
> >
> > The patch below provides support for DW_TAG_module which is used to
> > represent FORTRAN modules, and possibly other language constructs, such
> > as C++ namespaces. The patch basically provides support for a module as
> > a C++ class with static members and functions.
> >
> > 2002-07-16  Petr Sorfa <petrs@caldera.com>
> >
> >         * dwarf2read.c (dwarf2_update_pc_bounds): New function
> >         that updates a die's low and high pc bound. Used to
> >         remedy possible inconsistencies between ELF and DWARF
> >         information.
> >         (dwarf2_get_child_pc_bounds): New function that
> >         calculates the scope bounds of a die's children.
> >         (dwarf2_add_member_fn): Added a new argument that holds
> >         a parent die's tag type. Used to determine whether
> >         a child die's parent is a module and treat the child
> >         die as a static.
> >         (scan_partial_symbol): Support the DW_TAG_module tag.
> >         (add_partial_symbol): Support the DW_TAG_module tag.
> >         (process_die): Supports the DW_TAG_module tag by
> >         treating it as a structure scope.
> >         (read_file_scope): Moved  and enhanced some of the code
> >          to create the new dwarf2_get_child_pc_bounds()
> >         function.
> >         (read_structure_scope): Now supports the DW_TAG_module
> >         tag as a class and handles module variables and
> >         functions/subroutines as static members.
> >         (new_symbol): Now supports the DW_TAG_module tag and
> >         adds a module symbol to the global symbol list.
> >         (read_die_type): Now supports the DW_TAG_module tag
> >         as a structure scope.Index: dwarf2read.c

Re: [RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[Jim Blandy]

Petr Sorfa <petrs@caldera.com> writes:
> > It seems to me like it would work, since a class establishes a scope
> > that behaves very much like a module.  Isn't Class::member, where
> > member is a static thingy, very much like Module::member (or whatever
> > the Fortran syntax is)?  We already have logic for resolving class
> > scopes; I don't immediately see why module scopes need to behave any
> > differently.
> I thought it did this. Can you point out where I'm going wrong?

Your patch is right, as far as I can see.  In that paragraph I'm just
saying why I think the patch should work fine.  In the next paragraph
I explain my reservations.

> > But I think a module should be represented by something that calls
> > itself a module, not a typedef.  How will people feel reading a
> > comment explaining that a LOC_TYPEDEF for a type with TYPE_CODE_CLASS
> > is how we represent Fortran modules?  I'm not sure that counts as good
> > maintenance.
> I originally introduced a TYPE_CODE_MODULE that was basically equivalent
> to a TYPE_CODE_CLASS, as much as TYPE_CODE_CLASS is really
> TYPE_CODE_STRUCT. I think I must have pulled it out for the patch. I can
> put it back in and make it equivalent to TYPE_CODE_CLASS.

If we're going to use a struct/class-like thingy to represent a
Fortran module, then we should at least add DECLARED_TYPE_MODULE (see
DECLARED_TYPE_CLASS, ... in gdbtypes.h).

> > Or maybe this is okay for now.  When we provide better support for C++
> > namespaces, Fortran modules can become a variant of that, which feels
> > like a better fit.
> Yes. *Cough* maybe this patch can provide support for namespaces?  ;o)

I'm not sure I really want the C++ `std' namespace represented as a
struct type.

Re: [RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[Petr Sorfa]

Hi Jim,

> Your patch is right, as far as I can see.  In that paragraph I'm just
> saying why I think the patch should work fine.  In the next paragraph
> I explain my reservations.
Ok.

> > > But I think a module should be represented by something that calls
> > > itself a module, not a typedef.  How will people feel reading a
> > > comment explaining that a LOC_TYPEDEF for a type with TYPE_CODE_CLASS
> > > is how we represent Fortran modules?  I'm not sure that counts as good
> > > maintenance.
> > I originally introduced a TYPE_CODE_MODULE that was basically equivalent
> > to a TYPE_CODE_CLASS, as much as TYPE_CODE_CLASS is really
> > TYPE_CODE_STRUCT. I think I must have pulled it out for the patch. I can
> > put it back in and make it equivalent to TYPE_CODE_CLASS.
> 
> If we're going to use a struct/class-like thingy to represent a
> Fortran module, then we should at least add DECLARED_TYPE_MODULE (see
> DECLARED_TYPE_CLASS, ... in gdbtypes.h).
Will do. I think I had that done as well.

> > > Or maybe this is okay for now.  When we provide better support for C++
> > > namespaces, Fortran modules can become a variant of that, which feels
> > > like a better fit.
> > Yes. *Cough* maybe this patch can provide support for namespaces?  ;o)
> 
> I'm not sure I really want the C++ `std' namespace represented as a
> struct type.
Please ignore that suggestion, it was made in jest. But you are right,
once namespace support it put in, it should be able to support FORTRAN
modules.

If I update the patch to use TYPE_CODE_MODULE and DECLARED_TYPE_CLASS,
would you reconsider the patch?

Petr

Re: [RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[Jim Blandy]

Petr Sorfa <petrs@caldera.com> writes:
> > Your patch is right, as far as I can see.  In that paragraph I'm just
> > saying why I think the patch should work fine.  In the next paragraph
> > I explain my reservations.
> Ok.
> 
> > > > But I think a module should be represented by something that calls
> > > > itself a module, not a typedef.  How will people feel reading a
> > > > comment explaining that a LOC_TYPEDEF for a type with TYPE_CODE_CLASS
> > > > is how we represent Fortran modules?  I'm not sure that counts as good
> > > > maintenance.
> > > I originally introduced a TYPE_CODE_MODULE that was basically equivalent
> > > to a TYPE_CODE_CLASS, as much as TYPE_CODE_CLASS is really
> > > TYPE_CODE_STRUCT. I think I must have pulled it out for the patch. I can
> > > put it back in and make it equivalent to TYPE_CODE_CLASS.
> > 
> > If we're going to use a struct/class-like thingy to represent a
> > Fortran module, then we should at least add DECLARED_TYPE_MODULE (see
> > DECLARED_TYPE_CLASS, ... in gdbtypes.h).
> Will do. I think I had that done as well.
> 
> > > > Or maybe this is okay for now.  When we provide better support for C++
> > > > namespaces, Fortran modules can become a variant of that, which feels
> > > > like a better fit.
> > > Yes. *Cough* maybe this patch can provide support for namespaces?  ;o)
> > 
> > I'm not sure I really want the C++ `std' namespace represented as a
> > struct type.
> Please ignore that suggestion, it was made in jest. But you are right,
> once namespace support it put in, it should be able to support FORTRAN
> modules.
> 
> If I update the patch to use TYPE_CODE_MODULE and DECLARED_TYPE_CLASS,
> would you reconsider the patch?

I'd like to hear Elena's comments on it, but if she thinks it's okay,
then I'll review the patch in detail.

Re: [RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[Elena Zannoni]

Jim Blandy writes:
 > Petr Sorfa <petrs@caldera.com> writes:
 > > > Your patch is right, as far as I can see.  In that paragraph I'm just
 > > > saying why I think the patch should work fine.  In the next paragraph
 > > > I explain my reservations.
 > > Ok.
 > > 
 > > > > > But I think a module should be represented by something that calls
 > > > > > itself a module, not a typedef.  How will people feel reading a
 > > > > > comment explaining that a LOC_TYPEDEF for a type with TYPE_CODE_CLASS
 > > > > > is how we represent Fortran modules?  I'm not sure that counts as good
 > > > > > maintenance.
 > > > > I originally introduced a TYPE_CODE_MODULE that was basically equivalent
 > > > > to a TYPE_CODE_CLASS, as much as TYPE_CODE_CLASS is really
 > > > > TYPE_CODE_STRUCT. I think I must have pulled it out for the patch. I can
 > > > > put it back in and make it equivalent to TYPE_CODE_CLASS.
 > > > 
 > > > If we're going to use a struct/class-like thingy to represent a
 > > > Fortran module, then we should at least add DECLARED_TYPE_MODULE (see
 > > > DECLARED_TYPE_CLASS, ... in gdbtypes.h).
 > > Will do. I think I had that done as well.
 > > 
 > > > > > Or maybe this is okay for now.  When we provide better support for C++
 > > > > > namespaces, Fortran modules can become a variant of that, which feels
 > > > > > like a better fit.
 > > > > Yes. *Cough* maybe this patch can provide support for namespaces?  ;o)
 > > > 
 > > > I'm not sure I really want the C++ `std' namespace represented as a
 > > > struct type.
 > > Please ignore that suggestion, it was made in jest. But you are right,
 > > once namespace support it put in, it should be able to support FORTRAN
 > > modules.
 > > 
 > > If I update the patch to use TYPE_CODE_MODULE and DECLARED_TYPE_CLASS,
 > > would you reconsider the patch?
 > 
 > I'd like to hear Elena's comments on it, but if she thinks it's okay,
 > then I'll review the patch in detail.


I think the using TYPE_CODE_MODULE and DECLARED_TYPE_CLASS will help
in distinguishing the Fortran modules.

I would prefer however is there were lots of new comments added to things
like this, to make things clearer:

!       if (cu_language == language_cplus || cu_language == language_fortran)
  	{
  	  /* For C++, these implicitly act as typedefs as well. */
  	  add_psymbol_to_list (pdi->name, strlen (pdi->name),

and

+     case DW_TAG_module:
+       read_structure_scope (die, objfile, cu_header);
+       break;


I don't nderstand the comment here:
+     case DW_TAG_module:
+       /* Read the module scope a structure. */
+       read_structure_scope (die, objfile, cu_header);



Elena

Re: [RFA] Patch for supportinf DW_TAG_module / FORTRAN modules

[Petr Sorfa]

Hi Elena,

Will do.

Petr
> Jim Blandy writes:
>  > Petr Sorfa <petrs@caldera.com> writes:
>  > > > Your patch is right, as far as I can see.  In that paragraph I'm just
>  > > > saying why I think the patch should work fine.  In the next paragraph
>  > > > I explain my reservations.
>  > > Ok.
>  > >
>  > > > > > But I think a module should be represented by something that calls
>  > > > > > itself a module, not a typedef.  How will people feel reading a
>  > > > > > comment explaining that a LOC_TYPEDEF for a type with TYPE_CODE_CLASS
>  > > > > > is how we represent Fortran modules?  I'm not sure that counts as good
>  > > > > > maintenance.
>  > > > > I originally introduced a TYPE_CODE_MODULE that was basically equivalent
>  > > > > to a TYPE_CODE_CLASS, as much as TYPE_CODE_CLASS is really
>  > > > > TYPE_CODE_STRUCT. I think I must have pulled it out for the patch. I can
>  > > > > put it back in and make it equivalent to TYPE_CODE_CLASS.
>  > > >
>  > > > If we're going to use a struct/class-like thingy to represent a
>  > > > Fortran module, then we should at least add DECLARED_TYPE_MODULE (see
>  > > > DECLARED_TYPE_CLASS, ... in gdbtypes.h).
>  > > Will do. I think I had that done as well.
>  > >
>  > > > > > Or maybe this is okay for now.  When we provide better support for C++
>  > > > > > namespaces, Fortran modules can become a variant of that, which feels
>  > > > > > like a better fit.
>  > > > > Yes. *Cough* maybe this patch can provide support for namespaces?  ;o)
>  > > >
>  > > > I'm not sure I really want the C++ `std' namespace represented as a
>  > > > struct type.
>  > > Please ignore that suggestion, it was made in jest. But you are right,
>  > > once namespace support it put in, it should be able to support FORTRAN
>  > > modules.
>  > >
>  > > If I update the patch to use TYPE_CODE_MODULE and DECLARED_TYPE_CLASS,
>  > > would you reconsider the patch?
>  >
>  > I'd like to hear Elena's comments on it, but if she thinks it's okay,
>  > then I'll review the patch in detail.
> 
> I think the using TYPE_CODE_MODULE and DECLARED_TYPE_CLASS will help
> in distinguishing the Fortran modules.
> 
> I would prefer however is there were lots of new comments added to things
> like this, to make things clearer:
> 
> !       if (cu_language == language_cplus || cu_language == language_fortran)
>         {
>           /* For C++, these implicitly act as typedefs as well. */
>           add_psymbol_to_list (pdi->name, strlen (pdi->name),
> 
> and
> 
> +     case DW_TAG_module:
> +       read_structure_scope (die, objfile, cu_header);
> +       break;
> 
> I don't nderstand the comment here:
> +     case DW_TAG_module:
> +       /* Read the module scope a structure. */
> +       read_structure_scope (die, objfile, cu_header);
> 
> Elena