Re: [fortran, patch] Fix conflict detesction for VOLATILE attribute (PR30520)

[Brooks Moses <brooks dot moses at codesourcery dot com>]

A few comments on this, mostly on wording in the comments and error 
messages:

Tobias Burnus wrote:
> +       /* C1232 (R1221) For an actual argument which is an array sections or
> + 	 an assumed-shaped array, the dummy argument shall be an assumed-
> + 	 shaped array, if the dummy argument has the VOLATILE attribute.  */

s/sections/section/

> + 
> +       if (f->sym->attr.volatile_
> + 	  && ((a->expr->symtree->n.sym->as
> + 	       && a->expr->symtree->n.sym->as->type == AS_ASSUMED_SHAPE)
> + 	      ||(a->expr->ref && a->expr->ref->u.ar.type == AR_SECTION))
> +           && !(f->sym->as && f->sym->as->type == AS_ASSUMED_SHAPE))
> + 	{
> + 	  if (where)
> + 	    gfc_error ("Assumed-shaped or array-section actual argument at "
> + 		       "%L is incompatible with the non-assumed-shaped "
> + 		       "dummy argument '%s' due to VOLATILE attribute",
> + 		       &a->expr->where,f->sym->name);
> + 	  return 0;
> + 	}

This should be "assumed-shape", not "assumed-shaped", throughout.

I think this might also be somewhat clearer, both in the code and in the 
error message, if it was broken apart into one check for an 
assumed-shape actual argument and a second check for an array-section 
actual argument.  I was having a fairly hard time following the logic in 
the "if" statement.

> +       /* C1233 (R1221) For an actual argument which is a pointer array, the
> + 	 dummy argument shall be an assumed-shaped or pointer array, if the
> + 	 dummy argument has the VOLATILE attribute.  */
> + 
> +       if (f->sym->attr.volatile_
> + 	  && (a->expr->symtree->n.sym->attr.pointer
> + 	      && a->expr->symtree->n.sym->as)
> +           && !(f->sym->as && (f->sym->as->type == AS_ASSUMED_SHAPE
> +                || f->sym->attr.pointer)))

Either your indentation or your parentheses are incorrect here, on those 
last two lines; the "||" currently goes with the grouping after 
"f->sym->as &&".

Also, it looks like you've got some problems with spaces in place of 
tabs in some of those indents, both here and in the previous piece.

> + 	{
> +        	  if (where)
> + 	    gfc_error ("Pointer-array actual argument at %L is incompatible "
> + 		       "with the neither assumed-shaped nor pointer array "
> + 		       "dummy argument '%s' due to VOLATILE attribute",
> + 		       &a->expr->where,f->sym->name);
> + 	  return 0;
> + 	}

I think I would suggest wording that as "Pointer-array actual argument 
at %L requires an assumed-shape or pointer-array dummy argument".

[...]
> --- 877,890 ----
>   gfc_add_volatile (symbol_attribute * attr, const char *name, locus * where)
>   {
>   
> !   /* No check_used needed as 11.2.1 of the F2003 standard allows
> !      that the local identifier, made accessible by a use statment, can be
> !      given a VOLATILE attribute.  */

s/accessible/accessable/
s/statment/statement/

Also, I think that would be better without the commas.

[...]
> --- 4221,4229 ----
>   
>     for(;;)
>       {
> !       /* VOLATILE is special by allowing to locally overwrite the attribute
> !          of host-associated symbols.  */

This isn't overwriting, so much as adding -- it can't be subtracted. 
Maybe: "VOLATILE is special because it can be added to host-associated 
symbols locally"?

Other than that, this all looks OK to me, but I'm not a maintainer.

- Brooks