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

Rewrite assembler #5

Open
wants to merge 84 commits into
base: master
Choose a base branch
from
Open

Rewrite assembler #5

wants to merge 84 commits into from

Conversation

gipsond
Copy link
Contributor

@gipsond gipsond commented Jun 21, 2022

This is a rewrite of the assembler to use the chumsky parser combinator library (and its sister error presentation library, ariadne). The main feature it enables is that the parsing step implicitly records the span of each parse tree element as a Range<usize>, instead of storing &str slices. This removes the need for explicit lifetime management which I expect would have made the old assembler difficult to maintain.

The design of this rewrite differs in a few other key ways:

  • The top-level API is more specialized to the assembler binary and TUI use cases.
  • It uses traditional compiler modules; the input is passed, in order, through: lexer, parser, parse tree analysis, assembly, and linking.
    • Notably, analyzing and generating errors happens primarily on the parse tree in one step, rather than during a series of steps which combine parsing, assembly, and error analysis.
    • I believe this has greatly improved maintainability and extensibility.
  • Almost all errors are explicitly returned in Results, rather than panicking.
    • As far as I can tell, the assembler will not panic unless a very unusual input is given.

This rewrite also includes automated tests for error cases, something the original lacked.

There is at least one significant regression: some errors do not provide as much specific information, particularly lexical errors. For example, if a source includes the invalid operand #OOPS, the current assembler would point to OOPS as an invalid decimal number, whereas this rewrite will simply indicate #OOPS is an invalid token, but not why. This rewrite does indicate all of the same errors, but not with as much specificity in all cases, though this can be improved with future work.

gipsond added 30 commits April 23, 2020 19:58
@gipsond gipsond requested a review from rrbutani June 21, 2022 22:27
@gipsond gipsond marked this pull request as ready for review June 21, 2022 22:40
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.

2 participants