[fortran, patches] Two short patches to review

[fortran, patches] Two short patches to review

[FX]

PRs 50540 and 50404 each contain a short patch, written by Steve. Both patches are straightforward:

  -- 50404: refuse to have a CLOSE statement without a UNIT (F2008's C908 "A file-unit-number shall be specified in a close-spec-list") (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50404)
  -- 50540: issue a normal error instead of an internal error, in conditions that could reasonably be triggered by invalid code (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50540)

Both patches seem fine to me, and I have regtested them. Could someone review them and approve them for trunk? I'll then add the testcase from the PR and commit.

Cheers,
FX

Re: [fortran, patches] Two short patches to review

[Steve Kargl]

On Wed, Nov 09, 2011 at 12:13:10AM +0100, FX wrote:
> PRs 50540 and 50404 each contain a short patch, written by Steve.
> Both patches are straightforward:
> 
> -- 50404: refuse to have a CLOSE statement without a UNIT
> (F2008's C908 "A file-unit-number shall be specified in a
> close-spec-list") (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50404)

jerry already approved this one.

> -- 50540: issue a normal error instead of an internal error, in
> conditions that could reasonably be triggered by invalid code
> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50540)

Given that you've read the patch and my comments in the PR,
and that you've contributed/reviewed/committed hundreds of
patches, I think that this is sufficient review.  I certainly
have no problems with committing the patch.

PS: I would have committed these patch except I have spent a
week or so trying to get gcc to build again after I migrated my
systems to FreeBSD 10.0 (ie., the bleeding edge).

-- 
Steve

Re: [fortran, patches] Two short patches to review

[FX]

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

>> -- 50404: refuse to have a CLOSE statement without a UNIT
>> (F2008's C908 "A file-unit-number shall be specified in a
>> close-spec-list") (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50404)
> 
> jerry already approved this one.

And I committed it as rev. , with a slight modification to add a decent locus (otherwise, the current locus is at a totally wrong place) and a testcase. Committed patch is attached.


[-- Attachment #2: close.diff --]
[-- Type: application/octet-stream, Size: 2559 bytes --]

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(revision 181182)
+++ gcc/fortran/ChangeLog	(working copy)
@@ -1,5 +1,10 @@
 2011-11-08  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>
 
+	PR fortran/50404
+	* io.c (gfc_resolve_close): CLOSE requires a UNIT.
+
+2011-11-08  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>
+
 	PR fortran/50409
 	* expr.c (gfc_simplify_expr): Substrings can't have negative
 	length.
Index: gcc/fortran/io.c
===================================================================
--- gcc/fortran/io.c	(revision 181149)
+++ gcc/fortran/io.c	(working copy)
@@ -2295,6 +2295,24 @@ gfc_resolve_close (gfc_close *close)
   if (gfc_reference_st_label (close->err, ST_LABEL_TARGET) == FAILURE)
     return FAILURE;
 
+  if (close->unit == NULL)
+    {
+      /* Find a locus from one of the arguments to close, when UNIT is
+	 not specified.  */
+      locus loc = gfc_current_locus;
+      if (close->status)
+	loc = close->status->where;
+      else if (close->iostat)
+	loc = close->iostat->where;
+      else if (close->iomsg)
+	loc = close->iomsg->where;
+      else if (close->err)
+	loc = close->err->where;
+
+      gfc_error ("CLOSE statement at %L requires a UNIT number", &loc);
+      return FAILURE;
+    }
+
   if (close->unit->expr_type == EXPR_CONSTANT
       && close->unit->ts.type == BT_INTEGER
       && mpz_sgn (close->unit->value.integer) < 0)
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 181182)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,7 +1,12 @@
 2011-11-08  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>
 
+	PR fortran/50404
+	* gfortran.dg/io_constraints_3.f90: Improve testcase.
+
+2011-11-08  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>
+
 	PR fortran/50409
-	* gcc/testsuite/gfortran.dg/string_5.f90: Improve testcase.
+	* gfortran.dg/string_5.f90: Improve testcase.
 
 2011-10-23  Jason Merrill  <jason@redhat.com>
 
Index: gcc/testsuite/gfortran.dg/io_constraints_3.f90
===================================================================
--- gcc/testsuite/gfortran.dg/io_constraints_3.f90	(revision 181149)
+++ gcc/testsuite/gfortran.dg/io_constraints_3.f90	(working copy)
@@ -66,6 +66,7 @@
   close(10, iostat=u,status="keep")
   close(10, iostat=u,status="delete")
   close(10, iostat=u,status=foo) ! { dg-warning "STATUS specifier in CLOSE statement" }
+  close(iostat=u) ! { dg-error "requires a UNIT number" }
 
 
 

[-- Attachment #3: Type: text/plain, Size: 565 bytes --]




>> -- 50540: issue a normal error instead of an internal error, in
>> conditions that could reasonably be triggered by invalid code
>> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50540)
> 
> Given that you've read the patch and my comments in the PR,
> and that you've contributed/reviewed/committed hundreds of
> patches, I think that this is sufficient review.  I certainly
> have no problems with committing the patch.

Well, given the amount of compliment, I'm sure to screw up and then hide :)
I'll get to it tomorrow morning, after some sleep.

Thanks,
FX

Re: [fortran, patches] Two short patches to review

[Steve Kargl]

On Wed, Nov 09, 2011 at 12:57:29AM +0100, FX wrote:
> >> -- 50404: refuse to have a CLOSE statement without a UNIT
> >> (F2008's C908 "A file-unit-number shall be specified in a
> >> close-spec-list") (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50404)
> > 
> > jerry already approved this one.
> 
> And I committed it as rev. , with a slight modification to add
> a decent locus (otherwise, the current locus is at a totally
> wrong place) and a testcase. Committed patch is attached.

Thanks!

> >> -- 50540: issue a normal error instead of an internal error, in
> >> conditions that could reasonably be triggered by invalid code
> >> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50540)
> > 
> > Given that you've read the patch and my comments in the PR,
> > and that you've contributed/reviewed/committed hundreds of
> > patches, I think that this is sufficient review.  I certainly
> > have no problems with committing the patch.
> 
> Well, given the amount of compliment, I'm sure to screw up and then hide :)
> I'll get to it tomorrow morning, after some sleep.

Although I suspect you've been lurking in the background,
welcome back to the land of gfortran hacking.  Your first
screw up is free, additional screw ups require you to
fix your screw up and fix an additional bug as your reward.

:-)

-- 
Steve

Re: [fortran, patches] Two short patches to review

[FX]

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

> Although I suspect you've been lurking in the background,
> welcome back to the land of gfortran hacking.  Your first
> screw up is free, additional screw ups require you to
> fix your screw up and fix an additional bug as your reward.



Attached patch committed as revision 181200.
FX


[-- Attachment #2: convert.diff --]
[-- Type: application/octet-stream, Size: 2754 bytes --]

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(revision 181198)
+++ gcc/fortran/ChangeLog	(working copy)
@@ -1,5 +1,11 @@
 2011-11-08  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>
 
+	PR fortran/50540
+	* resolve.c (resolve_forall_iterators): Transform internal errors
+	to normal errors.
+
+2011-11-08  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>
+
 	PR fortran/38718
 	* intrinsic.c (add_functions): Allow dreal simplification.
 	* intrinsic.h (gfc_simplify_dreal): New prototype.
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(revision 181149)
+++ gcc/fortran/resolve.c	(working copy)
@@ -6427,14 +6427,14 @@ resolve_forall_iterators (gfc_forall_ite
 	gfc_error ("FORALL start expression at %L must be a scalar INTEGER",
 		   &iter->start->where);
       if (iter->var->ts.kind != iter->start->ts.kind)
-	gfc_convert_type (iter->start, &iter->var->ts, 2);
+	gfc_convert_type (iter->start, &iter->var->ts, 1);
 
       if (gfc_resolve_expr (iter->end) == SUCCESS
 	  && (iter->end->ts.type != BT_INTEGER || iter->end->rank != 0))
 	gfc_error ("FORALL end expression at %L must be a scalar INTEGER",
 		   &iter->end->where);
       if (iter->var->ts.kind != iter->end->ts.kind)
-	gfc_convert_type (iter->end, &iter->var->ts, 2);
+	gfc_convert_type (iter->end, &iter->var->ts, 1);
 
       if (gfc_resolve_expr (iter->stride) == SUCCESS)
 	{
@@ -6448,7 +6448,7 @@ resolve_forall_iterators (gfc_forall_ite
 		       &iter->stride->where);
 	}
       if (iter->var->ts.kind != iter->stride->ts.kind)
-	gfc_convert_type (iter->stride, &iter->var->ts, 2);
+	gfc_convert_type (iter->stride, &iter->var->ts, 1);
     }
 
   for (iter = it; iter; iter = iter->next)
Index: gcc/testsuite/gfortran.dg/forall_16.f90
===================================================================
--- gcc/testsuite/gfortran.dg/forall_16.f90	(revision 0)
+++ gcc/testsuite/gfortran.dg/forall_16.f90	(revision 0)
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! PR fortran/50540
+!
+  implicit none
+  integer i,dest(10)
+  forall (i=2:ix)  dest(i)=i ! { dg-error "has no IMPLICIT type" }
+end
+
+! { dg-excess-errors "Can't convert UNKNOWN to INTEGER" }
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 181199)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,5 +1,10 @@
 2011-11-09  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>
 
+	PR fortran/50540
+	* gfortran.dg/forall_16.f90: New test.
+
+2011-11-09  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>
+
 	PR fortran/38718
 	* gfortran.dg/initialization_29.f90: Expand test.