Skip to content

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.

3 participants