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

Implementing custom Span type. #198

Merged
merged 21 commits into from
Sep 17, 2021

Conversation

emilyaherbert
Copy link
Contributor

@emilyaherbert emilyaherbert commented Sep 1, 2021

Closes #152.

This PR implements a custom span type that allows the Span to keep track of the filename. This is useful because it could allow for users to see more descriptive error messages (#214 #73 ). As a biproduct of this change, the error types in the compiler had to change as well, with any &'sc str components changing to String components, among other small changes.

Summary

  1. Implement custom span type Span
  2. Have all parsing methods use Span

@emilyaherbert emilyaherbert added enhancement New feature or request compiler General compiler. Should eventually become more specific as the issue is triaged labels Sep 1, 2021
@emilyaherbert emilyaherbert self-assigned this Sep 1, 2021
@sezna
Copy link
Contributor

sezna commented Sep 6, 2021

Looks pretty solid so far, exactly what I was envisioning 😄

@emilyaherbert emilyaherbert marked this pull request as ready for review September 8, 2021 21:41
@sezna
Copy link
Contributor

sezna commented Sep 8, 2021

Could you clarify why we need to change all of our &'sc strs to Strings? This is a pretty significant change and introduces a lot of malloc overhead to our error/warning system. I'm not immediately seeing why we need to do this.

otrho
otrho previously approved these changes Sep 17, 2021
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although, for future reference, we should store a &'manifest PathBuf instead of a PathBuf to avoid all the allocations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom type replacement for all uses of pest::Span (to print out the file name in errors/warnings)
3 participants