[Patch, Fortran] Add parsing support for assumed-rank array

[Mikael Morin]

On 24.06.2012 17:34, Tobias Burnus wrote:
> Tobias Burnus wrote:
>> To cleanup my local trees; I had the patch lingering there for a many
>> weeks. User visible, it only adds parsing support for "dimension(..)"
>> and a sorry message.
>
> I have now updated the patch. Changes:
>

Hello,

some commen^Wbike shedding below. Overall it looks good.
I may have missed the point about the way you handle diagnostics.

Mikael

> diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
> index 7d505d5..b0c4b28 100644
> --- a/gcc/fortran/check.c
> +++ b/gcc/fortran/check.c
> @@ -619,6 +619,10 @@ dim_rank_check (gfc_expr *dim, gfc_expr *array, int allow_assumed)
>    else
>      rank = array->rank;
>
> +  /* Assumed-rank array.  */
> +  if (rank == -1)
> +    rank = GFC_MAX_DIMENSIONS;
> +
I think the  assumed-rank => rank == -1  convention should be documented 
in gfortran.h, at least for the gfc_array_spec::rank field.




> @@ -2990,6 +3008,15 @@ gfc_procedure_use (gfc_symbol *sym, gfc_actual_arglist **ap, locus *where)
>  	      gfc_error ("MOLD argument to NULL required at %L", &a->expr->where);
>  	      return;
>  	    }
> +
> +	  /* TS 29113, C407b.  */
> +	  if (a->expr && a->expr->expr_type == EXPR_VARIABLE
> +	      && symbol_rank (a->expr->symtree->n.sym) == -1)
> +	    {
> +	      gfc_error ("Assumed-rank argument requires an explicit interface "
> +			 "at %L", &a->expr->where);
> +	      return;
> +	    }
>  	}
>
>        return;

Doesn't this duplicates the other explicit interface diagnostic below...


> @@ -2194,6 +2206,15 @@ resolve_global_procedure (gfc_symbol *sym, locus *where,
>  			   sym->name, &sym->declared_at, arg->sym->name);
>  		break;
>  	      }
> +	    /* TS 29113, 6.2.  */
> +	    else if (arg->sym && arg->sym->as
> +		     && arg->sym->as->type == AS_ASSUMED_RANK)
> +	      {
> +		gfc_error ("Procedure '%s' at %L with assumed-rank dummy "
> +			   "argument '%s' must have an explicit interface",
> +			   sym->name, &sym->declared_at, arg->sym->name);
> +		break;
> +	      }
>  	    /* F2008, 12.4.2.2 (2c)  */
>  	    else if (arg->sym->attr.codimension)
>  	      {

... here?





> @@ -5067,13 +5097,26 @@ resolve_variable (gfc_expr *e)
>    sym = e->symtree->n.sym;
>
>    /* TS 29113, 407b.  */
> -  if (e->ts.type == BT_ASSUMED && !assumed_type_expr_allowed)
> +  if (e->ts.type == BT_ASSUMED && !assumed_rank_type_expr_allowed)
>      {
>        gfc_error ("Invalid expression with assumed-type variable %s at %L",
>  		 sym->name, &e->where);
>        return FAILURE;
>      }

I'm not sure I understand the logic with the mixed assumed rank/type 
flag. According to C407c, shouldn't we check that e is assumed rank/shape?


>
> +  /* TS 29113, C535b.  */
> +  if (((sym->ts.type == BT_CLASS && sym->attr.class_ok
> +	&& CLASS_DATA (sym)->as
> +	&& CLASS_DATA (sym)->as->type == AS_ASSUMED_RANK)
> +       || (sym->ts.type != BT_CLASS && sym->as
> +	   && sym->as->type == AS_ASSUMED_RANK))
> +      && !assumed_rank_type_expr_allowed)
> +    {
> +      gfc_error ("Invalid expression with assumed-rank variable %s at %L",
> +		 sym->name, &e->where);

The error message could be made more helpful. ;-)


> @@ -5084,6 +5127,22 @@ resolve_variable (gfc_expr *e)
>        return FAILURE;
>      }
>
> +  /* TS 29113, C535b.  */
> +  if (((sym->ts.type == BT_CLASS && sym->attr.class_ok
> +	&& CLASS_DATA (sym)->as
> +	&& CLASS_DATA (sym)->as->type == AS_ASSUMED_RANK)
> +       || (sym->ts.type != BT_CLASS && sym->as
> +	   && sym->as->type == AS_ASSUMED_RANK))
> +      && e->ref
> +      && !(e->ref->type == REF_ARRAY && e->ref->u.ar.type == AR_FULL
> +           && e->ref->next == NULL))
> +    {
> +      gfc_error ("Assumed-rank variable %s with designator at %L",
> +                 sym->name, &e->ref->u.ar.where);

Ditto here. And I think that C535b is more about the context of the 
expression rather than the expression itself.





> diff --git a/gcc/fortran/trans-array.c b/gcc/fortran/trans-array.c
> index f135af1..6c58a8e 100644
> --- a/gcc/fortran/trans-array.c
> +++ b/gcc/fortran/trans-array.c
> @@ -8319,12 +8323,15 @@ gfc_walk_array_ref (gfc_ss * ss, gfc_expr * expr, gfc_ref * ref)
>  	  break;
>
>  	case AR_FULL:
> -	  newss = gfc_get_array_ss (ss, expr, ar->as->rank, GFC_SS_SECTION);
> +	  newss = gfc_get_array_ss (ss, expr,
> +				    ar->as->rank < 0 ? GFC_MAX_DIMENSIONS
> +						     : ar->as->rank,
> +				    GFC_SS_SECTION);
>  	  newss->info->data.array.ref = ref;
>
>  	  /* Make sure array is the same as array(:,:), this way
>  	     we don't need to special case all the time.  */
> -	  ar->dimen = ar->as->rank;
> +	  ar->dimen = ar->as->rank < 0 ? GFC_MAX_DIMENSIONS : ar->as->rank;
>  	  for (n = 0; n < ar->dimen; n++)
>  	    {
>  	      ar->dimen_type[n] = DIMEN_RANGE;

I would rather avoid that if possible.
The scalarizer assumes the rank is known, and all hell breaks loose if 
it's not the case.
After quickly browsing through TS29113, I couldn't tell whether 
expressions like (ar + 1) would be valid as assumed rank actual argument.
In case it is, gfc_conv_expr_descriptor won't work correctly, as it will 
hardcode exactly GFC_MAX_DIMENSIONS loops to set the temporary, 
accessing the array descriptor's fields (i.e. bounds, etc) beyond the 
maximal dimension.
In case it's not, then everything is fine I guess, though I prefer 
avoiding polluting the scalarizer with assumed rank stuff ;-).