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

Bump line number after syntax errors #3270

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

d-torrance
Copy link
Member

This adds consistency with other errors. For example, this is what happens when we have higher-level errors:

i1 : 1/0
stdio:1:2:(3): error: division by zero

i2 : 1/0
stdio:2:2:(3): error: division by zero

i3 : 1/0
stdio:3:2:(3): error: division by zero

i4 : currentPosition()

o4 = stdio:5:0

o4 : FilePosition

The current line number increases by 1 each time, as does the file position.

However, if it's a syntax error, this doesn't happen:

i1 : )
stdio:1:1:(3): error: syntax error at ')'

i1 : )
stdio:1:2:(3): error: syntax error at ')'

i1 : )
stdio:1:3:(3): error: syntax error at ')'

i1 : currentPosition()

o1 = stdio:2:0

o1 : FilePosition

After

i1 : )
stdio:1:1:(3): error: syntax error at ')'

i2 : )
stdio:2:1:(3): error: syntax error at ')'

i3 : )
stdio:3:1:(3): error: syntax error at ')'

i4 : currentPosition()

o4 = stdio:5:0

o4 : FilePosition

@pzinn
Copy link
Contributor

pzinn commented May 30, 2024

I would be very careful changing this behaviour. I spent a lot of time fixing line numbers in error messages and I think I got it under control now (still haven't made a PR sadly) . Please give me some time to take a look at this. (I'm on the plane for the next 24h).

@d-torrance d-torrance requested a review from pzinn May 30, 2024 04:42
@pzinn
Copy link
Contributor

pzinn commented Jun 4, 2024

Looks good to me. I'm wondering if you also want to do something about this:

i1 : `
stdio:1:1:(3): error: invalid character

i1 : 

@d-torrance
Copy link
Member Author

Looks good to me. I'm wondering if you also want to do something about this:

i1 : `
stdio:1:1:(3): error: invalid character

i1 : 

Yes, definitely -- thanks for pointing this out!

@d-torrance
Copy link
Member Author

The "invalid character" error is one of a larger class of "token errors". I noticed the code earlier but wasn't sure how to reproduce one. Another example is trying to parse a real number that has e but no exponent.

Before

i1 : `
stdio:1:1:(3): error: invalid character

i1 : 1e
stdio:1:4:(3): error: exponent missing in floating point constant

i1 : 

After

i1 : `
stdio:1:1:(3): error: invalid character

i2 : 1e
stdio:2:3:(3): error: exponent missing in floating point constant

i3 : 

@mahrud
Copy link
Member

mahrud commented Jun 4, 2024

Are you sure the behavior you're changing wasn't intentional? The stdio line number is changing to signal where in the source an error occurred in order to fix it, but it's not necessarily supposed to match the i number of the input. Here's an example:

i1 : 1; 2; 3; 4; 5; 6; 7; 8; 9; )
stdio:1:28:(3): error: syntax error at ')'

i10 : 

@d-torrance
Copy link
Member Author

I agree that the stdio line number shouldn't necessarily match the i number. And it won't necessarily agree after this PR either:

i1 : 1; 2; 3; 4; 5; 6; 7; 8; 9; )
stdio:1:28:(3): error: syntax error at ')'

i11 : )
stdio:2:1:(3): error: syntax error at ')'

Note that the i number increased by 10 (the 9 evaluated expressions and the 1 syntax error) and the stdio line number increased by 1. Currently, the i number only increases 9 times for the evaluated expressions and the stdio line number doesn't change.

I noticed this issue after code didn't work properly for functions defined on the standard input after some errors. This is because we were adding a new entry to readline's history but not increasing the stdio line number, so the numbers got out of sync.

Currently:

i1 : )
stdio:1:1:(3): error: syntax error at ')'

i1 : f = x -> x^2

o1 = f

o1 : FunctionClosure

i2 : code f

o2 = stdio:1:7-1:12: --source code:
     )

But with this PR:

i1 : )
stdio:1:1:(3): error: syntax error at ')'

i2 : f = x -> x^2

o2 = f

o2 : FunctionClosure

i3 : code f

o3 = stdio:2:6-2:11: --source code:
     f = x -> x^2

@mahrud
Copy link
Member

mahrud commented Jun 4, 2024

Yes, I agree with the fix for stdio (which I believe should also fix the issue with code), but I don't think syntax errors should increment the line number. I think it is intentional that when a function returns an error (say division by zero), the input line number is increased because the interpreter understood the command, ran some stuff, and an error occurred, but if the interpreter can't even parse the input, then it perhaps it should ask for that input line again without incrementing anything other than stdio.

@d-torrance
Copy link
Member Author

Ok, that makes sense. I'll revert the i line number stuff.

@DanGrayson DanGrayson merged commit da09945 into Macaulay2:development Jun 5, 2024
6 checks passed
@d-torrance d-torrance deleted the bump-line-number branch June 6, 2024 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants