Hello,

Le 09/03/2014 13:59, jimmie.da...@l-3com.com a écrit :
> 
> 
> The code would only remove a variable from a common block if it was just 
> defined in the previous statement. The PR showed a case where a pre-existing 
> variable was put in the common block.  When it was not removed, the common 
> block list was wrong and segfaulted (or infinite looped) when used later on.
> 
> I changed it so it would remove a variable from a common block if it was just 
> defined, or if it previously existed and was put in the common block in the 
> last statement.
> 
> This patch works with the example given in the PR and has no regressions in 
> the testsuite.    
> 
> tested i686/linux.
> 
> 
> --bud davis
> 
[...]
> Index: gcc/gcc/fortran/symbol.c
> ===================================================================
> --- gcc/gcc/fortran/symbol.c  (revision 208437)
> +++ gcc/gcc/fortran/symbol.c  (working copy)
> @@ -3069,9 +3069,9 @@
>  
>    FOR_EACH_VEC_ELT (latest_undo_chgset->syms, i, p)
>      {
> -      if (p->gfc_new)
> +      if (p->gfc_new || (p->attr.in_common && 
> !p->old_symbol->attr.in_common) )
>       {
> -       /* Symbol was new.  */
> +       /* Symbol was new. Or was old and just put in common */
>         if (p->attr.in_common && p->common_block && p->common_block->head)
Please merge the two ifs; it seems they have exactly the same scope
after the patch.

>           {
>             /* If the symbol was added to any common block, it
> @@ -3115,6 +3115,9 @@
>         /* The derived type is saved in the symtree with the first
>            letter capitalized; the all lower-case version to the
>            derived type contains its associated generic function.  */
This comment applies to the TOUPPER thing below...

> +         }
> +         if (p->gfc_new) 
> +         {
... so it should be put here. Also the indentation is wrong.

>         if (p->attr.flavor == FL_DERIVED)
>           gfc_delete_symtree (&p->ns->sym_root, gfc_get_string ("%c%s",
>                          (char) TOUPPER ((unsigned char) p->name[0]),
> @@ -3125,7 +3128,9 @@
>         gfc_release_symbol (p);
>       }
>        else
> -     restore_old_symbol (p);
> +        {
> +       restore_old_symbol (p);
> +        }
>      }
This is unnecessary.

Otherwise looks goodish.  This is not a regression, but I suppose it
will be acceptable.

Mikael

Reply via email to