Skip to content

Commit

Permalink
Remove support for float literals starting with a period
Browse files Browse the repository at this point in the history 
Before this patch, writing .0 instead of 0.0 was allowed, as in C, C++, D,
Java, Go, Perl, Python and many other languages. This required a hack in the
form of an additional entry pointer for our scanner. We now get rid of it,
matching languages like Ada, Haskell, OCaml, Rust, Swift.
  • Loading branch information
@fingolfin
fingolfin committed Mar 28, 2022
1 parent 3af3ebb commit 6dc3d5d
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 113 deletions.
7 changes: 0 additions & 7 deletions src/read.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1495,13 +1495,6 @@ static void ReadFuncExprAbbrevSingle(ReaderState * rs, TypSymbolSet follow)
*/
static void ReadLiteral(ReaderState * rs, TypSymbolSet follow, Char mode)
{
if (rs->s.Symbol == S_DOT) {
// HACK: The only way a dot could turn up here is in a floating point
// literal that starts with '.'. Call back to the scanner to deal
// with this.
ScanForFloatAfterDotHACK(&rs->s);
}

switch (rs->s.Symbol) {

/* <Int> */
Expand Down
123 changes: 49 additions & 74 deletions src/scanner.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,6 @@ static UInt GetIdent(ScannerState * s, Int i, Char c)
**
** When 's->Value' is completely filled, then a GAP string object is
** created in 's->ValueObj' and all data is stored there.
**
** The argument is used to signal if a decimal point was already read,
** or whether we are starting from scratch..
**
*/
static UInt AddCharToBuf(Obj * string, Char * buf, UInt bufsize, UInt pos, Char c)
{
Expand All @@ -386,89 +382,79 @@ static UInt AddCharToValue(ScannerState * s, UInt pos, Char c)
return AddCharToBuf(&s->ValueObj, s->Value, MAX_VALUE_LEN - 1, pos, c);
}

static UInt GetNumber(ScannerState * s, Int readDecimalPoint, Char c)
static UInt GetNumber(ScannerState * s, Char c)
{
UInt symbol = S_ILLEGAL;
UInt i = 0;
BOOL seenADigit = FALSE;

s->ValueObj = 0;

if (readDecimalPoint) {
s->Value[i++] = '.';
GAP_ASSERT(IsDigit(c));

// read initial sequence of digits into 'Value'
while (IsDigit(c)) {
i = AddCharToValue(s, i, c);
c = GET_NEXT_CHAR();
}
else {
// read initial sequence of digits into 'Value'
while (IsDigit(c)) {
i = AddCharToValue(s, i, c);
seenADigit = TRUE;
c = GET_NEXT_CHAR();
}

// maybe we saw an identifier character and realised that this is an
// identifier we are reading
if (IsIdent(c) || c == '\\') {
// if necessary, copy back from s->ValueObj to s->Value
if (s->ValueObj) {
i = GET_LEN_STRING(s->ValueObj);
GAP_ASSERT(i >= MAX_VALUE_LEN - 1);
memcpy(s->Value, CONST_CSTR_STRING(s->ValueObj),
MAX_VALUE_LEN);
s->ValueObj = 0;
}
// this looks like an identifier, scan the rest of it
return GetIdent(s, i, c);
// maybe we saw an identifier character and realised that this is an
// identifier we are reading
if (IsIdent(c) || c == '\\') {
// if necessary, copy back from s->ValueObj to s->Value
if (s->ValueObj) {
i = GET_LEN_STRING(s->ValueObj);
GAP_ASSERT(i >= MAX_VALUE_LEN - 1);
memcpy(s->Value, CONST_CSTR_STRING(s->ValueObj),
MAX_VALUE_LEN);
s->ValueObj = 0;
}
// this looks like an identifier, scan the rest of it
return GetIdent(s, i, c);
}

// Or maybe we saw a '.' which could indicate one of three things:
// - a float literal: 12.345
// - S_DOT, i.e., '.' used to access a record entry: r.12.345
// - S_DDOT, i.e., '..' in a range expression: [12..345]
if (c == '.') {
GAP_ASSERT(i < MAX_VALUE_LEN - 1);

// If the symbol before this integer was S_DOT then we must be in
// a nested record element expression, so don't look for a float.
// This is a bit fragile
if (s->Symbol == S_DOT || s->Symbol == S_BDOT) {
symbol = S_INT;
goto finish;
}

// peek ahead to decide if we are looking at a range expression
if (PEEK_NEXT_CHAR(s->input) == '.') {
// we are looking at '..' and are probably inside a range
// expression
symbol = S_INT;
goto finish;
}
// Or maybe we saw a '.' which could indicate one of three things:
// - a float literal: 12.345
// - S_DOT, i.e., '.' used to access a record entry: r.12.345
// - S_DDOT, i.e., '..' in a range expression: [12..345]
if (c == '.') {
GAP_ASSERT(i < MAX_VALUE_LEN - 1);

// Now the '.' must be part of our number; store it and move on
i = AddCharToValue(s, i, '.');
c = GET_NEXT_CHAR();
// If the symbol before this integer was S_DOT then we must be in
// a nested record element expression, so don't look for a float.
// This is a bit fragile
if (s->Symbol == S_DOT || s->Symbol == S_BDOT) {
symbol = S_INT;
goto finish;
}
else {
// Anything else we see tells us that the token is done

// peek ahead to decide if we are looking at a range expression
if (PEEK_NEXT_CHAR(s->input) == '.') {
// we are looking at '..' and are probably inside a range
// expression
symbol = S_INT;
goto finish;
}

// Now the '.' must be part of our number; store it and move on
i = AddCharToValue(s, i, '.');
c = GET_NEXT_CHAR();
}
else {
// Anything else we see tells us that the token is done
symbol = S_INT;
goto finish;
}

// When we get here we have read possibly some digits, a . and possibly
// When we get here we have read some digits and a dot and possibly
// some more digits, but not an e,E,d,D,q or Q
// In any case, from now on, we know we are dealing with a float literal
symbol = S_FLOAT;

// read digits
// read digits after dot
while (IsDigit(c)) {
i = AddCharToValue(s, i, c);
seenADigit = TRUE;
c = GET_NEXT_CHAR();
}
if (!seenADigit)
SyntaxError(s,
"Badly formed number: need a digit before or after the "
"decimal point");
if (c == '\\')
SyntaxError(s, "Badly formed number");

Expand Down Expand Up @@ -529,17 +515,6 @@ static UInt GetNumber(ScannerState * s, Int readDecimalPoint, Char c)
}


/****************************************************************************
**
*F ScanForFloatAfterDotHACK()
**
*/
void ScanForFloatAfterDotHACK(ScannerState * s)
{
s->Symbol = GetNumber(s, 1, PEEK_CURR_CHAR(s->input));
}


/****************************************************************************
**
*F GetOctalDigits()
Expand Down Expand Up @@ -970,7 +945,7 @@ static UInt NextSymbol(ScannerState * s)

case '0': case '1': case '2': case '3': case '4':
case '5': case '6': case '7': case '8': case '9':
return GetNumber(s, 0, c);
return GetNumber(s, c);

case '\377': symbol = S_EOF; FlushRestOfInputLine(s->input); break;

Expand Down
13 changes: 0 additions & 13 deletions src/scanner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,17 +342,4 @@ void Match(ScannerState * s,
TypSymbolSet skipto);


/****************************************************************************
**
*F ScanForFloatAfterDotHACK()
**
** This function is called by 'ReadLiteral' if it encounters a single dot in
** form the of the symbol 'S_DOT'. The only legal way this could happen is
** if the dot is the start of a float literal like '.123'. As the scanner
** cannot detect this without being context aware, we must provide this
** function to allow the reader to signal to the scanner about this.
*/
void ScanForFloatAfterDotHACK(ScannerState * s);


#endif // GAP_SCANNER_H
5 changes: 2 additions & 3 deletions tst/testbugfix/2013-08-21-t00295.tst
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# 2013/08/21 (MH)
gap> . . . .
Syntax error: Badly formed number: need a digit before or after the decimal po\
int in stream:1
Syntax error: literal expected in stream:1
. . . .
^
Syntax error: Record component name expected in stream:2
Syntax error: ; expected in stream:2
2 changes: 1 addition & 1 deletion tst/testinstall/float.tst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ gap> ComplexConjugate(1.3);
#
gap> Display(1.3);
1.3
gap> Display(-.4e6);
gap> Display(-0.4e6);
-400000.
gap> PrintObj(1.3); Print("Q\n");
1.3Q
Expand Down
25 changes: 10 additions & 15 deletions tst/testinstall/longnumber.tst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,40 +152,35 @@ gap> 1.;
1.
gap> 0.;
0.
gap> .1;
0.1
gap> 0.1;
0.1
gap> 1111111111111111111111111111111111111.1;
1.11111e+36
gap> 1.11111111111111111111111111111111111111;
1.11111
gap> .;
Syntax error: Badly formed number: need a digit before or after the decimal po\
int in stream:1
Syntax error: literal expected in stream:1
.;
^
gap> .n;
Syntax error: Badly formed number: need a digit before or after the decimal po\
int in stream:1
Syntax error: literal expected in stream:1
.n;
^
gap> .q;
Syntax error: Badly formed number: need a digit before or after the decimal po\
int in stream:1
Syntax error: literal expected in stream:1
.q;
^
gap> .0n;
Error, failed to convert float literal
Syntax error: literal expected in stream:1
.0n;
^
gap> .0q;
Syntax error: Badly formed number: need at least one digit in the exponent in \
stream:1
Syntax error: literal expected in stream:1
.0q;
^^^
^
gap> .0qn;
Syntax error: Badly formed number: need at least one digit in the exponent in \
stream:1
Syntax error: literal expected in stream:1
.0qn;
^^^
^
gap> Unbind(x);
gap> STOP_TEST( "longnumber.tst", 1);

2 comments on commit 6dc3d5d

@laurentbartholdi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fingolfin, what's the motivation behind this change? GMP by default prints numbers starting with a dot, so in the current version of Floats we have

gap> LoadPackage("float");
float 1.0.3 with modules [mpfr, mpfi, mpc, fplll, cxsc] ...
true
gap> SetFloats(MPFR);
gap> 0.1;
.1e0

It's usually a standard assumption that GAP can parse the numbers that it prints, so Floats should be changed to reflect the new stricter rules, and I'm not enthusiastic for this change.

@fingolfin
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laurentbartholdi see #3280

Please sign in to comment.