Re: [Patch, fortran] PR40054 and PR63921 - Implement pointer function assignment - redux

[Mikael Morin <mikael dot morin at sfr dot fr>]

Hello Paul,

Le 25/09/2015 14:21, Paul Richard Thomas a Ãcrit :
> Dear Mikael, dear all,
>
> Please find attached a revised version of the patch that, I believe,
> addresses all the comments. The patch is very much improved and these
> improvements are verified by the two extra testcases. Thanks a
> million!
>
> Most of the effort involved in preparing this revised patch was
> associated with getting rid of ICEs/segfaults triggered by error
> recovery. The error handling in resolve_ptr_fcn_assign is still a bit
> clumsy but its behaviour is more consistent.
>
> Bootstraps and regtests on FC21/x86_64 - OK for trunk?
>
> Cheers
>
> Paul
>
> 2015-09-25  Paul Thomas  <pault@gcc.gnu.org>
>
>      * decl.c (get_proc_name): Return if statement function is
>      found.
>      * expr.c (gfc_check_vardef_context): Add error return for
>      derived type expression lacking the derived type itself.
>      * io.c (next_char_not_space): Change tab warning to warning now
>      to prevent locus being lost.
This has disappeared?

>      * match.c (gfc_match_ptr_fcn_assign): New function.
>      * match.h : Add prototype for gfc_match_ptr_fcn_assign.
>      * parse.c : Add static flag 'in_specification_block'.
>      (decode_statement): If in specification block match a statement
>      function, then, if standard embraces F2008 and no error arising
>      from statement function matching, try to match pointer function
>      assignment.
>      (parse_interface): Set 'in_specification_block' on exiting from
>      parse_spec.
>      (parse_spec): Set and then reset 'in_specification_block'.
>      (gfc_parse_file): Set 'in_specification_block'.
>      * resolve.c (get_temp_from_expr): Extend to include functions
>      and array constructors as rvalues..
>      (resolve_ptr_fcn_assign): New function.
>      (gfc_resolve_code): Call it on finding a pointer function as an
>      lvalue. If valid or on error, go back to start of resolve_code.
>      * symbol.c (gfc_add_procedure): Add a sentence to the error to
>      flag up the ambiguity between a statement function and pointer
>      function assignment at the end of the specification block.

> Index: gcc/fortran/parse.c
> ===================================================================
> *** gcc/fortran/parse.c	(revision 227854)
> --- gcc/fortran/parse.c	(working copy)

> *************** decode_statement (void)
> *** 356,362 ****
>
>     match (NULL, gfc_match_assignment, ST_ASSIGNMENT);
>     match (NULL, gfc_match_pointer_assignment, ST_POINTER_ASSIGNMENT);
> !   match (NULL, gfc_match_st_function, ST_STATEMENT_FUNCTION);
>
>     match (NULL, gfc_match_data_decl, ST_DATA_DECL);
>     match (NULL, gfc_match_enumerator_def, ST_ENUMERATOR);
> --- 357,375 ----
>
>     match (NULL, gfc_match_assignment, ST_ASSIGNMENT);
>     match (NULL, gfc_match_pointer_assignment, ST_POINTER_ASSIGNMENT);
> !
> !   if (in_specification_block)
> !     {
> !       m = match_word (NULL, gfc_match_st_function, &old_locus);
> !       if (m == MATCH_YES)
> ! 	return ST_STATEMENT_FUNCTION;
> !     }
> !
> !   if (!(in_specification_block && m == MATCH_ERROR)
> !       && !gfc_notification_std (GFC_STD_F2008))
> !     {
> !       match (NULL, gfc_match_ptr_fcn_assign, ST_ASSIGNMENT);
> !     }
I think that for better error reporting (avoid unclassifiable 
statement), the gfc_notification_std can be dropped, as there is a 
specific gfc_notify_std guarding resolution.

Same for the rest of the condition.  gfc_match_ptr_fcn_assign carrefully 
restores existing errors upon failure, so I would rather use it more often.

So, can you try removing the condition completely (and use the match 
macro above again)?  that should improve errors in ptr_func_assign_2, 
and hopefully not regress.
If it does regress, let's keep it as is.



> Index: gcc/fortran/resolve.c
> ===================================================================
> *** gcc/fortran/resolve.c	(revision 227854)
> --- gcc/fortran/resolve.c	(working copy)
> *************** generate_component_assignments (gfc_code
> *** 10133,10138 ****
> --- 10141,10205 ----
>   }
>
>
> + /* F2008: Pointer function assignments are of the form:
> + 	ptr_fcn (args) = expr
> +    This function breaks these assignments into two statements:
> + 	temporary_pointer => ptr_fcn(args)
> + 	temporary_pointer = expr  */
> +
> + static bool
> + resolve_ptr_fcn_assign (gfc_code **code, gfc_namespace *ns)
> + {
> +   gfc_expr *tmp_ptr_expr;
> +   gfc_code *this_code;
> +   gfc_component *comp;
> +   gfc_symbol *s;
> +
> +   if ((*code)->expr1->expr_type != EXPR_FUNCTION)
> +     return false;
> +
> +   /* Even if standard does not support this feature, continue to build
> +      the two statements to avoid upsetting frontend_passes.c.  */
> +   gfc_notify_std (GFC_STD_F2008, "Pointer procedure assignment at "
> + 		  "%L", &(*code)->loc);
> +
> +   comp = gfc_get_proc_ptr_comp ((*code)->expr1);
> +
> +   if (comp)
> +     s = comp->ts.interface;
> +   else
> +     s = (*code)->expr1->symtree->n.sym;
> +
> +   if (s == NULL || !s->result->attr.pointer)
> +     {
> +       gfc_error ("F2008: The function result at %L must have "
> + 		 "the pointer attribute.", &(*code)->expr1->where);
Nit: Usually, we don't put the 'F2008:' prefix.
Also may be explicit a bit more: "function result as assigned-to 
variable" or something alike.

Anyway, those are nits, and the rest looks good to me.
So, with the above comments, the patch is OK as far as I'm concerned.
Thanks

Mikael