Hello,
Le 09/03/2014 13:59, [email protected] 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
