[Bug fortran/58331] New: [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[Bug fortran/58331] New: [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

            Bug ID: 58331
           Summary: [OOP] Bogus rank checking with explicit-/assumed-size
                    arrays and CLASS
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Keywords: rejects-valid
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: burnus at gcc dot gnu.org
                CC: janus at gcc dot gnu.org

The following code seems to be valid and to work, except for the bogus error
message:

Error: Rank mismatch in argument 'a' at (1) (rank-1 and rank-2)


Reported by Simon Geard at
https://groups.google.com/forum/#!topic/comp.lang.fortran/5oi9Sa4lnX4


module mymod
   implicit none
contains
   subroutine mysub(a, n)
     integer, intent(in)  :: n
     class(*), intent(in) :: a(n)

     select type(a)
     type is(integer)
         print *,'a is integer'
     class default
         print *,'a is unsupported type'
     end select
   end subroutine mysub
end module mymod

program upv
   use mymod
   implicit none
   integer :: a(3), b(2,2)

   a = [1, 2, 3]
   call mysub(a,3)

   b = reshape([1, 2, 3, 4], [2,2])
   call mysub(b,4)
end program upv

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

--- Comment #1 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Untested patch:

--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -2135 +2135,2 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
-      || (actual->rank != 0 && !(is_elemental || formal->attr.dimension))
+      || (actual->rank != 0 && !(is_elemental || formal->attr.dimension
+                                || CLASS_DATA (formal)->attr.dimension))

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code

--- Comment #2 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Actually, the patch is sufficient as one then runs into a tree checking error:

class_array_20.f90:21:0: error: non-trivial conversion at assignment
  struct array1_unknown
  struct array2_integer(kind=4)

if one looks at the dump, one sees that the actual argument still sends the
array descriptor - but it shouldn't:
      parm.7.dim[0].lbound = 1;
      ...
      parm.7.data = (void *) &b[0];
      class.6._data = parm.7;
      mysub (&class.6, &C.1946);

Thus, the test case also doesn't work for rank == 1, if one checks the value.


TODO
* Sends in _data the array data, not the descriptor
* Add a test case for the coindexed part of the patch below.
* Augment the test case in comment 0 with a value check



Updated patch:
--- a/gcc/fortran/interface.c
+++ b/gcc/fortran/interface.c
@@ -2135 +2135,5 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
-      || (actual->rank != 0 && !(is_elemental || formal->attr.dimension))
+      || (actual->rank != 0
+          && !(is_elemental
+           || (formal->ts.type != BT_CLASS && formal->attr.dimension)
+           || (formal->ts.type == BT_CLASS
+           && CLASS_DATA (formal)->attr.dimension)))
@@ -2142 +2146,4 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
-      || (actual->rank == 0 && formal->attr.dimension
+      || (actual->rank == 0
+      && (formal->attr.dimension
+          || (formal->ts.type == BT_CLASS
+                  && CLASS_DATA (formal)->attr.dimension))
@@ -2150 +2157,4 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
-  else if (actual->rank != 0 && (is_elemental || formal->attr.dimension))
+  else if (actual->rank != 0
+       && (is_elemental || formal->attr.dimension
+           || (formal->ts.type == BT_CLASS
+           && CLASS_DATA (formal)->attr.dimension)))

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[dominiq at lps dot ens.fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-03-22
     Ever confirmed|0                           |1

--- Comment #3 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
With the patch in comment 2, I also get (r208765)

pr58331.f90: In function 'upv':
pr58331.f90:19:0: error: non-trivial conversion at assignment
 program upv 
 ^
struct array1_unknown
struct array2_integer(kind=4)
class.6._data = parm.7;
pr58331.f90:19:0: internal compiler error: verify_gimple failed

I also get the same ICE if I use

     class(*), intent(in) :: a(..)
...
      print *, rank(a)

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |anlauf at gcc dot gnu.org

--- Comment #4 from anlauf at gcc dot gnu.org ---
(In reply to Tobias Burnus from comment #2)
> Updated patch:
> --- a/gcc/fortran/interface.c
> +++ b/gcc/fortran/interface.c
> @@ -2135 +2135,5 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
> -      || (actual->rank != 0 && !(is_elemental || formal->attr.dimension))
> +      || (actual->rank != 0
> +          && !(is_elemental
> +	       || (formal->ts.type != BT_CLASS && formal->attr.dimension)
> +	       || (formal->ts.type == BT_CLASS
> +		   && CLASS_DATA (formal)->attr.dimension)))
> @@ -2142 +2146,4 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
> -      || (actual->rank == 0 && formal->attr.dimension
> +      || (actual->rank == 0
> +	  && (formal->attr.dimension
> +	      || (formal->ts.type == BT_CLASS
> +                  && CLASS_DATA (formal)->attr.dimension))
> @@ -2150 +2157,4 @@ compare_parameter (gfc_symbol *formal, gfc_expr *actual,
> -  else if (actual->rank != 0 && (is_elemental || formal->attr.dimension))
> +  else if (actual->rank != 0
> +	   && (is_elemental || formal->attr.dimension
> +	       || (formal->ts.type == BT_CLASS
> +		   && CLASS_DATA (formal)->attr.dimension)))

When adapting this patch to mainline it seems to work now and regtests ok.

Printing array a in mysub gives the right results, and also the
tree dump for the call with rank-2 argument looks slightly different:

    parm.12.data = (void *) &b[0];
    parm.12.offset = -3;
    class.11._data = VIEW_CONVERT_EXPR<struct array01_unknown>(parm.12);
    class.11._len = 0;
    mysub (&class.11, &C.4379);

So shall we proceed?

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

--- Comment #5 from anlauf at gcc dot gnu.org ---
The previous patch would accept invalid code, as it would miss assumed-shape.
Fixed by the additional:

@@ -2650,10 +2669,14 @@ compare_parameter (gfc_symbol *formal, gfc_expr
*actual,
   if (symbol_rank (formal) == actual->rank || symbol_rank (formal) == -1)
     return true;

-  rank_check = where != NULL && !is_elemental && formal->as
-              && (formal->as->type == AS_ASSUMED_SHAPE
-                  || formal->as->type == AS_DEFERRED)
-              && actual->expr_type != EXPR_NULL;
+  gfc_array_spec *formal_as;
+  formal_as = formal->ts.type == BT_CLASS ? CLASS_DATA (formal)->as
+                                         : formal->as;
+
+  rank_check = where != NULL && !is_elemental && formal_as
+    && (formal_as->type == AS_ASSUMED_SHAPE
+       || formal_as->type == AS_DEFERRED)
+    && actual->expr_type != EXPR_NULL;

   /* Skip rank checks for NO_ARG_CHECK.  */
   if (formal->attr.ext_attr & (1 << EXT_ATTR_NO_ARG_CHECK))

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

--- Comment #6 from Tobias Burnus <burnus at gcc dot gnu.org> ---
> So shall we proceed?

Looks like. I think it mostly needs a bunch of testcases to ensure it works,
including the coarray-testcase mentioned in one of the comments. If something
fails, we can have another look.

I think it makes sense to attached then a diff with the current patch and all
testcases (passing and failing) to this PR such that we can have a look.
Currently, the patches are spread out over comments, the testcases are also
separate etc.
(If everything works, it could be directly posted, I think.)

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

--- Comment #7 from anlauf at gcc dot gnu.org ---
Created attachment 54627
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54627&action=edit
Latest patch

This patch fixes the testcases in this PR.

I tried other testcases that use coarrays, but e.g. the following scalar
coarray case produces an ICE later on.  I believe this is a pre-existing
separate issue.

program p
  implicit none
  integer :: ca[*]
  ca = 2
  call sub_ca(ca)
contains
  subroutine sub_ca (a)
    class(*), intent(in) :: a[*]
  end
end

pr58331-ca.f90:8:17:

    8 |   call sub_ca(ca)
      |                 1
internal compiler error: in fold_convert_loc, at fold-const.cc:2627
0x726047 fold_convert_loc(unsigned int, tree_node*, tree_node*)
        ../../gcc-trunk/gcc/fold-const.cc:2627
0xa5edd2 gfc_conv_intrinsic_to_class(gfc_se*, gfc_expr*, gfc_typespec)
        ../../gcc-trunk/gcc/fortran/trans-expr.cc:1040
0xa70271 gfc_conv_procedure_call(gfc_se*, gfc_symbol*, gfc_actual_arglist*,
gfc_expr*, vec<tree_node*, va_gc, vl_embed>*)
        ../../gcc-trunk/gcc/fortran/trans-expr.cc:6239
0xadef24 gfc_trans_call(gfc_code*, bool, tree_node*, tree_node*, bool)
        ../../gcc-trunk/gcc/fortran/trans-stmt.cc:424
0xa0f555 trans_code
        ../../gcc-trunk/gcc/fortran/trans.cc:2018
0xa0fa1a gfc_trans_code(gfc_code*)
        ../../gcc-trunk/gcc/fortran/trans.cc:2303
0xa586b0 gfc_generate_function_code(gfc_namespace*)
        ../../gcc-trunk/gcc/fortran/trans-decl.cc:7704
0xa0fa5e gfc_generate_code(gfc_namespace*)
        ../../gcc-trunk/gcc/fortran/trans.cc:2320
0x9a5de6 translate_all_program_units
        ../../gcc-trunk/gcc/fortran/parse.cc:6720
0x9a66e8 gfc_parse_file()
        ../../gcc-trunk/gcc/fortran/parse.cc:7026
0x9f6d5e gfc_be_parse_file
        ../../gcc-trunk/gcc/fortran/f95-lang.cc:229

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |anlauf at gcc dot gnu.org

--- Comment #8 from anlauf at gcc dot gnu.org ---
Submitted: https://gcc.gnu.org/pipermail/fortran/2023-March/059077.html

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

--- Comment #9 from Tobias Burnus <burnus at gcc dot gnu.org> ---
COARRAYS:

(In reply to anlauf from comment #7)
> I tried other testcases that use coarrays, but e.g. the following scalar
> coarray case produces an ICE later on.  I believe this is a pre-existing
> separate issue.

A coarray 'ca' can either be a local variable (the usual case), a remove
reference (then with '[indx]' in the expr) - or, like here, the full thing.

In gfc_conv_intrinsic_to_class, the gfc_conv_expr_reference 'destroys' the
coarray part. I think we need something like 'gfc_conv_variable' — and I bet
that we need to ensure all combinations work:

(scalar,array w/o and w/ descriptor) → (assumed rank, array w/o and w/
descriptor) - where the 'to class' (and, obviously, the 'from' was well) are
coarrays.

Besides intrinsic types -> class(*), we should also check check
type(t) -> ('class(t)' and 'class(*)')

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Harald Anlauf <anlauf@gcc.gnu.org>:

https://gcc.gnu.org/g:901edd99b44976b3c2b13a7d525d9e315540186a

commit r13-6698-g901edd99b44976b3c2b13a7d525d9e315540186a
Author: Harald Anlauf <anlauf@gmx.de>
Date:   Tue Mar 14 20:23:06 2023 +0100

    Fortran: rank checking with explicit-/assumed-size arrays and CLASS
[PR58331]

    gcc/fortran/ChangeLog:

            PR fortran/58331
            * interface.cc (compare_parameter): Adjust check of array dummy
            arguments to handle the case of CLASS variables.

    gcc/testsuite/ChangeLog:

            PR fortran/58331
            * gfortran.dg/class_dummy_10.f90: New test.

    Co-authored-by: Tobias Burnus <tobias@codesourcery.com>

[Bug fortran/58331] [OOP] Bogus rank checking with explicit-/assumed-size arrays and CLASS

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58331

--- Comment #11 from anlauf at gcc dot gnu.org ---
(In reply to Tobias Burnus from comment #9)
> Besides intrinsic types -> class(*), we should also check check
> type(t) -> ('class(t)' and 'class(*)')

I've opened pr109148 for related tests and issues (ICEs).