Hi Harald,

On 26.07.21 23:55, Harald Anlauf wrote:
I've updated this for ALLOCATE/DEALLOCATE and STAT/ERRMSG, see
attached patch.  This required updating the error messages of
two existing files in the testsuite.
Thanks.
Also affected: Some I/O items, a bunch of other stat=%v and
errmsg=%v.
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?

         if ((stat->ts.type != BT_INTEGER
              && !(stat->ref && (stat->ref->type == REF_ARRAY
                                 || stat->ref->type == REF_COMPONENT)))
             || stat->rank > 0)
           gfc_error ("Stat-variable at %L must be a scalar INTEGER "
                      "variable", &stat->where);
I mean the ts.type != BT_INTEGER and stat->rank != 0 is clear,
but what's the reason for the refs?
Well, that needs to be answered by Steve (see commit 3759634).

(https://gcc.gnu.org/g:3759634f3208cbc1226bec19d22cbff989a287c3 (svn
r145331))

The reason for the ref checks is unclear and seem to be wrong. The added
testcases also only use 'x' (real) and n or i (integer) as input, i.e.
they do not exercise this. I did not look for the patch email for reasoning.
Well, there is some text in the standard that I added in front of
the for loops, and this code is now exercised in the new testcase.

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

  * * *

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 ?


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)

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;

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. :-)

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

Tobias

Fortran: ICE in resolve_allocate_deallocate for invalid STAT argument

gcc/fortran/ChangeLog:

      PR fortran/101564
      * 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.

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: Adjust error messages.
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas 
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht 
München, HRB 106955

Reply via email to