Am 06.10.19 um 17:26 schrieb Thomas Koenig:
> This > also restores Lapack compilation without warning. Well, up to an error in the testing routines, at least. TESTING/LIN/sdrvls.f has REAL, ALLOCATABLE :: WORK (:) ... REAL RESULT( NTESTS ), WQ and calls CALL SGELS( TRANS, M, N, NRHS, A, LDA, $ B, LDB, WQ, -1, INFO ) [...] CALL SGELS( TRANS, M, N, NRHS, A, LDA, B, $ LDB, WORK, LWORK, INFO ) so that one really is illegal and should be flagged. |
Hi Tobias,
> function ("o" missing); I think it is not clause 14 but paragraph 14. Fixed. (That one was easy :-) >> + warning for this can be suppressed later. */ >> + >> +bool >> +maybe_dummy_array_arg (gfc_expr *e) >> +{ >> + gfc_symbol *s; >> + >> + if (e->rank > 0) >> + return false; >> + >> + if (e->ts.type == BT_CHARACTER && e->ts.kind == 1) >> + return true; >> + >> + if (e->expr_type != EXPR_VARIABLE) >> + return false; > > What about PARAMETER? :-) I found that, by the time the code is reached, an element of a parameter array is already simplified; so I added a flag during constructor expansion. > >> + s = e->symtree->n.sym; >> + if (s->as == NULL) >> + return false; > > This looks wrong. You also want to permit dt%array(1) – but not > dt(1)%scalar Fixed. >> + if (s->ts.type == BT_CLASS || s->as->type == AS_ASSUMED_SHAPE >> + || s->attr.pointer) >> + return false; > > dt%foo – again, "foo" can be an allocatable of polymorphic type or a > pointer, but at least, it cannot be of assumed shape. Really? The paragraph reads # 14 If the actual argument is a noncoindexed scalar, the corresponding # dummy argument shall be scalar unless # * the actual argument is default character, of type character with the # C character kind (18.2.2), or is an element or substring of an # element of an array that is not an assumed-shape, pointer, or # polymorphic array, (The last two points do not apply here because they are invalid without explicit interface). Unless I have my negatives wrong, the code is correct (but I have been getting standardese wrong before). Anyway, here's an update of the patch. OK, or is there still something missing? Or how should I interpret that paragraph? :-) Regards Thomas |
Hi Thomas,
On 10/10/19 12:23 AM, Thomas Koenig wrote: >>> + if (s->ts.type == BT_CLASS || s->as->type == AS_ASSUMED_SHAPE >>> + || s->attr.pointer) >>> + return false; >> >> dt%foo – again, "foo" can be an allocatable of polymorphic type or a >> pointer, but at least, it cannot be of assumed shape. > > Really? The paragraph reads […] What I meant is assumed-shape implies dummy argument. Hence, "s->as->type" is a good check. Whereas for deferred-shape, one had to take care of "dt%allocatable_arg" – thus, the s->attr.pointer and the s->ts.type check aren't good. Technical background for those requirements: pointers and assumed-shape arrays can have strides, but if one passes a scalar to an array dummy argument, one wants to be reasonably sure that the memory is contiguous. (Actually, one could permit assumed-shape or pointer with contiguous argument. But as one doesn't want to encourage this abuse. The reason for permitting character(kind=1) is to call C "char*" functions without using ["H", "e", "l", "l", "o", null] instead of "Hello" + null].) > Anyway, here's an update of the patch. OK, or is there still something > missing? It would be nice to have a ChangeLog item (not as diff). > + /* Set if an interface to a procedure could actually be to an array > + although the actual argument is scalar. */ > + unsigned maybe_array:1; Actually, I find this sentence hard to parse. Maybe: "Set if the dummy argument of a procedure could be an array despite being called with a scalar actual argument." Or something along this line. > +/* Under certain conditions, a scalar actual argument can be passed > + to an array dummy argument - see F2018, 15.5.2.4, clause 14. This > + functin returns true for these conditions so that an error or Old patch? Still "functin". > + warning for this can be suppressed later. */ > + > +bool > +maybe_dummy_array_arg (gfc_expr *e) > +{ > + gfc_symbol *s; > + gfc_ref *ref; > + bool last_array_ref; > + > + if (e->rank > 0) > + return false; Maybe add a comment "/* Return false as for arrays, the rank always needs to be checked. */" or something like that. Otherweise, "maybe_dummy_array_arg" + description above the function cause one to stumble over this. > + s = e->symtree->n.sym; > + if (s->as == NULL) > + return false; Again, assume "call foo(dt%array(1))" – I think that's fine but rejected by this check as "dt" is a scalar and only "dt%array" is an array. – You have have to keep that array spec and then look the the last component reference and see at its array spec. > + if (s->ts.type == BT_CLASS || s->as->type == AS_ASSUMED_SHAPE > + || s->attr.pointer) > + return false; Similarly, "class%int_array(1)" is fine – I think you need "e->ts.type" instead of "s". For s->attr.pointer, likewise "ptr%int_array(1)" is fine, hence, "gfc_expr_attr (e).pointer" or something like that is needed. And for the "s->as->type", the following should be valid: type t integer :: ia(100) end type t type(t), allocatable :: x(:) allocate(x(1)) call foo(x(1)%ia(5), 100-5) But while x is assumed-shape > + last_array_ref = false; > + > + for (ref=e->ref; ref; ref=ref->next) > + last_array_ref = ref->type == REF_ARRAY; This rejects too much - you can also have a substring reference at the end – and then the arrayness still matters. character(type=4, len=5) :: str(50) call foo(str(1)) ! This makes sense call foo(str(1)(3:4)) ! Technically valid, but feels odd > argument_checking_24.f90 > I also would prefer to have some more test coverage. For instance: type(tt), pointer :: tt_var2 allocate(tt_var2) call s2(tt_var2%x(1)) ! Valid subroutine foo3(x) type(tt) :: tt_var2(:) call s1(tt_var2%x(1)) ! Valid call s4(dt%array_var%scalar) ! Invalid Actually, I wonder whether you code as any effects on strings as at least the test for "Element of assumed-shaped or pointer array passed to array dummy argument" permits any string and not only default-kind/c_char strings. – I am pretty sure that some C-binding test case already checks that those are accepted. Cheers, Tobias |
OK, so here's the update. There was a problem with uninitialized
variables, which for some reason was not detected on compilation. OK for trunk? 2019-10-13 Thomas Koenig <[hidden email]> PR fortran/92004 * array.c (expand_constructor): Set from_constructor on expression. * gfortran.h (gfc_symbol): Add maybe_array. (gfc_expr): Add from_constructor. * interface.c (maybe_dummy_array_arg): New function. (compare_parameter): If the formal argument is generated from a call, check the conditions where an array element could be passed to an array. Adjust error message for assumed-shape or pointer array. Use correct language for assumed shaped arrays. (gfc_get_formal_from_actual_arglist): Set maybe_array on the symbol if the actual argument is an array element fulfilling the conditions of 15.5.2.4. 2019-10-13 Thomas Koenig <[hidden email]> PR fortran/92004 * gfortran.dg/argument_checking_24.f90: New test. * gfortran.dg/abstract_type_6.f90: Add error message. * gfortran.dg/argument_checking_11.f90: Correct wording in error message. * gfortran.dg/argumeent_checking_13.f90: Likewise. * gfortran.dg/interface_40.f90: Add error message. |
Committed, with that nitch, r276972.
> OK with a minor nit. — Thanks for the patch. Thanks a lot for the review! Regards Thomas |
Free forum by Nabble | Edit this page |