| Message ID | CAKwh3qg9ydJPtWvU3c+oEuOkad5xdTTMJFs3aniqKkLP_1A0wQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 03, 2011 at 10:56:47PM +0100, Janus Weil wrote: > > At least add a comment about the re-use (abuse?) of the > > enum. > > Updated patch attached, which adds a short comment on the usage of 'match'. Thanks. > > This should reduce confusion months from when > > someone wonders why gfc_extend_expr returns a "match" > > for a non-matching function. > > Well, I think my approach is not as far-fetched as you seem to imply: > There are already a good number of procedures which use the 'match' > enum, although they're not related to matching at all. Listing only > those that occur in gfortran.h (I'm sure there are more): > > * match gfc_mod_pointee_as (gfc_array_spec *); > * match gfc_intrinsic_func_interface (gfc_expr *, int); > * match gfc_intrinsic_sub_interface (gfc_code *, int); > * match gfc_iso_c_sub_interface(gfc_code *, gfc_symbol *); > > The reason for this is of course that the YES/NO/ERROR triple is not > only useful in matching, but also in many other situations. > I think the patch is fine and can be committed. But, give Steven a chance to respond before committing.
Sounds like what everything needs is a differently named enum: say three_way_logic. On Nov 3, 2011, at 3:56 PM, Janus Weil wrote: >> At least add a comment about the re-use (abuse?) of the >> enum. > > Updated patch attached, which adds a short comment on the usage of 'match'. > > >> This should reduce confusion months from when >> someone wonders why gfc_extend_expr returns a "match" >> for a non-matching function. > > Well, I think my approach is not as far-fetched as you seem to imply: > There are already a good number of procedures which use the 'match' > enum, although they're not related to matching at all. Listing only > those that occur in gfortran.h (I'm sure there are more): > > * match gfc_mod_pointee_as (gfc_array_spec *); > * match gfc_intrinsic_func_interface (gfc_expr *, int); > * match gfc_intrinsic_sub_interface (gfc_code *, int); > * match gfc_iso_c_sub_interface(gfc_code *, gfc_symbol *); > > The reason for this is of course that the YES/NO/ERROR triple is not > only useful in matching, but also in many other situations. > > Cheers, > Janus > <gfc_extend_expr_v2.diff>
> Sounds like what everything needs is a differently named enum: say three_way_logic. Well, one might just rename the present enum 'match' (which anyway lacks the usual 'gfc' prefix), to something like 'gfc_three_way_logic' (or whatever name you prefer), with appropriately named values. Then one could apply it in a more general setting without causing confusion. But that would be a *huge* patch (though completely mechanical), and I'm not sure if I would be willing to waste my time writing a ChangeLog for that (unless there is an automated way for this by now?!?). Cheers, Janus > On Nov 3, 2011, at 3:56 PM, Janus Weil wrote: > >>> At least add a comment about the re-use (abuse?) of the >>> enum. >> >> Updated patch attached, which adds a short comment on the usage of 'match'. >> >> >>> This should reduce confusion months from when >>> someone wonders why gfc_extend_expr returns a "match" >>> for a non-matching function. >> >> Well, I think my approach is not as far-fetched as you seem to imply: >> There are already a good number of procedures which use the 'match' >> enum, although they're not related to matching at all. Listing only >> those that occur in gfortran.h (I'm sure there are more): >> >> * match gfc_mod_pointee_as (gfc_array_spec *); >> * match gfc_intrinsic_func_interface (gfc_expr *, int); >> * match gfc_intrinsic_sub_interface (gfc_code *, int); >> * match gfc_iso_c_sub_interface(gfc_code *, gfc_symbol *); >> >> The reason for this is of course that the YES/NO/ERROR triple is not >> only useful in matching, but also in many other situations. >> >> Cheers, >> Janus >> <gfc_extend_expr_v2.diff> > >
2011/11/3 Steve Kargl <sgk@troutmask.apl.washington.edu>: > On Thu, Nov 03, 2011 at 10:56:47PM +0100, Janus Weil wrote: >> > At least add a comment about the re-use (abuse?) of the >> > enum. >> >> Updated patch attached, which adds a short comment on the usage of 'match'. > > Thanks. > >> > This should reduce confusion months from when >> > someone wonders why gfc_extend_expr returns a "match" >> > for a non-matching function. >> >> Well, I think my approach is not as far-fetched as you seem to imply: >> There are already a good number of procedures which use the 'match' >> enum, although they're not related to matching at all. Listing only >> those that occur in gfortran.h (I'm sure there are more): >> >> * match gfc_mod_pointee_as (gfc_array_spec *); >> * match gfc_intrinsic_func_interface (gfc_expr *, int); >> * match gfc_intrinsic_sub_interface (gfc_code *, int); >> * match gfc_iso_c_sub_interface(gfc_code *, gfc_symbol *); >> >> The reason for this is of course that the YES/NO/ERROR triple is not >> only useful in matching, but also in many other situations. >> > > I think the patch is fine and can be committed. But, give > Steven a chance to respond before committing. Thanks, Steve. I think three days should be long enough. Will commit later today (if no one protests in the meantime). Cheers, Janus
>> I think the patch is fine and can be committed. But, give >> Steven a chance to respond before committing. > > Thanks, Steve. I think three days should be long enough. Will commit > later today (if no one protests in the meantime). Committed as r181044. Cheers, Janus
Index: gcc/fortran/interface.c =================================================================== --- gcc/fortran/interface.c (revision 180820) +++ gcc/fortran/interface.c (working copy) @@ -3220,12 +3220,11 @@ build_compcall_for_operator (gfc_expr* e, gfc_actu with the operator. This subroutine builds an actual argument list corresponding to the operands, then searches for a compatible interface. If one is found, the expression node is replaced with - the appropriate function call. - real_error is an additional output argument that specifies if FAILURE - is because of some real error and not because no match was found. */ + the appropriate function call. We use the 'match' enum to specify + whether a replacement has been made or not, or if an error occurred. */ -gfc_try -gfc_extend_expr (gfc_expr *e, bool *real_error) +match +gfc_extend_expr (gfc_expr *e) { gfc_actual_arglist *actual; gfc_symbol *sym; @@ -3239,7 +3238,6 @@ build_compcall_for_operator (gfc_expr* e, gfc_actu actual = gfc_get_actual_arglist (); actual->expr = e->value.op.op1; - *real_error = false; gname = NULL; if (e->value.op.op2 != NULL) @@ -3343,16 +3341,16 @@ build_compcall_for_operator (gfc_expr* e, gfc_actu result = gfc_resolve_expr (e); if (result == FAILURE) - *real_error = true; + return MATCH_ERROR; - return result; + return MATCH_YES; } /* Don't use gfc_free_actual_arglist(). */ free (actual->next); free (actual); - return FAILURE; + return MATCH_NO; } /* Change the expression node to a function call. */ @@ -3365,12 +3363,9 @@ build_compcall_for_operator (gfc_expr* e, gfc_actu e->user_operator = 1; if (gfc_resolve_expr (e) == FAILURE) - { - *real_error = true; - return FAILURE; - } + return MATCH_ERROR; - return SUCCESS; + return MATCH_YES; } Index: gcc/fortran/gfortran.h =================================================================== --- gcc/fortran/gfortran.h (revision 180820) +++ gcc/fortran/gfortran.h (working copy) @@ -2831,7 +2831,7 @@ void gfc_procedure_use (gfc_symbol *, gfc_actual_a void gfc_ppc_use (gfc_component *, gfc_actual_arglist **, locus *); gfc_symbol *gfc_search_interface (gfc_interface *, int, gfc_actual_arglist **); -gfc_try gfc_extend_expr (gfc_expr *, bool *); +match gfc_extend_expr (gfc_expr *); void gfc_free_formal_arglist (gfc_formal_arglist *); gfc_try gfc_extend_assign (gfc_code *, gfc_namespace *); gfc_try gfc_add_interface (gfc_symbol *); Index: gcc/fortran/resolve.c =================================================================== --- gcc/fortran/resolve.c (revision 180820) +++ gcc/fortran/resolve.c (working copy) @@ -4034,11 +4034,10 @@ resolve_operator (gfc_expr *e) bad_op: { - bool real_error; - if (gfc_extend_expr (e, &real_error) == SUCCESS) + match m = gfc_extend_expr (e); + if (m == MATCH_YES) return SUCCESS; - - if (real_error) + if (m == MATCH_ERROR) return FAILURE; }