[Patch, Fortran] PR 35831: [F95] Shape mismatch check missing for dummy procedure argument

[Patch, Fortran] PR 35831: [F95] Shape mismatch check missing for dummy procedure argument

[Janus Weil]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

Hi all,

here is a patch for a rather long-standing PR. It continues my ongoing
campaign of improving the checks for "procedure characteristics" (cf.
F08 chapter 12.3), which are relevant for dummy procedures, procedure
pointer assignments, overriding of type-bound procedures, etc.

This particular patch checks for the correct shape of array arguments,
in a manner similar to the recently added check for the string length
(PR 49638), namely via 'gfc_dep_compare_expr'.

The hardest thing about this PR was to find out what exactly the
standard requires (cf. c.l.f. thread linked in comment #12): Only the
shape of the argument has to match (i.e. upper minus lower bound), not
the bounds themselves (no matter if the bounds are constant or not).

I also added a FIXME, in order to remind myself of adding the same
check for function results soon.

The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?

Cheers,
Janus


2011-10-03  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/35831
	* interface.c (check_dummy_characteristics): Check the array shape.


2011-10-03  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/35831
	* gfortran.dg/dummy_procedure_6.f90: New.

[-- Attachment #2: pr35831_v2.diff --]
[-- Type: text/x-diff, Size: 2128 bytes --]

Index: gcc/fortran/interface.c
===================================================================
--- gcc/fortran/interface.c	(revision 179468)
+++ gcc/fortran/interface.c	(working copy)
@@ -69,6 +69,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "gfortran.h"
 #include "match.h"
+#include "arith.h"
 
 /* The current_interface structure holds information about the
    interface currently being parsed.  This structure is saved and
@@ -1071,13 +1072,51 @@ check_dummy_characteristics (gfc_symbol *s1, gfc_s
   /* Check array shape.  */
   if (s1->as && s2->as)
     {
+      int i, compval;
+      gfc_expr *shape1, *shape2;
+
       if (s1->as->type != s2->as->type)
 	{
 	  snprintf (errmsg, err_len, "Shape mismatch in argument '%s'",
 		    s1->name);
 	  return FAILURE;
 	}
-      /* FIXME: Check exact shape.  */
+
+      if (s1->as->type == AS_EXPLICIT)
+	for (i = 0; i < s1->as->rank + s1->as->corank; i++)
+	  {
+	    shape1 = gfc_subtract (gfc_copy_expr (s1->as->upper[i]),
+				  gfc_copy_expr (s1->as->lower[i]));
+	    shape2 = gfc_subtract (gfc_copy_expr (s2->as->upper[i]),
+				  gfc_copy_expr (s2->as->lower[i]));
+	    compval = gfc_dep_compare_expr (shape1, shape2);
+	    gfc_free_expr (shape1);
+	    gfc_free_expr (shape2);
+	    switch (compval)
+	    {
+	      case -1:
+	      case  1:
+	      case -3:
+		snprintf (errmsg, err_len, "Shape mismatch in dimension %i of "
+			  "argument '%s'", i, s1->name);
+		return FAILURE;
+
+	      case -2:
+		/* FIXME: Implement a warning for this case.
+		gfc_warning ("Possible shape mismatch in argument '%s'",
+			    s1->name);*/
+		break;
+
+	      case 0:
+		break;
+
+	      default:
+		gfc_internal_error ("check_dummy_characteristics: Unexpected "
+				    "result %i of gfc_dep_compare_expr",
+				    compval);
+		break;
+	    }
+	  }
     }
     
   return SUCCESS;
@@ -1131,6 +1170,8 @@ gfc_compare_interfaces (gfc_symbol *s1, gfc_symbol
 			  "of '%s'", name2);
 	      return 0;
 	    }
+
+	  /* FIXME: Check array bounds and string length of result.  */
 	}
 
       if (s1->attr.pure && !s2->attr.pure)

[-- Attachment #3: dummy_procedure_6.f90 --]
[-- Type: text/x-fortran, Size: 1126 bytes --]

! { dg-do compile }
!
! PR 35381: [F95] Shape mismatch check missing for dummy procedure argument
!
! Contributed by Janus Weil <janus@gcc.gnu.org>

module m

  implicit none

contains

  ! constant array bounds

  subroutine s1(a)
    integer :: a(1:2)
  end subroutine

  subroutine s2(a)
    integer :: a(2:3)
  end subroutine

  subroutine s3(a)
    integer :: a(2:4)
  end subroutine

  ! non-constant array bounds

  subroutine t1(a,b)
    integer :: b
    integer :: a(1:b,1:b)
  end subroutine

  subroutine t2(a,b)
    integer :: b
    integer :: a(1:b,2:b+1)
  end subroutine

  subroutine t3(a,b)
    integer :: b
    integer :: a(1:b,1:b+1)
  end subroutine

end module


program test
  use m
  implicit none

  call foo(s1)  ! legal
  call foo(s2)  ! legal
  call foo(s3)  ! { dg-error "Shape mismatch in dimension" }

  call bar(t1)  ! legal
  call bar(t2)  ! legal
  call bar(t3)  ! { dg-error "Shape mismatch in dimension" }

contains

  subroutine foo(f)
    procedure(s1) :: f
  end subroutine

  subroutine bar(f)
    procedure(t1) :: f
  end subroutine

end program

! { dg-final { cleanup-modules "m" } }

Re: [Patch, Fortran] PR 35831: [F95] Shape mismatch check missing for dummy procedure argument

[Mikael Morin]

On Monday 03 October 2011 23:02:15 Janus Weil wrote:
> Hi all,
> 
> here is a patch for a rather long-standing PR. It continues my ongoing
> campaign of improving the checks for "procedure characteristics" (cf.
> F08 chapter 12.3), which are relevant for dummy procedures, procedure
> pointer assignments, overriding of type-bound procedures, etc.
> 
> This particular patch checks for the correct shape of array arguments,
> in a manner similar to the recently added check for the string length
> (PR 49638), namely via 'gfc_dep_compare_expr'.
> 
> The hardest thing about this PR was to find out what exactly the
> standard requires (cf. c.l.f. thread linked in comment #12): Only the
> shape of the argument has to match (i.e. upper minus lower bound), not
> the bounds themselves (no matter if the bounds are constant or not).
> 
> I also added a FIXME, in order to remind myself of adding the same
> check for function results soon.
> 
> The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?
> 

The patch is basically OK.
Any reason to keep this disabled?

+             case -2:
+               /* FIXME: Implement a warning for this case.
+               gfc_warning ("Possible shape mismatch in argument '%s'",
+                           s1->name);*/
+               break;
+

Mikael

Re: [Patch, Fortran] PR 35831: [F95] Shape mismatch check missing for dummy procedure argument

[Janus Weil]

Hi Mikael,

>> here is a patch for a rather long-standing PR. It continues my ongoing
>> campaign of improving the checks for "procedure characteristics" (cf.
>> F08 chapter 12.3), which are relevant for dummy procedures, procedure
>> pointer assignments, overriding of type-bound procedures, etc.
>>
>> This particular patch checks for the correct shape of array arguments,
>> in a manner similar to the recently added check for the string length
>> (PR 49638), namely via 'gfc_dep_compare_expr'.
>>
>> The hardest thing about this PR was to find out what exactly the
>> standard requires (cf. c.l.f. thread linked in comment #12): Only the
>> shape of the argument has to match (i.e. upper minus lower bound), not
>> the bounds themselves (no matter if the bounds are constant or not).
>>
>> I also added a FIXME, in order to remind myself of adding the same
>> check for function results soon.
>>
>> The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?
>>
>
> The patch is basically OK.

thanks for the review!


> Any reason to keep this disabled?
>
> +             case -2:
> +               /* FIXME: Implement a warning for this case.
> +               gfc_warning ("Possible shape mismatch in argument '%s'",
> +                           s1->name);*/
> +               break;

Well, in principle not. As you might have noticed, such a warning is
already in effect e.g. when calling 'gfc_dep_compare_expr' from
'gfc_check_typebound_override'.

The problem with 'check_dummy_characteristics' is that it does not
directly throw an error, but only writes a message into a string
variable and returns FAILURE to the calling procedure, which itself is
responsible for throwing the error.

This mechanism is used to enhance flexibility and improve the error
message by prepending a string describing the context in which the
error message occurs (e.g. we might see a mismatch of characteristics
either in a procedure pointer assignment or when passing an actual
argument for a dummy procedure, etc).

The situation is further complicated by the fact that
'check_dummy_characteristics' is also called by
'gfc_compare_interfaces', which itself does not throw an error either,
but again just passes a string to the calling procedure.

If you have a cute idea how to elegantly introduce warnings into this
mechanism, I'm all ears. Otherwise I'll just start by committing the
patch as posted ...

Thanks again for the feedback,
Janus

Re: [Patch, Fortran] PR 35831: [F95] Shape mismatch check missing for dummy procedure argument

[Janus Weil]

>>> The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?
>>>
>> The patch is basically OK.


> Otherwise I'll just start by committing the
> patch as posted ...

Just did so (r179520).

Cheers,
Janus

Re: [Patch, Fortran] PR 35831: [F95] Shape mismatch check missing for dummy procedure argument

[Mikael Morin]

On Tuesday 04 October 2011 19:01:50 Janus Weil wrote:
> If you have a cute idea how to elegantly introduce warnings into this
> mechanism, I'm all ears.
I'm not sure that it qualifies as cute, but we could produce multi-line 
diagnostics in the same way c++ does (for template candidates for example), 
like:
error/warning: #the error/warning
note: in actual argument at <> # the context

> Otherwise I'll just start by committing the
> patch as posted ...
Sure, no problem.

Mikael

Re: [Patch, Fortran] PR 35831: [F95] Shape mismatch check missing for dummy procedure argument

[Janus Weil]

>> If you have a cute idea how to elegantly introduce warnings into this
>> mechanism, I'm all ears.
> I'm not sure that it qualifies as cute, but we could produce multi-line
> diagnostics in the same way c++ does (for template candidates for example),
> like:
> error/warning: #the error/warning
> note: in actual argument at <> # the context

Maybe not the worst idea. The question is, how would we do this
technically? Can it be handled with the existing infrastructure
(gfc_error_... )?

Alternatives might be:
 * simply throw the warning without context
 * have 'check_dummy_characteristics' not return SUCCESS/FAILURE, but
something like SUCCESS/WARNING/ERROR and then react on this
appropriately by throwing either an error or a warning (more tedious)

Cheers,
Janus

[committed] small change (was: Re: [Patch, Fortran] PR 35831: [F95] Shape mismatch check missing for dummy procedure argument)

[Mikael Morin]

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

On Tuesday 04 October 2011 20:54:21 Janus Weil wrote:
> >>> The patch was regtested on x86_64-unknown-linux-gnu. Ok for trunk?
> >> 
> >> The patch is basically OK.
> > 
> > Otherwise I'll just start by committing the
> > patch as posted ...
> 
> Just did so (r179520).
> 

Hello, 

I've just committed the following amendment as revision 179726.

Mikael


[-- Attachment #2: r179726.diff --]
[-- Type: text/x-patch, Size: 901 bytes --]

Index: interface.c
===================================================================
--- interface.c	(révision 179725)
+++ interface.c	(révision 179726)
@@ -1098,7 +1098,7 @@ check_dummy_characteristics (gfc_symbol *s1, gfc_s
 	      case  1:
 	      case -3:
 		snprintf (errmsg, err_len, "Shape mismatch in dimension %i of "
-			  "argument '%s'", i, s1->name);
+			  "argument '%s'", i + 1, s1->name);
 		return FAILURE;
 
 	      case -2:
Index: ChangeLog
===================================================================
--- ChangeLog	(révision 179725)
+++ ChangeLog	(révision 179726)
@@ -1,3 +1,8 @@
+2011-10-09  Mikael Morin  <mikael.morin@sfr.fr>
+
+	* interface.c (check_dummy_characteristics): Count dimensions starting
+	from one in diagnostic.
+
 2011-10-09  Tobias Burnus  <burnus@net-b.de>
 
 	* Make-lang.in (F95_PARSER_OBJS, GFORTRAN_TRANS_DEPS): Add