Hi Tobias,

> > We should rather open a separate PR on auditing the related uses
> > of gfc_match.
> 
> I concur – I just wanted to quickly check how many %v are there –
> besides %v, there are also direct calls to gfc_match_variable.
> 
> Can you open a PR?

this is now https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101652

> The loops are clear – but the
>   '!stat->ref || (...ref->type != ARRAY || ref->type != COMPONENT))'
> is still not clear to me.

Ah, I was really missing the point and looking in the wrong place.
Actually, I also do not understand the reason for this version of
the check, and it also leads to a strange accepts-invalid for
certain non-integer STAT variables.  Removing the stat->ref part
fixes that without introducing any regression in the testsuite.
(There was an analogous part in the check for ERRMSG.)

> Can you add the (working) test:
>    allocate (A, stat=y%stat%kind)  ! { dg-error "cannot be a constant" }
>    deallocate (A, stat=y%stat%kind)  ! { dg-error "cannot be a constant" }
> to your testcase gcc/testsuite/gfortran.dg/allocate_stat_3.f90 ?

Done.

> And also the following one, which does not get diagnosed and, hence,
> later gives an ICE during gimplification.
> 
>    type tc
>      character (len=:), allocatable :: str
>    end type tc
> ...
>    type(tc) :: z
> ...
>    allocate(character(len=13) :: z%str)
>    allocate (A, stat=z%str%len)
>    deallocate (A, stat=z%str%len)
> 
> To fix it, I think the solution is to do the following:
> * In gfc_check_vardef_context, handle also REF_INQUIRY; in the
>      for (ref = e->ref; ref && check_intentin; ref = ref->next)
>    loop, I think there should be a
>      if (ref->type == REF_INQUIRY)
>        {
>          if (context)
>           gfc_error ("Type parameter inquiry for %qs in "
>                      "variable definition context (%s) at %L",
>                      name, context, &e->where);
>          return false;
>        }
> (untested)

This almost worked, needing only a restriction to %KIND and %LEN.
Note that %RE and %IM are usually definable.

> I assume (but have not tested it) that will give
> two error messages for:
>    allocate (A, errmsg=z%str%len)
>    deallocate (A, errmsg=z%str%len)
> one for the new type-param-inquiry check and one for
>    != BT_CHARACTER
> if you want to prevent the double error, consider to
> replace
>     gfc_check_vardef_context (...);
> by
>     if (!gfc_check_vardef_context (...))
>       goto done_errmsg;

Yes, that is reasonable.  Done.

> > Regtested on x86_64-pc-linux-gnu.  OK?
> LGTM - except for the two testcase additions proposed above
> and fixing the ICE. If you are happy with my changes and they
> work, feel free add them and commit without further review.
> In either case, I have the feeling we are nearly there. :-)

I have added the updated "final" version of the patch to give
everybody another 24h to have a look, and will commit if nobody
complains.

> Thanks for the patch and the review-modification-review-... patience!

Well, I believe this was really a worthwile review process,
with fixing a few issues on the way before Gerhard finds them...

Thanks,
Harald


Fortran: ICE in resolve_allocate_deallocate for invalid STAT argument

gcc/fortran/ChangeLog:

	PR fortran/101564
	* expr.c (gfc_check_vardef_context): Add check for KIND and LEN
	parameter inquiries.
	* match.c (gfc_match): Fix comment for %v code.
	(gfc_match_allocate, gfc_match_deallocate): Replace use of %v code
	by %e in gfc_match to allow for function references as STAT and
	ERRMSG arguments.
	* resolve.c (resolve_allocate_deallocate): Avoid NULL pointer
	dereferences and shortcut for bad STAT and ERRMSG argument to
	(DE)ALLOCATE.  Remove bogus parts of checks for STAT and ERRMSG.

gcc/testsuite/ChangeLog:

	PR fortran/101564
	* gfortran.dg/allocate_stat_3.f90: New test.
	* gfortran.dg/allocate_stat.f90: Adjust error messages.
	* gfortran.dg/implicit_11.f90: Likewise.
	* gfortran.dg/inquiry_type_ref_3.f90: Likewise.