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

Maintain line and character number after parsing #86

Open
gregorybchris opened this issue Jan 10, 2024 · 10 comments
Open

Maintain line and character number after parsing #86

gregorybchris opened this issue Jan 10, 2024 · 10 comments

Comments

@gregorybchris
Copy link
Collaborator

Evaluation has many failure cases, many of which have pretty reasonable error messages. However we do not maintain the line and character numbers associated with nodes in the AST. The interpreter can't point to the part of the code associated with a failure that occurs during evaluation.

This feature request is to make code index information available in the evaluation stage.

@tekknolagi
Copy link
Owner

This is a little finicky right now unless we want to store (invalid) line number information on all objects because the AST structures are objects themselves. Perhaps we should separate them out (ehhhh) or find another way to do a mixin or something.

@tekknolagi
Copy link
Owner

Screenshot from 2024-01-10 11-31-29

came across this out of nowhere this morning

@tekknolagi
Copy link
Owner

the reasoning is a little weird but i think we can calculate the line/col from the byte position easily and it's only one number to store

@tekknolagi
Copy link
Owner

I saw/skimmed part of a cool talk that did this for Circle, I think. Perhaps this talk https://www.youtube.com/watch?v=1m_5SVmGA4k

They have some cool tricks

@tekknolagi
Copy link
Owner

tekknolagi commented Jan 10, 2024

Oh, no, lmao, it was Carbon: https://www.youtube.com/watch?v=ZI198eFghJk
Starting ~30mins?

@tekknolagi
Copy link
Owner

cc @neuroevolutus maybe this is also interesting

I think we want to keep line/col on all AST objects but not on interstitial/value objects. This probably means setting them dynamically after construction and having helper functions has_sourcepos and sourcepos. Then hopefully we can use this in parse/eval/compile errors!

Maybe we can one day even emit #line directives in the compiler.

@neuroevolutus
Copy link
Contributor

Neat, I'll have to take a look at the referenced article and video.

@neuroevolutus
Copy link
Contributor

@tekknolagi I started working on a pull request for this, and I realised that adding additional fields to the Token class to represent column numbers and/or byte positions would cause the golden tests that check for the string contents of ParseErrors to fail. Would it be acceptable to change those tests to instead test for the presence of the relevant fields and their specific values instead so that they become more immune to changes in the string representation of ParseErrors?

@tekknolagi
Copy link
Owner

Sure! Please split that out into a separate commit (same PR) onto which you stack your lineno commit.

@tekknolagi
Copy link
Owner

In general, tests are as mutable as the rest of the code, and I like your philosophy of having stable tests.

Some tests should check end-to-end string stuff though so that we have tests for stuff users see.

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

No branches or pull requests

3 participants