diff mbox series

Fortran : Fill in missing array dimensions using the lower, bound (for review)

Message ID afd9adba-a5e7-e981-17aa-644c24344049@codethink.co.uk
State New
Headers show
Series Fortran : Fill in missing array dimensions using the lower, bound (for review) | expand

Commit Message

Mark Eggleston June 25, 2020, 7:22 a.m. UTC
Please find attached a proposed legacy extension to allow dimensions to 
be omitted when referring to an array element.  The declared lower bound 
being used by default.

This feature is known to exist in earlier compilers and is believed to 
be a DEC extension and is definitely supported by flang.  As it is 
non-standard the feature will only be enabled using 
-fdec-add-missing-indexes (and -fdec), a warning will be produced when 
used.  As warnings are commonly regarded as errors in build environments 
-Wno-missing-index has be provided to suppress the warning if needed.

Commit message:

Fortran  : Fill in missing array dimensions using the lower bound

Use -fdec-add-missing-indexes to enable feature. Also enabled by fdec.
A warning that the lower bound is being used for a mission dimension
is output unless suppressed by using -Wno-missing-index.

2020-06-25  Jim MacArthur <jim.macarthur@codethink.co.uk>
         Mark Eggleston  <markeggleston@gcc.gnu.org>

gcc/fortran/

     * lang.opt: Add options -Wmissing-index and
     -fdec-add-missing-indexes.
     * options.c (set_dec_flags): Add SET_BITFLAG for new option.
     * resolve.c (compare_spec_to_ref): If the flag is set and
     the number of dimension is less than the rank use the lower
     bound for the missing dimensions.

2020-06-25  Jim MacArthur <jim.macarthur@codethink.co.uk>
             Mark Eggleston <markeggleston@gcc.gnu.org>

gcc/testsuite/

     * gfortran.dg/array_6.f90: New test.
     * gfortran.dg/array_7.f90: New test.
     * gfortran.dg/array_8.f90: New test.
     * gfortran.dg/array_9.f90: New test.

Comments

Thomas Koenig June 27, 2020, 8:40 a.m. UTC | #1
Hi Mark,

> Use -fdec-add-missing-indexes to enable feature. Also enabled by fdec.
> A warning that the lower bound is being used for a mission dimension
> is output unless suppressed by using -Wno-missing-index.

This is... seriously problematic.  I forsee all sorts of not-so-funny
interactions with more modern features.

What do people actually do with this kind of code?  What kind of test
cases do you have that "work" with this?

And people would actually want to save a few keystrokes so they don't
have to write A(N,M,1) instead of A(N,M)?  And is this even the right
fix, how sure are you of the semantics; is there documentation for
this feature (maybe on Bitsavers)?  If not, this is not be done.

If this goes in at all, I want this rejected with any modern Fortran
feature, i.e. it should not be contain

- allocatable arrays
- coarrays
- pointers
- derived types
- CLASS
- assumed-shape arrays
- assumed-rank arrays (well, it probably doesn't make sense)
- KIND=4 characters
- as an argument to any of the array intrinsics like MATMUL,
   EOSHIFT, ...

but even with these restrictions, I will still take a lot of convincing
that this make sense.

Just imagine what will happen if people specify -fdec for some
relatively benign reason (for example because they want -fdec-math)
and start not finding bugs in their code because of this feature.

Best regards

	Thomas
Jerry D July 1, 2020, 10:01 p.m. UTC | #2
On 6/27/20 1:40 AM, Thomas Koenig via Fortran wrote:
> Hi Mark,
>
>> Use -fdec-add-missing-indexes to enable feature. Also enabled by fdec.
>> A warning that the lower bound is being used for a mission dimension
>> is output unless suppressed by using -Wno-missing-index.
>
> This is... seriously problematic.  I forsee all sorts of not-so-funny
> interactions with more modern features.
>
> What do people actually do with this kind of code?  What kind of test
> cases do you have that "work" with this?
>
> And people would actually want to save a few keystrokes so they don't
> have to write A(N,M,1) instead of A(N,M)?  And is this even the right
> fix, how sure are you of the semantics; is there documentation for
> this feature (maybe on Bitsavers)?  If not, this is not be done.
>
> If this goes in at all, I want this rejected with any modern Fortran
> feature, i.e. it should not be contain
>
> - allocatable arrays
> - coarrays
> - pointers
> - derived types
> - CLASS
> - assumed-shape arrays
> - assumed-rank arrays (well, it probably doesn't make sense)
> - KIND=4 characters
> - as an argument to any of the array intrinsics like MATMUL,
>   EOSHIFT, ...
>
> but even with these restrictions, I will still take a lot of convincing
> that this make sense.
>
> Just imagine what will happen if people specify -fdec for some
> relatively benign reason (for example because they want -fdec-math)
> and start not finding bugs in their code because of this feature.
>
> Best regards
>
>     Thomas

Please stop fixing problematic DEC programs by using the compiler as the 
pet tool. Use an editor or python or some suitable tool to initialize 
arrays properly.

I appreciate the effort, but need things run by here before the effort 
so you can spend the effort on really true compiler bugs, and not on the 
wishes of perhaps a paying customer.

We should never have caved on the previous DEC enhancement.

Just my humble opinion.

Jerry
Mark Eggleston July 2, 2020, 11:42 a.m. UTC | #3
On 01/07/2020 23:01, Jerry DeLisle wrote:
>
>
> On 6/27/20 1:40 AM, Thomas Koenig via Fortran wrote:
>> Hi Mark,
>>
>>> Use -fdec-add-missing-indexes to enable feature. Also enabled by fdec.
>>> A warning that the lower bound is being used for a mission dimension
>>> is output unless suppressed by using -Wno-missing-index.
>>
>> This is... seriously problematic.  I forsee all sorts of not-so-funny
>> interactions with more modern features.
>>
>> What do people actually do with this kind of code?  What kind of test
>> cases do you have that "work" with this?
>>
>> And people would actually want to save a few keystrokes so they don't
>> have to write A(N,M,1) instead of A(N,M)?  And is this even the right
>> fix, how sure are you of the semantics; is there documentation for
>> this feature (maybe on Bitsavers)?  If not, this is not be done.
>>
>> If this goes in at all, I want this rejected with any modern Fortran
>> feature, i.e. it should not be contain
>>
>> - allocatable arrays
>> - coarrays
>> - pointers
>> - derived types
>> - CLASS
>> - assumed-shape arrays
>> - assumed-rank arrays (well, it probably doesn't make sense)
>> - KIND=4 characters
>> - as an argument to any of the array intrinsics like MATMUL,
>>   EOSHIFT, ...
>>
>> but even with these restrictions, I will still take a lot of convincing
>> that this make sense.
>>
>> Just imagine what will happen if people specify -fdec for some
>> relatively benign reason (for example because they want -fdec-math)
>> and start not finding bugs in their code because of this feature.
>>
>> Best regards
>>
>>     Thomas
>
> Please stop fixing problematic DEC programs by using the compiler as 
> the pet tool. Use an editor or python or some suitable tool to 
> initialize arrays properly.

Agreed, I was tasked with upstreaming various legacy patches, 
fortunately there has been a parallel effort refactor Fortran code to 
reduce the need for these patches.  It is likely that this one is no 
longer required.  There is only one more is left, hopefully that will 
also be addressed in the code.  I'll relay your opinion as an argument 
for not attempting the last patch and I can address the many Fortran PRs 
instead.

regards,

Mark

>
> I appreciate the effort, but need things run by here before the effort 
> so you can spend the effort on really true compiler bugs, and not on 
> the wishes of perhaps a paying customer.
>
> We should never have caved on the previous DEC enhancement.
>
> Just my humble opinion.
>
> Jerry
>
diff mbox series

Patch

From 3ab6576ec34dd14cac10a52a1a7e08c176ebc357 Mon Sep 17 00:00:00 2001
From: Mark Eggleston <markeggleston@gcc.gnu.org>
Date: Mon, 3 Feb 2020 10:56:36 +0000
Subject: [PATCH] Fortran  : Fill in missing array dimensions using the lower
 bound

Use -fdec-add-missing-indexes to enable feature. Also enabled by fdec.
A warning that the lower bound is being used for a mission dimension
is output unless suppressed by using -Wno-missing-index.

2020-06-24  Jim MacArthur  <jim.macarthur@codethink.co.uk>
	    Mark Eggleston  <markeggleston@gcc.gnu.org>

gcc/fortran/

	* lang.opt: Add options -Wmissing-index and
	-fdec-add-missing-indexes.
	* options.c (set_dec_flags): Add SET_BITFLAG for new option.
	* resolve.c (compare_spec_to_ref): If the flag is set and
	the number of dimension is less than the rank use the lower
	bound for the missing dimensions.

2020-06-24  Jim MacArthur  <jim.macarthur@codethink.co.uk>
            Mark Eggleston  <markeggleston@gcc.gnu.org>

gcc/testsuite/

	* gfortran.dg/array_6.f90: New test.
        * gfortran.dg/array_7.f90: New test.
        * gfortran.dg/array_8.f90: New test.
	* gfortran.dg/array_9.f90: New test.
---
 gcc/fortran/invoke.texi               | 40 +++++++++++++++++++++++------------
 gcc/fortran/lang.opt                  |  8 +++++++
 gcc/fortran/options.c                 |  1 +
 gcc/fortran/resolve.c                 | 20 ++++++++++++++++++
 gcc/testsuite/gfortran.dg/array_6.f90 | 24 +++++++++++++++++++++
 gcc/testsuite/gfortran.dg/array_7.f90 | 24 +++++++++++++++++++++
 gcc/testsuite/gfortran.dg/array_8.f90 | 24 +++++++++++++++++++++
 gcc/testsuite/gfortran.dg/array_9.f90 | 24 +++++++++++++++++++++
 8 files changed, 151 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/array_6.f90
 create mode 100644 gcc/testsuite/gfortran.dg/array_7.f90
 create mode 100644 gcc/testsuite/gfortran.dg/array_8.f90
 create mode 100644 gcc/testsuite/gfortran.dg/array_9.f90

diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi
index 052d3178244..989011af868 100644
--- a/gcc/fortran/invoke.texi
+++ b/gcc/fortran/invoke.texi
@@ -118,16 +118,17 @@  by type.  Explanations are in the following sections.
 @xref{Fortran Dialect Options,,Options controlling Fortran dialect}.
 @gccoptlist{-fall-intrinsics -fallow-argument-mismatch -fallow-invalid-boz @gol
 -fbackslash -fcray-pointer -fd-lines-as-code -fd-lines-as-comments @gol
--fdec -fdec-char-conversions -fdec-structure -fdec-intrinsic-ints @gol
--fdec-static -fdec-math -fdec-include -fdec-format-defaults @gol
--fdec-blank-format-item -fdefault-double-8 -fdefault-integer-8 @gol
--fdefault-real-8 -fdefault-real-10 -fdefault-real-16 -fdollar-ok @gol
--ffixed-line-length-@var{n} -ffixed-line-length-none -fpad-source @gol
--ffree-form -ffree-line-length-@var{n} -ffree-line-length-none @gol
--fimplicit-none -finteger-4-integer-8 -fmax-identifier-length @gol
--fmodule-private -ffixed-form -fno-range-check -fopenacc -fopenmp @gol
--freal-4-real-10 -freal-4-real-16 -freal-4-real-8 -freal-8-real-10 @gol
--freal-8-real-16 -freal-8-real-4 -std=@var{std} -ftest-forall-temp
+-fdec -fdec-add-missing-indexes -fdec-char-conversions -fdec-structure @gol
+-fdec-intrinsic-ints -fdec-static -fdec-math -fdec-include @gol
+-fdec-format-defaults -fdec-blank-format-item -fdefault-double-8 @gol
+-fdefault-integer-8 -fdefault-real-8 -fdefault-real-10 -fdefault-real-16 @gol
+-fdollar-ok -ffixed-line-length-@var{n} -ffixed-line-length-none @gol
+-fpad-source -ffree-form -ffree-line-length-@var{n} @gol
+-ffree-line-length-none -fimplicit-none -finteger-4-integer-8 @gol
+-fmax-identifier-length -fmodule-private -ffixed-form -fno-range-check @gol
+-fopenacc -fopenmp -freal-4-real-10 -freal-4-real-16 -freal-4-real-8 @gol
+-freal-8-real-10 -freal-8-real-16 -freal-8-real-4 -std=@var{std} @gol
+-ftest-forall-temp
 }
 
 @item Preprocessing Options
@@ -149,7 +150,7 @@  and warnings}.
 -Wc-binding-type -Wcharacter-truncation -Wconversion @gol
 -Wdo-subscript -Wfunction-elimination -Wimplicit-interface @gol
 -Wimplicit-procedure -Wintrinsic-shadow -Wuse-without-only @gol
--Wintrinsics-std -Wline-truncation -Wno-align-commons @gol
+-Wintrinsics-std -Wline-truncation -Wno-align-commons -Wno-missing-index@gol
 -Wno-overwrite-recursive -Wno-tabs -Wreal-q-constant -Wsurprising @gol
 -Wunderflow -Wunused-parameter -Wrealloc-lhs -Wrealloc-lhs-all @gol
 -Wfrontend-loop-interchange -Wtarget-lifetime -fmax-errors=@var{n} @gol
@@ -274,14 +275,20 @@  For details on GNU Fortran's implementation of these extensions see the
 full documentation.
 
 Other flags enabled by this switch are:
-@option{-fdollar-ok} @option{-fcray-pointer} @option{-fdec-char-conversions}
-@option{-fdec-structure} @option{-fdec-intrinsic-ints} @option{-fdec-static}
-@option{-fdec-math} @option{-fdec-include} @option{-fdec-blank-format-item}
+@option{-fdollar-ok} @option{-fcray-pointer} @option{-fdec-add-missing-indexes}
+@option{-fdec-char-conversions} @option{-fdec-structure} 
+@option{-fdec-intrinsic-ints} @option{-fdec-static} @option{-fdec-math}
+@option{-fdec-include} @option{-fdec-blank-format-item}
 @option{-fdec-format-defaults}
 
 If @option{-fd-lines-as-code}/@option{-fd-lines-as-comments} are unset, then
 @option{-fdec} also sets @option{-fd-lines-as-comments}.
 
+@item -fdec-add-missing-indexes
+@opindex @code{fdec-add-missing-indexes}
+Enable the use of the lower bound of an index when there are fewer indexes
+than the rank of the variable.
+
 @item -fdec-char-conversions
 @opindex @code{fdec-char-conversions}
 Enable the use of character literals in assignments and @code{DATA} statements
@@ -1005,6 +1012,11 @@  it as @code{EXTERNAL} procedure because of this.  @option{-fall-intrinsics} can
 be used to never trigger this behavior and always link to the intrinsic
 regardless of the selected standard.
 
+@item -Wno-missing-index
+@opindex @code{Wmissing-index}
+@cindex warnings
+Do not warn when the lower bound is used as the index of an unspecified dimension.
+ 
 @item -Wno-overwrite-recursive
 @opindex @code{Woverwrite-recursive}
 @cindex  warnings, overwrite recursive
diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
index da4b1aa879a..3885f676dda 100644
--- a/gcc/fortran/lang.opt
+++ b/gcc/fortran/lang.opt
@@ -281,6 +281,10 @@  Wmissing-include-dirs
 Fortran
 ; Documented in C/C++
 
+Wmissing-index
+Fortran Var(warn_missing_index) Warning Init(1)
+Warn that the lower bound of a missing index will be used.
+
 Wuse-without-only
 Fortran Var(warn_use_without_only) Warning
 Warn about USE statements that have no ONLY qualifier.
@@ -456,6 +460,10 @@  fdec
 Fortran Var(flag_dec)
 Enable all DEC language extensions.
 
+fdec-add-missing-indexes
+Fortran Var(flag_dec_add_missing_indexes)
+Enable the addition of missing indexes using their lower bounds.
+
 fdec-blank-format-item
 Fortran Var(flag_dec_blank_format_item)
 Enable the use of blank format items in format strings.
diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c
index d844fa93115..5c660621a5e 100644
--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -77,6 +77,7 @@  set_dec_flags (int value)
   SET_BITFLAG (flag_dec_format_defaults, value, value);
   SET_BITFLAG (flag_dec_blank_format_item, value, value);
   SET_BITFLAG (flag_dec_char_conversions, value, value);
+  SET_BITFLAG (flag_dec_add_missing_indexes, value, value);
 }
 
 /* Finalize DEC flags.  */
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index c53b312f7ed..7ce5741385d 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -4711,6 +4711,26 @@  compare_spec_to_ref (gfc_array_ref *ar)
   if (ar->type == AR_FULL)
     return true;
 
+  if (flag_dec_add_missing_indexes && as->rank > ar->dimen)
+    {
+      /* Add in the missing dimensions, assuming they are the lower bound
+         of that dimension if not specified.  */
+      gfc_warning (OPT_Wmissing_index, "Using the lower bound for "
+		   "unspecified dimensions in array reference at %L",
+		   &ar->where);
+      /* Other parts of the code iterate ar->start and ar->end from 0 to
+	 ar->dimen, so it is safe to assume slots from ar->dimen upwards
+	 are unused (i.e. there are no gaps; the specified indexes are
+	 contiguous and start at zero.  */
+      for(int j = ar->dimen; j <= as->rank; j++)
+        {
+	  ar->start[j] = gfc_copy_expr (as->lower[j]);
+	  ar->end[j]   = gfc_copy_expr (as->lower[j]);
+	  ar->dimen_type[j] = DIMEN_ELEMENT;
+        }
+      ar->dimen = as->rank;
+    }
+
   if (as->rank != ar->dimen)
     {
       gfc_error ("Rank mismatch in array reference at %L (%d/%d)",
diff --git a/gcc/testsuite/gfortran.dg/array_6.f90 b/gcc/testsuite/gfortran.dg/array_6.f90
new file mode 100644
index 00000000000..73a0b627c32
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/array_6.f90
@@ -0,0 +1,24 @@ 
+! { dg-do run }
+! { dg-options "-fdec" }
+!
+! Checks that under-specified arrays (referencing arrays with fewer
+! dimensions than the array spec) generates a warning.
+!
+! Contributed by Jim MacArthur <jim.macarthur@codethink.co.uk>
+! Updated by Mark Eggleston <mark.eggleston@codethink.co.uk>
+!
+
+program under_specified_array
+    integer chessboard(8,8)
+    integer chessboard3d(8,8,3:5)
+    chessboard(3,1) = 5
+    chessboard(3,2) = 55
+    chessboard3d(4,1,3) = 6
+    chessboard3d(4,1,4) = 66
+    chessboard3d(4,4,3) = 7
+    chessboard3d(4,4,4) = 77
+  
+    if (chessboard(3).ne.5) stop 1  ! { dg-warning "Using the lower bound for unspecified dimensions in array reference" }
+    if (chessboard3d(4).ne.6) stop 2  ! { dg-warning "Using the lower bound for unspecified dimensions in array reference" }
+    if (chessboard3d(4,4).ne.7) stop 3  ! { dg-warning "Using the lower bound for unspecified dimensions in array reference" }
+end program
diff --git a/gcc/testsuite/gfortran.dg/array_7.f90 b/gcc/testsuite/gfortran.dg/array_7.f90
new file mode 100644
index 00000000000..924057c8ac2
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/array_7.f90
@@ -0,0 +1,24 @@ 
+! { dg-do run }
+! { dg-options "-fdec-add-missing-indexes" }
+!
+! Checks that under-specified arrays (referencing arrays with fewer
+! dimensions than the array spec) generates a warning.
+!
+! Contributed by Jim MacArthur <jim.macarthur@codethink.co.uk>
+! Updated by Mark Eggleston <mark.eggleston@codethink.co.uk>
+!
+
+program under_specified_array
+    integer chessboard(8,8)
+    integer chessboard3d(8,8,3:5)
+    chessboard(3,1) = 5
+    chessboard(3,2) = 55
+    chessboard3d(4,1,3) = 6
+    chessboard3d(4,1,4) = 66
+    chessboard3d(4,4,3) = 7
+    chessboard3d(4,4,4) = 77
+  
+    if (chessboard(3).ne.5) stop 1  ! { dg-warning "Using the lower bound for unspecified dimensions in array reference" }
+    if (chessboard3d(4).ne.6) stop 2  ! { dg-warning "Using the lower bound for unspecified dimensions in array reference" }
+    if (chessboard3d(4,4).ne.7) stop 3  ! { dg-warning "Using the lower bound for unspecified dimensions in array reference" }
+end program
diff --git a/gcc/testsuite/gfortran.dg/array_8.f90 b/gcc/testsuite/gfortran.dg/array_8.f90
new file mode 100644
index 00000000000..66573a0a474
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/array_8.f90
@@ -0,0 +1,24 @@ 
+! { dg-do compile }
+! { dg-options "-fdec -fno-dec-add-missing-indexes" }
+!
+! Checks that under-specified arrays (referencing arrays with fewer
+! dimensions than the array spec) generates a warning.
+!
+! Contributed by Jim MacArthur <jim.macarthur@codethink.co.uk>
+! Updated by Mark Eggleston <mark.eggleston@codethink.co.uk>
+!
+
+program under_specified_array
+    integer chessboard(8,8)
+    integer chessboard3d(8,8,3:5)
+    chessboard(3,1) = 5
+    chessboard(3,2) = 55
+    chessboard3d(4,1,3) = 6
+    chessboard3d(4,1,4) = 66
+    chessboard3d(4,4,3) = 7
+    chessboard3d(4,4,4) = 77
+  
+    if (chessboard(3).ne.5) stop 1  ! { dg-error "Rank mismatch" }
+    if (chessboard3d(4).ne.6) stop 2  ! { dg-error "Rank mismatch" }
+    if (chessboard3d(4,4).ne.7) stop 3  ! { dg-error "Rank mismatch" }
+end program
diff --git a/gcc/testsuite/gfortran.dg/array_9.f90 b/gcc/testsuite/gfortran.dg/array_9.f90
new file mode 100644
index 00000000000..17cce5347ed
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/array_9.f90
@@ -0,0 +1,24 @@ 
+! { dg-do run }
+! { dg-options "-fdec -Wno-missing-index" }
+!
+! Checks that under-specified arrays (referencing arrays with fewer
+! dimensions than the array spec) generates a warning.
+!
+! Contributed by Jim MacArthur <jim.macarthur@codethink.co.uk>
+! Updated by Mark Eggleston <mark.eggleston@codethink.co.uk>
+!
+
+program under_specified_array
+    integer chessboard(8,8)
+    integer chessboard3d(8,8,3:5)
+    chessboard(3,1) = 5
+    chessboard(3,2) = 55
+    chessboard3d(4,1,3) = 6
+    chessboard3d(4,1,4) = 66
+    chessboard3d(4,4,3) = 7
+    chessboard3d(4,4,4) = 77
+  
+    if (chessboard(3).ne.5) stop 1
+    if (chessboard3d(4).ne.6) stop 2
+    if (chessboard3d(4,4).ne.7) stop 3
+end program
-- 
2.11.0