Re: [Patch, fortran] PR26074, PR25103, PR20858, PR20861, PR20871, PR25083 & PR25088 - Joost's PRs

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

Paul Thomas wrote:
> Index: gcc/fortran/expr.c
> ===================================================================
> *** gcc/fortran/expr.c	(revision 110513)
> --- gcc/fortran/expr.c	(working copy)
> *************** gfc_check_pointer_assign (gfc_expr * lva
> *** 1980,1986 ****
>     /* If rvalue is a NULL() or NULLIFY, we're done. Otherwise the type,
>        kind, etc for lvalue and rvalue must match, and rvalue must be a
>        pure variable if we're in a pure function.  */
> !   if (rvalue->expr_type == EXPR_NULL)
>       return SUCCESS;
>   
>     if (!gfc_compare_types (&lvalue->ts, &rvalue->ts))
> --- 1980,1986 ----
>     /* If rvalue is a NULL() or NULLIFY, we're done. Otherwise the type,
>        kind, etc for lvalue and rvalue must match, and rvalue must be a
>        pure variable if we're in a pure function.  */
> !   if (rvalue->expr_type == EXPR_NULL && rvalue->ts.type <= BT_UNKNOWN)
>       return SUCCESS;
>   
>     if (!gfc_compare_types (&lvalue->ts, &rvalue->ts))

.LE.?

> *************** resolve_call (gfc_code * c)
> *** 1579,1584 ****
> --- 1593,1607 ----
>   {
>     try t;
>   
> +   if (c->symtree && c->symtree->n.sym
> + 	&& c->symtree->n.sym->ts.type > BT_UNKNOWN)

I'd strongly prefer !=

> *************** resolve_derived (gfc_symbol *sym)
> *** 4493,4500 ****

You have no ChangeLog entry for this hunk and the one to resolve_charlen.

> --- 4780,4860 ----
>     switch (sym->attr.flavor)
>       {
>       case FL_VARIABLE:
> !       /* The shape of a main program or module array needs to be constant.  */

To me it would be clearer if you said "the shape of an array in a main program
or a module needs to be constant."

> ! 	  /* Unfortunately, !gfc_is_compile_time_shape hits a legal
> ! 	     case that has not been simplified; parameter array
> ! 	     references.  Do the simplification now.  */
> ! 	  flag = 0;
> ! 	  for (i = 0; i < sym->as->rank; i++)
> ! 	    {
> ! 	      e = sym->as->lower[i];
> ! 	      if (e && (resolve_index_expr (e) == FAILURE
> ! 	          || !gfc_is_constant_expr (e)))
> ! 		{
> ! 		  flag = 1;
> ! 		  break;
> ! 		}
> ! 
> ! 	      e = sym->as->upper[i];
> ! 	      if (e && (resolve_index_expr (e) == FAILURE
> ! 	          || !gfc_is_constant_expr (e)))
> ! 		{
> ! 		  flag = 1;
> ! 		  break;
> ! 		}
> ! 	    }

Maybe isolate this into its own function, keeping resolve_symbol more of an
outline of what its doing so that one can easily find which path it should
proceed through?

> !       /* Ensure that derived type formal arguments of a public procedure
> ! 	 are not of a private type.  */
>         if (gfc_check_access(sym->attr.access, sym->ns->default_access))
>   	{
> ! 	  for (arg = sym->formal; arg; arg = arg->next)
>   	    {
> ! 	      if (arg->sym
> ! 		    && arg->sym->ts.type == BT_DERIVED
> ! 		    && !arg->sym->ts.derived->attr.use_assoc
> ! 		    && !gfc_check_access(arg->sym->ts.derived->attr.access,
> ! 				arg->sym->ts.derived->ns->default_access))
> ! 		{
> ! 		  gfc_error_now ("'%s' is a PRIVATE type and cannot be "
                                       ^^^^^^^^^^^^^^^^^
> ! 			         "a dummy argument of '%s', which is "
> ! 				 "PUBLIC at %L", arg->sym->name, sym->name,
                                                 ^^^^^^^^^^^^^^
Make that 'is of a PRIVATE type', or replace arg->sym->name by
arg->ts.derived->name.  I think the former would be preferable.

> *************** check_data_variable (gfc_data_variable *
> *** 5027,5032 ****
> --- 5112,5124 ----
>     if (e->expr_type != EXPR_VARIABLE)
>       gfc_internal_error ("check_data_variable(): Bad expression");
>   
> +   if (e->symtree->n.sym->ns->is_block_data
> + 	&& !e->symtree->n.sym->attr.in_common)
> +     {
> +       gfc_error ("BLOCK DATA element '%s' at %L must be in COMMON",
> + 	         e->symtree->n.sym->name, &e->symtree->n.sym->declared_at);
> +     }
> + 

Probably not the right place, or do all variables in block data come this way?

Thanks,
- Tobi

:REVIEWMAIl: