Re: [PATCH] take 3: pragma FLOAT_CONST_DECIMAL64 for decimal float support

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Wed, 6 May 2009, Janis Johnson wrote:

> > > Index: gcc/c-cppbuiltin.c
> > > ===================================================================
> > > --- gcc/c-cppbuiltin.c	(revision 144896)
> > > +++ gcc/c-cppbuiltin.c	(working copy)
> > > @@ -626,7 +626,7 @@ c_cpp_builtins (cpp_reader *pfile)
> > >    if (flag_single_precision_constant)
> > >      builtin_define_float_constants ("DBL", "L", "((double)%s)", double_type_node);
> > >    else
> > > -    builtin_define_float_constants ("DBL", "", "%s", double_type_node);
> > > +    builtin_define_float_constants ("DBL", "L", "((double)%s)", double_type_node);
> > 
> > The effect of this is to use the new definition unconditionally, in which 
> > case you should remove the if and insert a comment explaining the use 
> > cases (-fsingle-precision-constant and this pragma) needing the definition 
> > as long double and the cast.
> 
> Fixed.
> 
> The comment says that the cast prevents the predefined macros from being
> used in const expressions; if that's a problem we might just leave this
> part out and force code that uses these macros to do so within code
> blocks that turn off the pragma.

This does not prevent them from being used in constant expressions; the 
macros of floating-point type are only required to be usable in arithmetic 
constant expressions, not integer constant expressions, and casts to 
floating-point type are fine in arithmetic constant expressions.  So 
remove that claim from the comment.

> Index: gcc/c-common.c
> ===================================================================
> --- gcc/c-common.c	(revision 147110)
> +++ gcc/c-common.c	(working copy)
> @@ -416,6 +416,10 @@ int flag_signed_bitfields = 1;
>  
>  int warn_unknown_pragmas; /* Tri state variable.  */
>  
> +/* Warn about float constants with suffixes.  */
> +
> +int warn_unsuffixed_float_consts;

Unused variable (the one that is used, warn_unsuffixed_float_constants, 
has an automatic definition).

> Index: gcc/c-parser.c
> ===================================================================
> --- gcc/c-parser.c	(revision 147110)
> +++ gcc/c-parser.c	(working copy)
> @@ -979,7 +979,9 @@ c_parser_translation_unit (c_parser *par
>        do
>  	{
>  	  ggc_collect ();
> +	  mark_valid_location_for_stdc_pragma (true);
>  	  c_parser_external_declaration (parser);
> +	  mark_valid_location_for_stdc_pragma (false);
>  	  obstack_free (&parser_obstack, obstack_position);
>  	}
>        while (c_parser_next_token_is_not (parser, CPP_EOF));

What I think should be done here is marking it an invalid location before 
c_parser_external_declaration and a valid location after, and just 
allowing it in c_parser_external_declaration in the specific case that the 
external declaration is a pragma.  Then you shouldn't need any special 
code in c_parser_declspecs to mark it invalid there.

> @@ -3356,6 +3362,7 @@ c_parser_compound_statement_nostart (c_p
>        c_parser_consume_token (parser);
>        return;
>      }
> +  mark_valid_location_for_stdc_pragma (true);
>    if (c_parser_next_token_is_keyword (parser, RID_LABEL))
>      {
>        location_t err_loc = c_parser_peek_token (parser)->location;

This looks right.

> @@ -3409,6 +3416,7 @@ c_parser_compound_statement_nostart (c_p
>  	    label_loc = c_parser_peek_token (parser)->location;
>  	  last_label = true;
>  	  last_stmt = false;
> +	  mark_valid_location_for_stdc_pragma (false);
>  	  c_parser_label (parser);
>  	}
>        else if (!last_label
> @@ -3416,6 +3424,7 @@ c_parser_compound_statement_nostart (c_p
>  	{
>  	  last_label = false;
>  	  c_parser_declaration_or_fndef (parser, true, true, true, true);
> +	  mark_valid_location_for_stdc_pragma (false);

I'd expect this to go before parsing the declaration, not after (thereby 
ensuring that the pragma isn't allowed in struct definitions within a 
function without needing anything in c_parser_declspecs).

Don't you also need such calls in the __extension__ case?

As this is a global flag, you really should be saving and restoring in 
this function; it looks rather like it might allow

{
  {
#pragma
  }
#pragma
}

at present because the flag gets cleared in the inner scope and not set 
again.

> Index: gcc/doc/invoke.texi
> ===================================================================
> --- gcc/doc/invoke.texi	(revision 147110)
> +++ gcc/doc/invoke.texi	(working copy)
> @@ -4218,6 +4218,15 @@ minimum maximum, so we do not diagnose o
>  
>  This option is implied by @option{-pedantic}, and can be disabled with
>  @option{-Wno-overlength-strings}.
> +
> +@item -Wunsuffixed-float-constants
> +@opindex Wunsuffixed-float-constants
> +
> +GCC will issue a warning for any floating constant that does not have
> +a suffix.  When used together with @option{-Wsystem-headers} it will
> +warn about such constants in system header files.  This can be useful
> +when preparing code to use with the @code{FLOAT_CONST_DECIMAL64} pragma
> +from the decimal floating-point extension to C99.
>  @end table

There are summary tables of options earlier in invoke.texi that need the 
new option added somewhere.

-- 
Joseph S. Myers
joseph@codesourcery.com