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

Reply via email to