Re: [PATCH,gfortran] Fix for PR 17941

[Steve Kargl <sgk at troutmask dot apl dot washington dot edu>]

On Sat, Jan 15, 2005 at 11:07:44AM +0000, Paul Brook wrote:
> On Friday 31 December 2004 05:57, Steve Kargl wrote:
> > The attached patch fixes PR 17941.  The attached program
> > is a test program suitable for inclusion in the gfortran
> > testsuite.
> >
> > Briefly, gfortran could not deal with spaces between an
> > uniary plus/minus sign in a complex constant.  That is,
> "complex, parameter :: c = (-   1.0,   +  2)" would invoke
> > an error.
> 
> This is supposed to be an error. Your code is illegal in free-form source, and 
> already accepted in fixed-form.

NAG's compiler issues neither a warning nor an error on the test program.
Of course, NAG may be treating "complex, parameter :: c = (-  1.0, + 2)"
as a constant initialization expression.

> Section 4.3.1.3 "Complex type" defines a complex literal constant as a pair of 
> signed real literal constants (plus "(,)" separators).
> Section 3.2 "Low-level syntax" says that a real literal constant is a single 
> lexical token.
> Section 3.3.1 "Free source form" says that whitespace may not occur within a 
> lexical token.
> 
> We may wish to allow whitespace between the sign character and 
> digits in signed literal constants as an extension. If so you should also 
> modify the other places where this can occur.
> 
>   DATA a /- 1.0/
> 
> I'm not sure why we don't just use signed_real_constant here. That's what the 
> standard says.

The comment in primary.c gives the reason.

/* Match the real and imaginary parts of a complex number.  This
   subroutine is essentially match_real_constant() modified in a
   couple of ways: A sign is always allowed and numbers that would
   look like an integer to match_real_constant() are automatically
   created as floating point numbers.  The messiness involved with
   making sure a decimal point belongs to the number and not a
   trailing operator is not necessary here either (Hooray!).  */

> A few comments on the patch itself:
> 
> > +#include <stdlib.h> /* Needed for alloca.  */
> > +#include <string.h> /* Needed for strlen.  */
> 
> This is wrong.
> These are already included under the proper conditionals by 
> system.h.

See date of email to which you replied and then see ChangeLog
2005-01-03  Steven G. Kargl  <kargls@comcast.net>

> 
> > @@ -1936,7 +1940,21 @@
> >      t = buffer + 1;
> >    else
> >      t = buffer;
> > -  mpfr_set_str (e->value.real, t, 10, GFC_RND_MODE);
> > +
> > +  /* Spaces are not allowed in the string passed to mpfr_set_str.  The 
> > +     parsing of complex constants may have spaces, so copy the t to u
> > +     where we remove spaces.  */
> > +  u = alloca (strlen (t) + 1);
> 
> "buffer" is already a temporary copy of the string. Fix that instead of making 
> another copy. I'd suggest copying just the unsigned  part of the value (passed 
> fo gfc_convert_real), then negate the result as necessary.
> 
> > +  /* Account for whitespace between the uniary minus/plus and the number.  
> */
> > +  while (c == ' ' || c == '\t')
> > +    {
> > +      c = gfc_next_char ();
> > +      count++;
> 
> Use gfc_gobble_whitespace().

gfc_gobble_whitespace() does not increment a counter, which I needed
for the patch.

-- 
Steve