Re: [Patch, Fortran + wwwdocs] PR93253 – Document BOZ changes, make it friendlier in legacy code

[Steve Kargl <sgk@troutmask.apl.washington.edu>]

On Wed, Jan 15, 2020 at 05:37:11PM +0100, Tobias Burnus wrote:
> Fortran before 2013 only allowed binary-octal-hex values (BOZ literals) 
> in DATA statements; Fortran 2013 extended it to support them also as 
> argument to INT(), REAL() etc. — Additionally, various compilers 
> (including gfortran) support more as (legacy) extension.
> 
> Diagnostic: DATA vs. non DATA usage was/is diagnosed with -std=f95 
> ("Error: Fortran 2003: …"). Using "X" instead of "Z" for hexadecimals 
> was/is diagnosed (accepted: before GCC 10 with -std=legacy, now with 
> -fallow-invalid-boz).
> 
> However, GCC 9 and older silently accepted nonstandard use, e.g. "print 
> *, Z'A'" – even with -std=f2003/f2008! Since GCC 10, it is now rejected 
> unless -fallow-invalid-boz is used.

It was much worse than your simple example suggests.  My rework fixed
a poor design decision I made some 15 years ago.  Prior to the BOZ rework,
a BOZ could appear anywhere an INTEGER(8) or INTEGER(16) could appear
(kind=8 or 16 is target dependent!).  This allowed things like

% gfortran9 -o z a.f90
program foo
   real a(100), b(4)
   a = [(i,i=0,99)]
   print *, a(z'2f')              ! BOZ index?
   a = a * b'11' + o'7'           ! BOZ in an expression?
   print *, a(z'2f')              ! BOZ index?
   b = [z'0', z'1', z'2', z'3']   ! BOZ in an array construct?
   print '(4(F8.3))', b
end 

Note, the above requires INTEGER(8/16) conversion to default integer
kind for the array index reference, and INTEGER(8/16) to default
real kind for assignment.  Yes, those conversions are folded, but this
is requiring the compiler to do unneeded work.  Also note, that in
F08 and F18, a BOZ is a typeless and kindless entity.  Why should
'a*b'11' treat the BOZ as an integer.  Why not convert it to type
of 'a' (using the rules of F08/18) and then do the multiplication?  

% ./z
   46.0000000
   145.000000
   0.000   1.000   2.000   3.000

All of the above use cases are invalid.  gfortran now rejects the
above code, and more importantly, -fallow-invalid-boz *DOES NOT*
allow these uses cases.

% gfcx -o z -fallow-invalid-boz a.f90
a.f90:4:20:

    4 |    print *, a(z'2f')              ! BOZ index?
      |                    1
Error: Invalid BOZ literal constant used in subscript at (1)
a.f90:6:20:

    6 |    print *, a(z'2f')              ! BOZ index?
      |                    1
Error: Invalid BOZ literal constant used in subscript at (1)
a.f90:7:8:

    7 |    b = [z'0', z'1', z'2', z'3']   ! BOZ in an array construct?
      |        1
Error: BOZ literal constant at (1) cannot appear in an array constructor
a.f90:5:7:

    5 |    a = a * b'11' + o'7'           ! BOZ in an expression?
      |       1
Error: Operands of binary numeric operator '*' at (1) are REAL(4)/BOZ

A side effect of the BOZ rework is that one no longer needs to use
-fno-range-check for things like Inf and NaN and negative integers.

Consider

program foo
   real :: inf = z'7f800000'    ! Extension:  BOZ in initialization expr.
   real :: nan = z'ffc00000'    ! Extension:  BOZ in initialization expr.
   integer :: neg = z'81111111' ! Extension:  BOZ in initialization expr.
   real b
   print '(Z8.8,1X,F8.3)', inf, inf
   print '(Z8.8,1X,F8.3)', nan, nan
   print '(Z8.8,1X,I0)', neg, neg
end

% gfortran9 -o z a.f90
a.f90:2:16:

    2 |    real :: inf = z'7f800000'  ! Extension:  BOZ in initialization expr.
      |                1
Error: Arithmetic overflow of bit-wise transferred BOZ at (1). This check can be disabled with the option '-fno-range-check'
a.f90:3:16:

    3 |    real :: nan = z'ffc00000'  ! Extension:  BOZ in initialization expr.
      |                1
Error: Arithmetic NaN of bit-wise transferred BOZ at (1). This check can be disabled with the option '-fno-range-check'
a.f90:4:19:

    4 |    integer :: neg = z'81111111'
      |                   1
Error: Arithmetic overflow converting INTEGER(8) to INTEGER(4) at (1). This check can be disabled with the option '-fno-range-check'

% gfortran9 -o z -fno-range-check a.f90
 ./z
7F800000 Infinity
7FC00000      NaN
81111111 -2129587951

There is no warning about extension!

Because of the extension for a BOZ in an initialization expression, one
needs the new -fallow-invalid-boz.

 gfcx -o z -fallow-invalid-boz a.f90
a.f90:2:16:

    2 |    real :: inf = z'7f800000'  ! Extension:  BOZ in initialization expr.
      |                1
Warning: BOZ literal constant at (1) is neither a data-stmt-constant nor an actual argument to INT, REAL, DBLE, or CMPLX intrinsic function
a.f90:3:16:

    3 |    real :: nan = z'ffc00000'  ! Extension:  BOZ in initialization expr.
      |                1
Warning: BOZ literal constant at (1) is neither a data-stmt-constant nor an actual argument to INT, REAL, DBLE, or CMPLX intrinsic function
a.f90:4:19:

    4 |    integer :: neg = z'81111111'
      |                   1
Warning: BOZ literal constant at (1) is neither a data-stmt-constant nor an actual argument to INT, REAL, DBLE, or CMPLX intrinsic function
mobile:kargl[233] ./z
7F800000 Infinity
7FC00000      NaN
81111111 -2129587951

Yes, you get warnings about the extension.  The only way to suppress
those warnings is to use the -w option.  There is no requirement of 
using -fno-range-check.

> Main issue: While gfortran 10 still documents the BOZ extensions, 
> neither the extension documentation nor the error message nor point to 
> -fallow-invalid-boz – and the release notes is completely silent 
> regarding BOZ.
> 
> Hence, I included two patches:
> 
> * The WWWDOCS patch now mentions the change in GCC 10.

See below.

> * The Fortran patch does: (a) Mention 'X' in the flag documentation (as 
> it is not in an invalid context but just an invalid letter). (b) Tweak 
> error message for 'X' to state how to fix it. (c) When both -std=legacy 
> and -fallow-invalid-boz has been used, it no longer prints a warning; 
> for -std=gnu and -std=f... (+ -fallow…) we still do. (d) If -std=legacy 
> has been used without -fallow-invalid-boz, it now suggests to use 
> -fallow…, but only if fixing is not possible.

This is wrong on a number of level.

(a) To be pedantic it is an invalid context as 'X' is not invalid
    in the context of a BOZ literal constant.

(b) The only way to "fix it" is to change the code to use 'Z' instead
    of 'X'.  -fallow-invalid-boz does not "fix" the issue.  If you
    want to alert the usr to the option, then simple add "[See
    -fallow-invalid-boz]" to the error messsage.  

(c,d) NO. NO. NO.

    -std=legacy is intentionally not used.  If -fallow-invalid-boz
    can be used to prevent the error (see above examples), then it
    degrades the error to a warning.  This warning can only be suppressed
    with the -w option.  ***This was intentional.***  Nonstandard code
    will never be fixed if the user no longer sees an issue exists.

There is no such things as "code that cannot be fixed".  Silently
accepting nonstandard code gives users an excuse to not fix their
code.  Yes, someone will state that if they change their code, then
they need to go through a re-certification process.  So what?  It is
okay to ask a Fortran vendor to accept invalid Fortran, but not okay
to fix the code to conform to the Fortran standard.  That's idiotic
to me. 
vendor to support the nonstandard code.  
vendor to supp

> I hope with this patch, that using the compiler is more user friendly 
> but still giving the intensive to actually fix the legacy code.
> 
> Are the two patches OK – for wwwdocs and for the trunk, respectively?

IMHO, No.

> diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html
> index caa9df70..7df5124f 100644
> --- a/htdocs/gcc-10/changes.html
> +++ b/htdocs/gcc-10/changes.html
> @@ -260,6 +260,13 @@ a work-in-progress.</p>
>      with <code>-std=legacy</code>.  <code>-Wargument-mismatch</code>
>      has been removed.
>    </li>
> +  <li>
> +    Nonstandard use of BOZ literals now requires the
> +    <code>-fallow-invalid-boz</code> flag.  Before, some nonstandard
> +    use was only accepted with <code>-std=legacy</code> while
> +    other nonstandard usage was silently accepted even with
> +    <code>-std=f2003</code>.
> +  </li>

This isn't completely correct.  Some nonstandard uses can be allowed by
the option and some simply cannot (see above examples).  A better 
statement might be 


<li>
   The handling of a BOZ literal constant has been reworked to provide
   better conformance to the Fortran 2008 and 2018 standards.  In these
   Fortran standards, a BOZ literal constant is a typeless and kindless
   entity.  As a part of the rework, documented and undocumented
   extensions to the Fortran standard now emit errors during compilation.
   Some these extensions are permitted with the -fallow-invalid-boz, where
   the error is degraded to a warning and the code is compiled as wiht older
   gfortran.
<li>

>      At any optimization level except<code>-Os</code>, gfortran now
>      uses inline packing for arguments instead of calling a library

> 	PR fortran/93253
> 	* check.c (gfc_invalid_boz): With -std=legacy, suggest the new
> 	-fallow-invalid-boz flag. With -std=legacy and that flag, no
> 	longer warn.

No.

> 	* gfortran.texi (BOZ literal constants): List another missing
> 	extension and refer to -fallow-invalid-boz.
> 	* lang.opt (fallow-invalid-boz): Also mention 'X' in the help text
> 	as it is not covered by the previous wording.
> 	* primary.c (match_boz_constant): Tweak wording such that it is
> 	clear how to fix the nonstandard use.
> 
> 	PR fortran/93253
> 	* fortran.dg/boz_7.f90: Updated dg-error.
> 
> 
> diff --git a/gcc/fortran/check.c b/gcc/fortran/check.c
> index c7f0187b377..f03764abaef 100644
> --- a/gcc/fortran/check.c
> +++ b/gcc/fortran/check.c
> @@ -56,18 +56,30 @@ reset_boz (gfc_expr *x)
>     gfc_invalid_boz() is a helper function to simplify error/warning
>     generation.  gfortran accepts the nonstandard 'X' for 'Z', and gfortran
>     allows the BOZ indicator to appear as a suffix.  If -fallow-invalid-boz
> -   is used, then issue a warning; otherwise issue an error.  */
> +   is used, then issue a warning (unless -std=legacy); otherwise issue an
> +   error.  */

No.

>  
>  bool
>  gfc_invalid_boz (const char *msg, locus *loc)
>  {
>    if (flag_allow_invalid_boz)
>      {
> -      gfc_warning (0, msg, loc);
> +      if (gfc_option.warn_std & GFC_STD_LEGACY)

No.

> +	gfc_warning (0, msg, loc);
>        return false;
>      }
>  
> -  gfc_error (msg, loc);
> +  if (!(gfc_option.warn_std & GFC_STD_LEGACY))

No.

> +    {
> +      const char hint[] = ". Use %<-fallow-invalid-boz%> if you cannot fix it";

Suggest adding ". [See %<-fallow-invalid-boz%>.]"

There is no such thing as "cannot fix it".  

> +      size_t len = strlen (msg) + strlen (hint) + 1;
> +      char *msg2 = (char *) alloca (len);
> +      strcpy (msg2, msg);
> +      strcat (msg2, hint);
> +      gfc_error (msg2, loc);
> +    }
> +  else
> +    gfc_error (msg, loc);
>    return true;
>  }
>  
> diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
> index 4cf8b3a5c24..98fc74f3e67 100644
> --- a/gcc/fortran/gfortran.texi
> +++ b/gcc/fortran/gfortran.texi
> @@ -1863,9 +1863,11 @@ Fortran standard states that the treatment of the sign bit is processor
>  dependent.  Gfortran interprets the sign bit as a user would expect.
>  
>  As a deprecated extension, GNU Fortran allows hexadecimal BOZ literal
> -constants to be specified using the @code{X} prefix.  The BOZ literal
> +constants to be specified using the @code{X} prefix.  That the BOZ literal
>  constant can also be specified by adding a suffix to the string, for
> -example, @code{Z'ABC'} and @code{'ABC'X} are equivalent.
> +example, @code{Z'ABC'} and @code{'ABC'X} are equivalent.  And BOZ literals
> +outside @code{DATA} and the intrinsic functions listed in the Fortran
> +standard.  Use @option{-fallow-invalid-boz} to enable the extension.

The ". And BOZ literals outside ... Fortran standard." is not a sentence.

It is true -fallow-invalid-boz allows some extention but not all of them
"real :: x = z'7fc000000'" is allowed.  In fact, I think J3 is considering
to make this conforming.  "[b'1', b'2']" is nonconforming and it is not
allowed with the option.

>  @node Real array indices
>  @subsection Real array indices
> diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
> index 3858331bcc0..59523f74acf 100644
> --- a/gcc/fortran/lang.opt
> +++ b/gcc/fortran/lang.opt
> @@ -387,7 +387,7 @@ All intrinsics procedures are available regardless of selected standard.
>  
>  fallow-invalid-boz
>  Fortran RejectNegative Var(flag_allow_invalid_boz)
> -Allow a BOZ literal constant to appear in an invalid context.
> +Allow a BOZ literal constant to appear in an invalid context and with X instead of Z.

Ok.

>  fallow-leading-underscore
>  Fortran Undocumented Var(flag_allow_leading_underscore)
> diff --git a/gcc/fortran/primary.c b/gcc/fortran/primary.c
> index e2b6fcb2106..07b8ac08ba2 100644
> --- a/gcc/fortran/primary.c
> +++ b/gcc/fortran/primary.c
> @@ -433,7 +433,7 @@ match_boz_constant (gfc_expr **result)
>  
>    if (x_hex
>        && gfc_invalid_boz ("Hexadecimal constant at %L uses "
> -			  "nonstandard syntax", &gfc_current_locus))
> +			  "nonstandard X instead of Z", &gfc_current_locus))

I suppose ok, but don't think a change is needed.

>      return MATCH_ERROR;
>  
>    old_loc = gfc_current_locus;
> diff --git a/gcc/testsuite/gfortran.dg/boz_7.f90 b/gcc/testsuite/gfortran.dg/boz_7.f90
> index 45fa7a7df19..d2a51ac03e2 100644
> --- a/gcc/testsuite/gfortran.dg/boz_7.f90
> +++ b/gcc/testsuite/gfortran.dg/boz_7.f90
> @@ -7,6 +7,6 @@
>  !
>  integer :: k, m
>  integer :: j = z'000abc' ! { dg-error "BOZ used outside a DATA statement" }
> -data k/x'0003'/ ! { dg-error "nonstandard syntax" }
> +data k/x'0003'/ ! { dg-error "nonstandard X instead of Z" }
>  data m/'0003'z/ ! { dg-error "nonstandard postfix" }
>  end

-- 
Steve