Re: [Patch, gfortran] PR22304, 17917, 16511, 18870 and 23270 - modules,equivalences and commons

[Tobias Schlüter <tobias dot schlueter at physik dot uni-muenchen dot de>]

Paul Thomas wrote:
> I am aware that this has become a bit of a monster but it should be 
> quite comprehensible with the changelog remarks and the comments.
> 
> The improvement in functionality with modules/commons/equivalences 
> should be apparent from the testcases.
> 
> Bootstrapped and regtested on Athlon1700/FC3.
> 
> OK for mainline and 4.0?

Thanks for the work you put into this.  I have a few remarks below; except for
those, and the issues you pointed out in the followup, this is ok.

> =================fortran.diff===============
> 
> 2005-08-22  Paul Thomas  <pault@gcc.gnu.org>
> 
>     PR fortran/23270
>     * gfortran.h: Move definition of BLANK_COMMON_NAME from trans-
>     common.c so that it is accessible to module.c.
>     * module.c (write_blank_common): New.
>     * module.c (write_module): Call write_blank_common.
> 
> 2005-08-22  Paul Thomas  <pault@gcc.gnu.org>
> 
>     PR fortran/18870
>     * gfortran.h: Add common_head field to gfc_symbol structure.
>     * match.c (gfc_match_common, gfc_match_equivalence): In loops
>     that flag common block equivalences, emit an error if the
>     common blocks are different, using sym->common_head as the
>     common block identifier.
> 
> 2005-08-22  Paul Thomas  <pault@gcc.gnu.org>
> 
>     * module.c (write_common): Demangle common block names
>     and restore them to their fortran form.
>     * match.c (gfc_get_common): Mangle use associated common
>     blocks with an incrementing serial number for each block
>     and in each namespace. The common block symtree name is
>     left unmangled so that the external reference is OK.
> 
> 2005-08-22  Paul Thomas  <pault@gcc.gnu.org>
> 
>     * trans-decl.c (gfc_generate_function_code): Move the call to
>     gfc_generate_contained_functions to after the call to
>     gfc_trans_common so the use-associated, use-associated common
>     blocks produce the correct references.
> 
> 2005-08-22  Paul Thomas  <pault@gcc.gnu.org>
> 
>     PR fortran/16511
>     * gfortran.h: new attr field, in_equivalence.
>     * match.c (gfc_match_common, gfc_match_equivalence): Ensure that
>     symbols that are equivalence associated with a common block are
>     marked as being in_common.
> 
> 2005-08-22  Paul Thomas  <pault@gcc.gnu.org>
> 
>     PR fortran/17917
>     * module.c (load_equiv): New function ported from g95.
>     (read_module): Call load_equiv.
>     (write_equiv): New function ported from g95. Correct
>     string referencing for gfc functions. Give module
>     equivalences a unique name.
>     (write_module): Call write_equiv.
>     * trans_decl.c (gfc_create_module_variable): Return for
>     equivalenced symbols with existing backend declaration.
>     * trans-common.c (finish_equivalences): Provide the call
>     to create_common with a gfc_common_header so that
>     module equivalences are made external, rather than local.
>     * gfortran.h (gfc_equiv): Add field for the equivalence
>     name.
> 
> 2005-08-22  Paul Thomas  <pault@gcc.gnu.org>
>         Mike Albert  <albertm@uphs.upenn.edu>
> 
>     PR fortran/22304
>     * trans-common.c (find_equivalences): Ensure that all members
>     in common block equivalences are marked as used. This prevents
>     the subsequent call to this function from making local unions.

You should merge these ChangeLog entries, there's also nothing left of Mike's
patch, so I don't think you would taking away the credit he deserves by not
mentioning him in the ChangeLog.  If you think different, keep the last entry
separate.

> *************** typedef struct gfc_symbol
> *** 702,707 ****
> --- 704,714 ----
>     gfc_component *components;	/* Derived type components */
>   
>     struct gfc_symbol *common_next;	/* Links for COMMON syms */
> + 
> +   /* This is in fact a gfc_common_head but it is only used for pointer
> +      comparisons to check if symbols are in the same common block.  */
> +   void *common_head;
> + 
>     /* Make sure setup code for dummy arguments is generated in the correct
>        order.  */
>     int dummy_order;

Is there any reason to make this a void *?

> --- 2161,2194 ----
>   gfc_get_common (const char *name, int from_module)
>   {
>     gfc_symtree *st;
> !   int serial;
>     char mangled_name[GFC_MAX_SYMBOL_LEN+1];
>   
> +     /* A use associated common block is only needed to correctly layout
> +        the variables it contains.  Thus we give it a mangled name with
> +        a series number, to ensure that each common declaration is
> +        independent from the previous ones.  Making sure that this mangled
> +        name does not feed through to the mod files is done by write_module.  */
>     if (from_module)
>       {
> !       for (serial = 0;; serial++)
> ! 	{
> ! 	  snprintf(mangled_name, GFC_MAX_SYMBOL_LEN, "_%d_%s", serial, name);
> ! 	  st = gfc_find_symtree (gfc_current_ns->common_root, mangled_name);
> ! 	  if (!st)
> ! 	    break;
> ! 	}

Why don't you use unique serial numbers as the old code did?  This would save
you from doing this (I take it this is meant to ensure uniqueness of the
mangled symbol name).

>       }
>     else
> !     snprintf(mangled_name, GFC_MAX_SYMBOL_LEN, name);

strcpy()

> *** 2354,2361 ****
> --- 2365,2408 ----
>   
>   	      sym->as = as;
>   	      as = NULL;
> + 
>   	    }
>   
> + 	  sym->common_head = (void*)t;
> + 
> + 	  /* Check to see if the symbol is already in an equivalence group.
> + 	     If it is, set the other members as being in common.  */
> + 	  if (sym->attr.in_equivalence)
> + 	    {
> + 	      for (e1 = gfc_current_ns->equiv; e1; e1 = e1->next)
> + 	        {
> + 	          equiv_flag = false;
> + 	          for (e2 = e1; e2; e2 = e2->eq)
> + 	            if (e2->expr->symtree->n.sym == sym)
> + 		      {
> + 			equiv_flag = true;
> + 			break;
> + 		      }
> + 		    if (equiv_flag == false)
> + 		      continue;
> + 		    for (e2 = e1; e2; e2 = e2->eq)
> + 		      {
> + 			other = e2->expr->symtree->n.sym;
> + 			if (other->common_head
> + 			      && other->common_head != sym->common_head)
> + 			  {
> + 			    gfc_error ("Symbol '%s' at %C is being indirectly "
> + 				       "equivalenced to another COMMON block"
> + 				       , sym->name);
> + 			    goto cleanup;
> + 			  }

If the common head member were not a void* you could print the name of the
common block in the error message.  You could do away with the equiv_flag
variable if you put the thirde for loop inside if (e2->expr->symtree->n.sym ==
sym), but it may be that I only find this easier because I was mildly confused
because the indentation levels got confused in my mailer :-)

> *** 3258,3263 ****
> --- 3314,3364 ----
>     mio_rparen();
>   }
>   
> + /* Write the blank common block to the module */
> + 
> + static void
> + write_blank_common (void)
> + {
> +   const char * name = {BLANK_COMMON_NAME};

Um, why the braces?