Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some malformed floats do not raise an error, instead evaluate to fail #1105

Closed
fingolfin opened this issue Jan 25, 2017 · 4 comments
Closed
Labels
kind: bug Issues describing general bugs, and PRs fixing them topic: parser

Comments

@fingolfin
Copy link
Member

Consider this:

gap> 1.1x;
fail
gap> 1.1;
1.1
gap> 1.x1
Syntax error: Badly formed number
1.x1
  ^

I would expect the first example to raise an error, not return fail.

My guess would be that this is caused by parsing of float expressions in exponent notation, i.e.

gap> 1.1e4;
11000.
gap> 1.1x4;
Syntax error: Badly formed number
1.1x4;
   ^
gap> 1.1x;
fail
@fingolfin fingolfin added the kind: bug Issues describing general bugs, and PRs fixing them label Jan 25, 2017
@ChrisJefferson
Copy link
Contributor

So, this seems to be an intentional choice. There is a comment in float parsing

/* We allow one letter on the end of the numbers -- could be an i, C99 style */

This is, I believe, to support float extensions which want an extra character. Because of this, we can't raise an error until we try converting the float, as a user-installed float package can use this character.

So, two questions:

  • Do we want to this? Does float use it?
  • Should the default float parser do something more sensible than return fail? Perhaps throw an Error.

@ChrisJefferson
Copy link
Contributor

ChrisJefferson commented Jan 25, 2017

How about something like:

gap> 1.1x;
Error, Invalid float: '1.1x' at /Users/caj/reps/gap/gap/lib/float.gi:208 called from
...
gap> f := y -> 1.1x * y;
function( y ) ... end
gap> f(2);
Error, Invalid float: '1.1x' at /Users/caj/reps/gap/gap/lib/float.gi:208 called from
...

Note that we don't error until we interpret the float, we just error instead of returning float at that point. This would technically change behaviour, but I hope no-one is relying on these weird floats becomes fail!

@stevelinton
Copy link
Contributor

@laurentbartholdi could you comment on this?

  1. Are there any reasons to allow any letter in that position except 'i'?
  2. Is it reasoanble for the parser always to throw an error if a floating point literal conversion returns fail.

@laurentbartholdi
Copy link
Contributor

@stevelinton : 1. I think it's better not to duplicate the checking: there could be other options in the future, i,j,k for quaternions, etc.
2. Definitely 1.1x should raise an error, not return fail. I think your suggestion is best.

fingolfin added a commit to fingolfin/gap that referenced this issue Dec 19, 2017
fingolfin added a commit to fingolfin/gap that referenced this issue Dec 19, 2017
fingolfin added a commit to fingolfin/gap that referenced this issue Jan 4, 2018
fingolfin added a commit to fingolfin/gap that referenced this issue Apr 17, 2018
fingolfin added a commit to fingolfin/gap that referenced this issue Apr 17, 2018
fingolfin added a commit to fingolfin/gap that referenced this issue Apr 17, 2018
fingolfin added a commit to fingolfin/gap that referenced this issue Apr 17, 2018
fingolfin added a commit to fingolfin/gap that referenced this issue Apr 18, 2018
fingolfin added a commit to fingolfin/gap that referenced this issue Apr 19, 2018
ChrisJefferson pushed a commit to ChrisJefferson/gap that referenced this issue Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Issues describing general bugs, and PRs fixing them topic: parser
Projects
None yet
Development

No branches or pull requests

4 participants