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

chore(errors): Remove span end offset to surface problems #1682

Closed
wants to merge 1 commit into from

Conversation

phated
Copy link
Contributor

@phated phated commented Jun 13, 2023

Description

Problem*

Resolves

Summary*

This removes the + 1 offset on span ends. This will allow us to have a clear understanding of where span start/ends might be wrong in the compiler. I first noticed this off-by-one when outputting diagnostics via the LSP and will open a follow-up issue to fix the spans in the compiler.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@phated phated requested a review from jfecher June 13, 2023 21:33
@phated phated mentioned this pull request Jun 13, 2023
@kevaundray
Copy link
Contributor

Since this doesn't produce any errors, we should check where the errors/problems are and fix them in this PR too instead of merging this in as is

@phated
Copy link
Contributor Author

phated commented Jun 14, 2023

Since this doesn't produce any errors, we should check where the errors/problems are and fix them in this PR too instead of merging this in as is

The purpose of opening this PR is so we can get issues reported for the various places this is a problem in the compiler. There could be tens/hundreds of these and I'm unfamiliar where these problems exist in the compiler so I'm looking to @kevaundray and @jfecher to fix these. Feel free to push on this PR.

@jfecher
Copy link
Contributor

jfecher commented Jun 14, 2023

I mentioned possibly more sources for error in the standup but I'm anticipating the lexer to be the one at fault here since it assigns spans to tokens to begin with. I haven't investigated yet but I'm assuming it issues inclusive-end spans when we'd want exclusive-end instead.

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.

3 participants