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

Improve AST-level source position tracking #2071

Closed
sauclovian-g opened this issue Jul 17, 2024 · 3 comments
Closed

Improve AST-level source position tracking #2071

sauclovian-g opened this issue Jul 17, 2024 · 3 comments
Assignees
Labels
tech debt Issues that document or involve technical debt topics: error-handling Issues involving the way SAW responds to an error condition topics: error-messages Issues involving the messages SAW produces on error

Comments

@sauclovian-g
Copy link
Contributor

At the AST level (src/SAWScript/AST.hs) statements have source positions on each constructor, but expressions, patterns, and types are instead handled by having an extra constructor with just a position that's inserted at, in theory, strategic places to assure that position information is available. The idea is to use the next enclosing such position.

This doesn't actually work out very well. First, for type errors, one really wants the exact position of the exact expression that one's complaining about and not an approximation. Second, it's hard to use in practice (since it has to be carried around as pass state) and it's also hard to know in general what position you have and what it's actually the position of.

We decided this should be removed in favor of just storing positions in each constructor, at least for expressions and patterns. So far whether to extend this to types is TBD.

@sauclovian-g sauclovian-g added topics: error-messages Issues involving the messages SAW produces on error tech debt Issues that document or involve technical debt topics: error-handling Issues involving the way SAW responds to an error condition labels Jul 17, 2024
@sauclovian-g sauclovian-g self-assigned this Jul 17, 2024
@sauclovian-g
Copy link
Contributor Author

Update: we decided to extend it to types.

After that there's a third set of changes, which is that some downstream code is using a global variable in a shared monad to push positions around. This should be ripped out; it's difficult to reason about global variables and after the position tracking updates it should no longer be necessary either.

@sauclovian-g
Copy link
Contributor Author

Further update: that global variable reaches almost everywhere and clearing out is a big deal. I'm going to open a new issue for it and close this one, as this one's done.

@sauclovian-g
Copy link
Contributor Author

That issue is #2128.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Issues that document or involve technical debt topics: error-handling Issues involving the way SAW responds to an error condition topics: error-messages Issues involving the messages SAW produces on error
Projects
None yet
Development

No branches or pull requests

1 participant