Hi Tobias, thanks for the review!
>> here is a patch for a rather old PR, which deals with correctness >> checking for several cases, such as: >> 1) dummy procedures >> 2) proc-ptr assignments >> 3) type-bound procedure overloading >> >> The patch adds a new function 'check_result_characteristics' to do >> various checks on function results. This is largely analogous to >> 'check_dummy_characteristics'. In both of them, there are still a few >> attributes left to check, which I may add in a follow-up patch. Also I >> had to disable the warning for cases where we can not finally >> determine whether the string length or shape expressions are equal. >> The treatment for those cases should be improved at some point, or one >> should think about re-enabling the warnings. >> >> Regtested on x86_64-unknown-linux-gnu. Ok for trunk? > > > Thanks for the patch. However, I wonder about: > > + /* Check ALLOCATABLE attribute. */ > + if (r1->attr.allocatable != r2->attr.allocatable) > + { > + snprintf (errmsg, err_len, "ALLOCATABLE mismatch in function > result"); > + return FAILURE; > + } > > Shouldn't one check whether errmsg != NULL? That should only occur with > generic resolution, but my impression is that that code is reached. I have to admit that I did not properly think about this point. Now that I do, I actually conclude that 'check_result_characteristics' is apparently never called without errmsg. 'gfc_compare_interfaces' only calls it when 'strict_flag' is set, and it is never called with strict_flag and without errmsg. > Additionally, I find "ALLOCATABLE mismatch" a bit unclear and would prefer > something like "ALLOCATABLE attribute mismatch". Ok, I have done so for this one and the other attribute checks. > Other items to check for: CONTIGUOUS, type-length parameter (string length), > procedure-pointer results. The string length checking I already had, and the other two I added now. > For strings, one can have deferred ("len=:", > ts.deferred) and explicit size (len=n, len=3 etc.). The deferred checking was still missing for the string length, but I have added it now. > With those changes, I am fine with the patch. Thanks. I will commit the attached version after another regtest - unless there are further complaints in the meantime. [Your further remarks I will ignore for now. For some of them, one should maybe open a PR to keep track.] Cheers, Janus > PS: Some remarks, triggered by your patch but only remotely related. Though, > I wouldn't mind if you (or someone else) could work on one of those ;-) > > Side remark 1: Talking about generic resolution: I think we should at some > point implement diagnostic for generic calls as well. If the interface > matches (according to ambiguity rules), it makes sense to also print an > error if there is a mismatch. Currently, gfortran just prints that no > specific proc to the a generic name has been found, which requires extensive > digging until one find the "correct" specific function and sees that, e.g., > the dummy procedure has an argument with intent(inout) and the one of the > acutal > argument has no intent specified. [PR40276.] > > Side remark 1a: F2008 has relaxed some ambiguity rules; at some point we > have to implement them. (PR 45521) > > Side remark 2: I think we should split the type-rank mismatch diagnostic and > print the gfc_typename() and, respectively, rank in the error message; one > probably should pass on the errmsg as one also needs to check the kind and > not only the type. > > (I recently run into the issues of side remark 1 and 2: First the > no-specific-proc-found error; then, calling the specific function, the > type/rank error; it took us a while to see that the type was okay about not > the rank.) > > Side remark 3: I probably introduced a bug with my recent > assumed-type/assumed-rank patches: It regards assumed-type/assumed-rank > compatible to any other type/rank. That makes sense for checking dummy > arguments - but not to check the interface of dummy procedures. For > assumed-rank, an as->type check might catch it (untested), but I fear that > type(*) is mishandled. > > Side remark 4: The following program ICEs in check_assumed_size_reference's > 1386 if ((e->ref->u.ar.end[e->ref->u.ar.as->rank - 1] == NULL) > as e->ref == NULL. > > implicit none > contains > function g() > integer :: g(*) > end function g > > subroutine test() > procedure(g), pointer :: x > x => g > end subroutine test > end > > >> 2012-08-05 Janus Weil <ja...@gcc.gnu.org> >> >> PR fortran/35831 >> * interface.c (check_result_characteristics): New function, which >> checks >> the characteristics of function results. >> (gfc_compare_interfaces,gfc_check_typebound_override): Call it. >> >> 2012-08-05 Janus Weil <ja...@gcc.gnu.org> >> >> PR fortran/35831 >> * gfortran.dg/dummy_procedure_5.f90: Modified. >> * gfortran.dg/dummy_procedure_8.f90: New. >> * gfortran.dg/interface_26.f90: Modified. >> * gfortran.dg/proc_ptr_11.f90: Modified. >> * gfortran.dg/proc_ptr_15.f90: Modified. >> * gfortran.dg/proc_ptr_result_5.f90: Modified. >> * gfortran.dg/typebound_override_1.f90: Modified. >> * gfortran.dg/typebound_proc_6.f03: Modified.
pr35831_v2.diff
Description: Binary data
dummy_procedure_8.f90
Description: Binary data