diff mbox

PR47040 - Make error message for empty array constructor more helpful/correct

Message ID 4C6F6CD1-9AAB-459A-9182-655EDB56C437@lps.ens.fr
State New
Headers show

Commit Message

Dominique d'Humières April 6, 2016, 3:44 p.m. UTC
Is the following patch OK (regtested on x86_64-apple-darwin15)? Should it be back ported to the gcc-5 branch?

TIA

Dominique

Comments

Steve Kargl April 7, 2016, 5:48 a.m. UTC | #1
On Wed, Apr 06, 2016 at 05:44:55PM +0200, Dominique d'Humières wrote:
> Is the following patch OK (regtested on x86_64-apple-darwin15)? Should it be back ported to the gcc-5 branch?

No and No.
Dominique d'Humières April 7, 2016, 5:51 a.m. UTC | #2
Could you please elaborate.

Dominique

> Le 7 avr. 2016 à 07:48, Steve Kargl <sgk@troutmask.apl.washington.edu> a écrit :
> 
> On Wed, Apr 06, 2016 at 05:44:55PM +0200, Dominique d'Humières wrote:
>> Is the following patch OK (regtested on x86_64-apple-darwin15)? Should it be back ported to the gcc-5 branch?
> 
> No and No.
> 
> -- 
> Steve
Steve Kargl April 7, 2016, 1:59 p.m. UTC | #3
The latter is obvious as this "fixes" neither a regression
nor documentation.  For the former, see Fortran 95, section 4.5.
Dominique d'Humières April 9, 2016, 10:28 a.m. UTC | #4
>>> On Wed, Apr 06, 2016 at 05:44:55PM +0200, Dominique d'Humières wrote:
>>>> Is the following patch OK (regtested on x86_64-apple-darwin15)? Should it be back ported to the gcc-5 branch?
>>> 
>>> No and No.
> Le 7 avr. 2016 à 15:59, Steve Kargl <sgk@troutmask.apl.washington.edu> a écrit :
> 
> The latter is obvious as this "fixes" neither a regression
> nor documentation.  

I won’t argue.

> For the former, see Fortran 95, section 4.5.
> 
> -- 
> steve


Are you referring to

(a)  An empty sequence forms a zero-sized rank-one array.

(b) The type and type parameters of an array constructor are those of the ac-value expressions.

(c) something else?

For (a) gfortran accepts zero-sized rank-one array with a type as shown by the instances in the added test.

AFAIU (b), it implicitly rules out a zero-sized rank-one array without a type, which is rejected by gfortran (if not what is the type of ‘[ ]’ and what should be the result of SUM([ ])?). Note that I see

NOTE 4.73
Examples of zero-size array constructors are:
[ INTEGER :: ]

in my copy of the f2015 draft.

Note that the patch is not mine but has been submitted by Tobias Burnus more than five years ago. It does not change any gfortran functionality and is only intended to help the end user. 

I have no intention to fight for this patch. If you think it is really irrelevant, please close the PR according your taste.

Dominique
Steve Kargl April 9, 2016, 3:39 p.m. UTC | #5
On Sat, Apr 09, 2016 at 12:28:12PM +0200, Dominique d'Humières wrote:
> >>> On Wed, Apr 06, 2016 at 05:44:55PM +0200, Dominique d'Humières wrote:
> >>>> Is the following patch OK (regtested on x86_64-apple-darwin15)? Should it be back ported to the gcc-5 branch?
> >>> 
> >>> No and No.
> > Le 7 avr. 2016 à 15:59, Steve Kargl <sgk@troutmask.apl.washington.edu> a écrit :
> > 
> > The latter is obvious as this "fixes" neither a regression
> > nor documentation.  
> 
> I won’t argue.
> 
> > For the former, see Fortran 95, section 4.5.
> > 
> > -- 
> > steve
> 
> 
> Are you referring to
> 
> (a)  An empty sequence forms a zero-sized rank-one array.
> 
> (b) The type and type parameters of an array constructor are those of the ac-value expressions.
> 
> (c) something else?
> 

Fortran 95 doesn't have a type-spec in an array constructor.
Fortran 95 explicitly states

  "The type and type parameters of an array constructor are
   those of the ac-value expressions."

(/ /) is valid Fortran 95 syntax while (/ type-spec :: /) is
not valid.  type-spec was introduced in Fortran 2003.  The
error message as written is correct.

(/ /) is empty.
(/type-spec :: /) is not empty, and is  invalid Fortan 95.
(/1, 2, 3/) is not empty and has a type of INTEGER.

program foo
   call bar((/ /))
end program foo

% gfc -c -std=f95 foo.f90
foo.f90:2:17:

    call bar((/ /))
                 1
Error: Empty array constructor at (1) is not allowed

The above error is correct.  Adding any text referring
to type-spec is wrong.
Dominique d'Humières April 9, 2016, 3:56 p.m. UTC | #6
> (/ /) is valid Fortran 95 syntax
> ...
> 
> program foo
>   call bar((/ /))
> end program foo
> 
> % gfc -c -std=f95 foo.f90
> foo.f90:2:17:
> 
>    call bar((/ /))
>                 1
> Error: Empty array constructor at (1) is not allowed
> 
> The above error is correct.  

Well the two assertions look contradictory: if (/ /) is valid Fortran 95 syntax
why is it  not allowed?

> Adding any text referring
> to type-spec is wrong.
> 
> -- 
> Steve

Are you considering (/ ( i, i = 1, 0 ) /) as non empty?

Dominique
Steve Kargl April 9, 2016, 4:13 p.m. UTC | #7
On Sat, Apr 09, 2016 at 05:56:12PM +0200, Dominique d'Humières wrote:
> 
> > (/ /) is valid Fortran 95 syntax
> > ...
> > 
> > program foo
> >   call bar((/ /))
> > end program foo
> > 
> > % gfc -c -std=f95 foo.f90
> > foo.f90:2:17:
> > 
> >    call bar((/ /))
> >                 1
> > Error: Empty array constructor at (1) is not allowed
> > 
> > The above error is correct.  
> 
> Well the two assertions look contradictory: if (/ /) is valid
> Fortran 95 syntax why is it  not allowed?

It is valid syntax because of 

"An empty sequence forms a zero-sized rank-one array."

It seems that J3 saw the error in their ways as (/ /) is clearly
an empty array constructor, and fixed the possibility of creating
a typeless zero-sized, rank-one array.

> > Adding any text referring
> > to type-spec is wrong.
> > 
> 
> Are you considering (/ ( i, i = 1, 0 ) /) as non empty?
> 

It is a zero-sized rank-one array with type INTEGER.  

"The type and type parameters of an array constructor are those
 of the ac-value expressions."

ac-value    is  expr
            or  ac-implied-do

"If an ac-value is an ac-implied-do, it is expanded to form an ac-value
sequence under the control of the ac-do-variable..."
Dominique d'Humières April 9, 2016, 4:51 p.m. UTC | #8
> 
> It is valid syntax because of 
> 
> "An empty sequence forms a zero-sized rank-one array."
> 
> It seems that J3 saw the error in their ways as (/ /) is clearly
> an empty array constructor, and fixed the possibility of creating
> a typeless zero-sized, rank-one array.

This is exactly the point of the patch! typeless zero-sized, rank-one array
are not allowed, while typed zero-sized, rank-one array are.

> 
>>> Adding any text referring
>>> to type-spec is wrong.
>>> 
>> 
>> Are you considering (/ ( i, i = 1, 0 ) /) as non empty?
>> 
> 
> It is a zero-sized rank-one array with type INTEGER.  

That is not the answer to the question. Is (/ ( i, i = 1, 0 ) /) non empty, yes or no?

IMO "empty array constructor" is ambiguous. One meaning is nothing but spaces between (/ and /) in the source code, another one is zero-sized rank-one array.

Dominique

PS I have wasted enough time on this patch. If you don’t accept it, close PR47040.
Steve Kargl April 9, 2016, 5:03 p.m. UTC | #9
On Sat, Apr 09, 2016 at 06:51:50PM +0200, Dominique d'Humières wrote:
> 
> > 
> > It is valid syntax because of 
> > 
> > "An empty sequence forms a zero-sized rank-one array."
> > 
> > It seems that J3 saw the error in their ways as (/ /) is clearly
> > an empty array constructor, and fixed the possibility of creating
> > a typeless zero-sized, rank-one array.
> 
> This is exactly the point of the patch! typeless zero-sized, rank-one array
> are not allowed, while typed zero-sized, rank-one array are.
> 

And, you're missing the point that your patch is suggesting
that one use (/ INTEGER :: /), which is clearly invalid
Fortran 95. 

> >>> Adding any text referring
> >>> to type-spec is wrong.
> >>> 
> >> 
> >> Are you considering (/ ( i, i = 1, 0 ) /) as non empty?
> >> 
> > 
> > It is a zero-sized rank-one array with type INTEGER.  
> 
> That is not the answer to the question.

I answered the question that was asked.  The crucial point
is that (/ (i,i=1,0) /) as a type!

> Is (/ ( i, i = 1, 0 ) /) non empty, yes or no?

That wasn't that originally asked question.

> IMO "empty array constructor" is ambiguous.

Yes, it is.  That is why J3 made changes in Fortran 2003.
Dominique d'Humières April 9, 2016, 6:42 p.m. UTC | #10
Patch withdrawn, PR47040 closed as INVALID.

Dominique
diff mbox

Patch

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(revision 234788)
+++ gcc/fortran/ChangeLog	(working copy)
@@ -1,3 +1,10 @@ 
+2016-04-06  Tobias Burnus  <burnus@net-b.de>
+	    Dominique d'Humieres <dominiq@lps.ens.fr>
+
+	PR fortran/47040
+	* array.c (gfc_match_array_constructor): add "without type-spec"
+	to the error message.
+
 2016-04-04  Andre Vehreschild  <vehre@gcc.gnu.org>
 
 	PR fortran/67538
Index: gcc/fortran/array.c
===================================================================
--- gcc/fortran/array.c	(revision 234788)
+++ gcc/fortran/array.c	(working copy)
@@ -1136,7 +1136,8 @@ 
 	goto done;
       else
 	{
-	  gfc_error ("Empty array constructor at %C is not allowed");
+	  gfc_error ("Empty array constructor without type-spec at %C "
+		     "is not allowed");
 	  goto cleanup;
 	}
     }
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 234788)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,7 @@ 
+2016-04-06  Dominique d'Humieres  <dominiq@lps.ens.fr>
+
+	* gfortran.dg/empty_constructor.f90: New test.
+
 2016-04-06  Eric Botcazou  <ebotcazou@adacore.com>
 
 	* gcc.c-torture/execute/20101011-1.c (__VISIUM__): Set DO_TEST to 0.
Index: gcc/testsuite/gfortran.dg/empty_constructor.f90
===================================================================
--- gcc/testsuite/gfortran.dg/empty_constructor.f90	(nonexistent)
+++ gcc/testsuite/gfortran.dg/empty_constructor.f90	(working copy)
@@ -0,0 +1,17 @@ 
+! { dg-do compile }
+! PR 47040
+! Contributed by Tobias Burnus <burnus@net-b.de>
+!
+program test_print 
+  implicit none
+  integer :: i 
+  call print( [ ] ) ! { dg-error "Empty array constructor without type-spec" }
+  call print( [integer :: ] )
+  call print( pack( [ 1 ], [ 2 ] == 3 ) )
+  call print( [ ( i, i = 1, 0 ) ] )
+contains 
+  subroutine print( array ) 
+    integer, dimension(:) :: array 
+    write(*,*) size(array) 
+  end subroutine print 
+end program test_print