Re: *ping* Re: [PATCH] PR fortran/89103 - Allow blank format items in format strings
I will see if I can get this one. On 6/17/19 6:37 AM, Mark Eggleston wrote: On 12/06/2019 19:11, Steve Kargl wrote: On Tue, Jun 11, 2019 at 11:50:40AM +0200, Jakub Jelinek wrote: On Tue, Jun 11, 2019 at 10:30:59AM +0100, Mark Eggleston wrote: Jim MacArthur Mark Eggleston Two spaces before < instead of one. This is not a patch review, just comments: Mark, do you plan to address any of Jakub's comments. Do note, I just 'OK' Jakub's patch that uses G_() forms for the strings. Now that Jakubs's patch has been committed, please find attached an updated patch and updated change logs: gcc/fortran Jim MacArthur Mark Eggleston PR fortran/89103 * gfortran.texi: Add -fdec-blank-format-item * invoke.texi: Add option to list of options. * invoke.texi: Add to section on Commas in FORMAT specifications. * io.c (check_format): At FMT_RPAREN goto finished if -fdec-blank-format-item otherwise set error string. * lang.opt: Add new option. * options.c (set_dec_flags): Add SET_BITFLAG for flag_dec_format_defaults. gcc/testsuite Jim MacArthur Mark Eggleston PR fortran/89103 * gfortran.dg/dec_format_empty_item_1.f: New test. * gfortran.dg/dec_format_empty_item_2.f: New test. * gfortran.dg/dec_format_empty_item_3.f: New test. as before... Please can someone commit this as do not have commit rights. Also, do you have plans to contribute additional patches (either for -fdec* extensions or preferrably to help with bug fixes and new features)? It may be advantageous for you to get a commit bit. Yes, I do intend to contribute additional patches, mostly -fdec- patches, there are also some patches unrelated to -fdec* extensions.
Re: *ping* Re: [PATCH] PR fortran/89103 - Allow blank format items in format strings
On 12/06/2019 19:11, Steve Kargl wrote:
On Tue, Jun 11, 2019 at 11:50:40AM +0200, Jakub Jelinek wrote:
On Tue, Jun 11, 2019 at 10:30:59AM +0100, Mark Eggleston wrote:
Jim MacArthur
Mark Eggleston
Two spaces before < instead of one.
This is not a patch review, just comments:
Mark, do you plan to address any of Jakub's comments.
Do note, I just 'OK' Jakub's patch that uses G_()
forms for the strings.
Now that Jakubs's patch has been committed, please find attached an
updated patch and updated change logs:
gcc/fortran
Jim MacArthur
Mark Eggleston
PR fortran/89103
* gfortran.texi: Add -fdec-blank-format-item
* invoke.texi: Add option to list of options.
* invoke.texi: Add to section on Commas in FORMAT specifications.
* io.c (check_format): At FMT_RPAREN goto finished if
-fdec-blank-format-item otherwise set error string.
* lang.opt: Add new option.
* options.c (set_dec_flags): Add SET_BITFLAG for
flag_dec_format_defaults.
gcc/testsuite
Jim MacArthur
Mark Eggleston
PR fortran/89103
* gfortran.dg/dec_format_empty_item_1.f: New test.
* gfortran.dg/dec_format_empty_item_2.f: New test.
* gfortran.dg/dec_format_empty_item_3.f: New test.
as before... Please can someone commit this as do not have commit rights.
Also, do you have plans to contribute additional
patches (either for -fdec* extensions or preferrably
to help with bug fixes and new features)? It may be
advantageous for you to get a commit bit.
Yes, I do intend to contribute additional patches, mostly -fdec-
patches, there are also some patches unrelated to -fdec* extensions.
--
https://www.codethink.co.uk/privacy.html
>From 48c734966d0f5d9f618b532d12b24fe784679dea Mon Sep 17 00:00:00 2001
From: Jim MacArthur
Date: Thu, 4 Feb 2016 16:59:41 +
Subject: [PATCH 01/10] Allow blank format items in format strings
Use -fdec-blank-format-item to enable. Also enabled by -fdec.
---
gcc/fortran/gfortran.texi | 7 ++-
gcc/fortran/invoke.texi | 13 +
gcc/fortran/io.c| 9 +
gcc/fortran/lang.opt| 4
gcc/fortran/options.c | 1 +
gcc/testsuite/gfortran.dg/dec_format_empty_item_1.f | 19 +++
gcc/testsuite/gfortran.dg/dec_format_empty_item_2.f | 19 +++
gcc/testsuite/gfortran.dg/dec_format_empty_item_3.f | 19 +++
8 files changed, 86 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/dec_format_empty_item_1.f
create mode 100644 gcc/testsuite/gfortran.dg/dec_format_empty_item_2.f
create mode 100644 gcc/testsuite/gfortran.dg/dec_format_empty_item_3.f
diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
index 57461e0e42f..c887e7d1a42 100644
--- a/gcc/fortran/gfortran.texi
+++ b/gcc/fortran/gfortran.texi
@@ -1761,11 +1761,16 @@ When omitted, the count is implicitly assumed to be one.
To support legacy codes, GNU Fortran allows the comma separator
to be omitted immediately before and after character string edit
-descriptors in @code{FORMAT} statements.
+descriptors in @code{FORMAT} statements. A comma with no following format
+decriptor is permited if the @option{-fdec-blank-format-item} is given on
+the command line. This is considered non-conforming code and is
+discouraged.
@smallexample
PRINT 10, 2, 3
10 FORMAT ('FOO='I1' BAR='I2)
+ print 20, 5, 6
+20 FORMAT (I3, I3,)
@end smallexample
diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi
index 2e2cb5b2728..2b08ac4de22 100644
--- a/gcc/fortran/invoke.texi
+++ b/gcc/fortran/invoke.texi
@@ -119,10 +119,10 @@ by type. Explanations are in the following sections.
@gccoptlist{-fall-intrinsics -fbackslash -fcray-pointer -fd-lines-as-code @gol
-fd-lines-as-comments -fdec -fdec-structure -fdec-intrinsic-ints @gol
-fdec-static -fdec-math -fdec-include -fdec-format-defaults @gol
--fdefault-double-8 -fdefault-integer-8 -fdefault-real-8 -fdefault-real-10 @gol
--fdefault-real-16 -fdollar-ok -ffixed-line-length-@var{n} @gol
--ffixed-line-length-none -fpad-source -ffree-form @gol
--ffree-line-length-@var{n} -ffree-line-length-none @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
@@ -289,6 +289,11 @@ be on a single line and can use line continuations.
Enable format specifiers F, G and I to be used without width specifiers,
default widths will be used instead.
+@
Re: *ping* Re: [PATCH] PR fortran/89103 - Allow blank format items in format strings
On Tue, Jun 11, 2019 at 11:50:40AM +0200, Jakub Jelinek wrote: > On Tue, Jun 11, 2019 at 10:30:59AM +0100, Mark Eggleston wrote: > > Jim MacArthur > > Mark Eggleston > > Two spaces before < instead of one. > > This is not a patch review, just comments: Mark, do you plan to address any of Jakub's comments. Do note, I just 'OK' Jakub's patch that uses G_() forms for the strings. Also, do you have plans to contribute additional patches (either for -fdec* extensions or preferrably to help with bug fixes and new features)? It may be advantageous for you to get a commit bit. -- Steve
Re: *ping* Re: [PATCH] PR fortran/89103 - Allow blank format items in format strings
On Tue, Jun 11, 2019 at 10:30:59AM +0100, Mark Eggleston wrote:
> Jim MacArthur
> Mark Eggleston
Two spaces before < instead of one.
This is not a patch review, just comments:
> This has to be written in a slightly verbose manner because GCC 7
> defaults to building with -Werror=implicit-fallthrough which prevents
> us from just falling through to the default: case.
That is not true, one can fall through just fine, just there needs to be a
comment or attribute or builtin that says so.
> --- a/gcc/fortran/io.c
> +++ b/gcc/fortran/io.c
> @@ -598,6 +598,7 @@ check_format (bool is_input)
> {
>const char *posint_required = _("Positive width required");
>const char *nonneg_required = _("Nonnegative width required");
> + const char *missing_item = _("Missing item");
>const char *unexpected_element = _("Unexpected element %qc in format "
> "string at %L");
>const char *unexpected_end = _("Unexpected end of format string");
> @@ -756,6 +757,16 @@ format_item_1:
>error = unexpected_end;
>goto syntax;
>
> +case FMT_RPAREN:
> + /* Oracle allows a blank format item. */
> + if (flag_dec_blank_format_item)
> + goto finished;
> + else
> + {
> + error = missing_item;
> + goto syntax;
> + }
So, if you want to fall thru, just do:
case FMT_RPAREN:
if (flag_dec_blank_format_item)
goto finished;
/* FALLTHRU */
> +
> default:
>error = unexpected_element;
>goto syntax;
and that is it. Not sure I'd mention the Oracle fortran compiler there,
either it is common to other DEC based compilers too (DEC fortran, ifort,
...) and then the comment makes no sense, or it might not be best to call
the flag -fdec-whatever.
Furthermore, even if you want to have a _("Missing item"), you should write
it as error = _("Missing item");, not the way it is written, as that way it
is inefficient at compile time.
The rest is just a general comment on the preexisting code.
Using a bunch of const char *whatever = _("...");
at the start of function is undesirable, it means any time this function is
called, even in the likely case there is no error, all those strings need to
be translated. It would be better to e.g. replace all _("...") in that
function with G_("...") (i.e. mark for translation, but don't translate),
and only when actually using that translate:
if (error == unexpected_element)
gfc_error (error, error_element, &format_locus);
else
gfc_error ("%s in format string at %L", error, &format_locus);
The first case is translated already by gfc_error, the second one would need
_(error) instead of error (but is generally wrong anyway, because you are
constructing a diagnostics from two pieces which might not be ok for
translations. So, likely you want to append " in format string at %L" to
all the error string literals inside of G_("...") and just pass error as
first argument to gfc_error.
Jakub
Re: *ping* Re: [PATCH] PR fortran/89103 - Allow blank format items in format strings
On 10/06/2019 15:10, Mark Eggleston wrote:
On 10/06/2019 15:07, Thomas Koenig wrote:
Am 10.06.19 um 15:33 schrieb Mark Eggleston:
This patch is for a customer that has a huge codebase and that is
the only reason for its existence.
I didn't know gfortran as a whole has customers as such :-)
The error message has not changed. Making it more informative,
indicating an option that will allow this non-standard usage, is a
bad idea as it could result in its spread.
OK, I understand that. So scrap the idea of pointing towards the
option.
However, making it more informative _without_ pointing towards that
option is still a good idea (such as "missing item in format list").
That's a much better idea. I'll implement that and when it's ready
I'll update the patch.
Patch updated and attached. ChangeLogs:
gcc/fortran
Jim MacArthur
Mark Eggleston
PR fortran/89103
* gfortran.texi: Add -fdec-blank-format-item
* invoke.texi: Add to section on Commas in FORMAT specifications.
* io.c (check_format): Add new string missing_item.
* io.c (check_format): At FMT_RPAREN goto finished if
-fdec-blank-format-item otherwise set error string to missing_item.
* lang.opt: Add new option.
* options.c (set_dec_flags): Add SET_BITFLAG for
flag_dec_format_defaults.
Jim MacArthur
Mark Eggleston
gcc/testsuite
PR fortran/89103
* gfortran.dg/dec_format_empty_item_1.f: New test.
* gfortran.dg/dec_format_empty_item_2.f: New test.
* gfortran.dg/dec_format_empty_item_3.f: New test.
If OK, please can someone commit this.
If we want to allow legacy extensions to clutter our code, getting
a more informative error message is something we can get in return,
at least.
Regards
Thomas
--
https://www.codethink.co.uk/privacy.html
>From 5a78d94a626444d4f71bcda99c5fc6ebb0509f46 Mon Sep 17 00:00:00 2001
From: Jim MacArthur
Date: Thu, 4 Feb 2016 16:59:41 +
Subject: [PATCH 01/10] Allow blank format items in format strings
This has to be written in a slightly verbose manner because GCC 7
defaults to building with -Werror=implicit-fallthrough which prevents
us from just falling through to the default: case.
Test written by: Francisco Redondo Marchena
Use -fdec-blank-format-item to enable. Also enabled by -fdec.
---
gcc/fortran/gfortran.texi | 7 ++-
gcc/fortran/invoke.texi | 13 +
gcc/fortran/io.c| 11 +++
gcc/fortran/lang.opt| 4
gcc/fortran/options.c | 1 +
gcc/testsuite/gfortran.dg/dec_format_empty_item_1.f | 19 +++
gcc/testsuite/gfortran.dg/dec_format_empty_item_2.f | 19 +++
gcc/testsuite/gfortran.dg/dec_format_empty_item_3.f | 19 +++
8 files changed, 88 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/dec_format_empty_item_1.f
create mode 100644 gcc/testsuite/gfortran.dg/dec_format_empty_item_2.f
create mode 100644 gcc/testsuite/gfortran.dg/dec_format_empty_item_3.f
diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
index 57461e0e42f..c887e7d1a42 100644
--- a/gcc/fortran/gfortran.texi
+++ b/gcc/fortran/gfortran.texi
@@ -1761,11 +1761,16 @@ When omitted, the count is implicitly assumed to be one.
To support legacy codes, GNU Fortran allows the comma separator
to be omitted immediately before and after character string edit
-descriptors in @code{FORMAT} statements.
+descriptors in @code{FORMAT} statements. A comma with no following format
+decriptor is permited if the @option{-fdec-blank-format-item} is given on
+the command line. This is considered non-conforming code and is
+discouraged.
@smallexample
PRINT 10, 2, 3
10 FORMAT ('FOO='I1' BAR='I2)
+ print 20, 5, 6
+20 FORMAT (I3, I3,)
@end smallexample
diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi
index 2e2cb5b2728..2b08ac4de22 100644
--- a/gcc/fortran/invoke.texi
+++ b/gcc/fortran/invoke.texi
@@ -119,10 +119,10 @@ by type. Explanations are in the following sections.
@gccoptlist{-fall-intrinsics -fbackslash -fcray-pointer -fd-lines-as-code @gol
-fd-lines-as-comments -fdec -fdec-structure -fdec-intrinsic-ints @gol
-fdec-static -fdec-math -fdec-include -fdec-format-defaults @gol
--fdefault-double-8 -fdefault-integer-8 -fdefault-real-8 -fdefault-real-10 @gol
--fdefault-real-16 -fdollar-ok -ffixed-line-length-@var{n} @gol
--ffixed-line-length-none -fpad-source -ffree-form @gol
--ffree-line-length-@var{n} -ffree-line-length-none @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-leng
