This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Ping**2! Re: [PATCH, Fortran] Extension: AUTOMATIC/STATIC symbol attributes with -fdec-static


Hi Fritz,

please note: I do not have official review privileges. So my vote here
is rather an advise to you and the official reviewers. Often such a
inofficial review helps to speed things up, because the official ones
are pointed to the nics and nacs and don't have to bother with the
minor things.

So here it comes:

- Do I understand this correctly: AUTOMATIC and STATIC have to come last,
  i.e., right before the :: where declaring, e.g., a variable?


- Running:

  $ contrib/check_GNU_style.sh dec_static.patch

  Reports some style issues in the C code, that should be fixed before
  commit. (Style in Fortran testcases does not matter that much.)


- I have deleted huge parts of the diff and just kept the parts I had a
  question/remark for:

> diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
> index b34ae86..a0cf78b 100644
> --- a/gcc/fortran/gfortran.texi
> +++ b/gcc/fortran/gfortran.texi
> @@ -2120,7 +2121,6 @@ consider @code{BACKSPACE} or @code{REWIND} to properly position
>  the file before the EOF marker.  As an extension, the run-time error may
>  be disabled using -std=legacy.
>  
> -

Please change formatting in a separate patch or not at all (here!).
This policy is to distinguish cosmetic changes from relevant ones.

>  @node STRUCTURE and RECORD
>  @subsection @code{STRUCTURE} and @code{RECORD}
>  @cindex @code{STRUCTURE}
> @@ -2420,6 +2420,53 @@ here:
>    @tab @code{--} @tab @code{FLOATI} @tab @code{FLOATJ} @tab @code{FLOATK}
>  @end multitable
>  
> +@node AUTOMATIC and STATIC attributes
> +@subsection @code{AUTOMATIC} and @code{STATIC} attributes
> +@cindex variable attributes
> +@cindex @code{AUTOMATIC}
> +@cindex @code{STATIC}
> +
> +With @option{-fdec-static} GNU Fortran supports the explicit specification of
> +two addition variable attributes: @code{STATIC} and @code{AUTOMATIC}. These

two additional variable ...
            ^^ 

But is it only for variables? Can't it be used for equivalences or
other constructs, too?

> diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi
> index 15c131a..a5da59e 100644
> --- a/gcc/fortran/invoke.texi
> +++ b/gcc/fortran/invoke.texi
> @@ -255,6 +255,11 @@ instead where possible.
>  Enable B/I/J/K kind variants of existing integer functions (e.g. BIAND, IIAND,
>  JIAND, etc...). For a complete list of intrinsics see the full documentation.
>  
> +@item -fdec-static
> +@opindex @code{fdec-static}
> +Enable STATIC and AUTOMATIC as attributes specifying storage location.
> +STATIC is equivalent to SAVE, and locals are typically AUTOMATIC by default.

Well, this description to me sounds like: "Those attributes are
useless, because they can be substituted." This is clearly not what you
intend. I propose to include into the description that with "this
switch the dec-extension" is available "to explicitly specify the
storage of entities". Then the last sentence is still a good hint for
all fortraneers that don't know the extension.

> +
>  @item -fdollar-ok
>  @opindex @code{fdollar-ok}
>  @cindex @code{$}
> diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt
> index 8ec5400..260512d 100644
> --- a/gcc/fortran/lang.opt
> +++ b/gcc/fortran/lang.opt
> @@ -432,6 +432,10 @@ fdec-structure
>  Fortran
>  Enable support for DEC STRUCTURE/RECORD.
>  
> +fdec-static
> +Fortran Var(flag_dec_static)
> +Enable STATIC and AUTOMATIC attributes.

How about: Enable the dec-extension of STATIC and AUTOMATIC attributes.
Just a proposal.

> +
>  fdefault-double-8
>  Fortran Var(flag_default_double)
>  Set the default double precision kind to an 8 byte wide type.


> diff --git a/gcc/testsuite/gfortran.dg/dec_static_1.f90 b/gcc/testsuite/gfortran.dg/dec_static_1.f90
> new file mode 100644
> index 0000000..4dcfc7c
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/dec_static_1.f90

-  Please add some testcases where the new error messages are tested.

So much from my side. Btw, I haven't applied the patch and tested
whether it runs or collides with other proposed patches. That is
usually done by Dominique and I did not want to waste doing it a second
time.

Regards,
	Andre
-- 
Andre Vehreschild * Email: vehre ad gmx dot de 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]