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

parser+source: improvements and cleanups #934

Merged
merged 15 commits into from
Aug 31, 2023
Merged

parser+source: improvements and cleanups #934

merged 15 commits into from
Aug 31, 2023

Conversation

feds01
Copy link
Contributor

@feds01 feds01 commented Aug 31, 2023

Changes

This PR is an aggregation of various improvements and cleanups to the parser and constants API in the compiler.

Improvements to constant storage:

  • Move out methods from CONSTANT_MAP and just place them onto each Interned<ConstKind> - this improves code readability in quite a few places.
  • Don't use a HashMap for storing floats/ints as they don't need to be looked up in that manner.
  • use fxhash to look up identifiers and strings instead of the default dashmap hasher.

Improvements to the parser:

  • Use a more expressive ExpectedItem type to represent what the parser might be expecting when parsing items, which gives it more flexibility to specify expected items other than tokens. Additionally, it saves on quite a bit of memory around the place: the size of ParseResult<T> went down from a minimum bound of 70 bytes to 40 bytes.
  • Use a perfect hash function to lookup if an identifier is a keyword, there should be a 5% improvement in speed to lexing (there is more that could be done here, maybe use gprerf).
  • Collect tokenisation and generation timings, and report them as other stages do.
  • remove is_compound_expr flag on AstGen.
  • Use ThinVec for diagnostics across compiler, which should reduce several items that used to store Vecs inline (which seemed wasteful). It also optimises for the common case of no diagnostics which effectively means that it will be a std::ptr::nullptr() and only takes up 8bytes instead of 24bytes as a Vec would.
  • Share diagnostics among all AstGens which again reduces the size of an AstGen from 112 bytes to 64 bytes.
  • parser: fix bug where errors were being entirely ignored - due to not consuming errors when they should be.
  • parser: fix span of ParseWarning::RedundantParenthesees to be more accurate.

Todo

@feds01 feds01 added the parser Issues related with parsing sub-system. label Aug 31, 2023
@feds01 feds01 self-assigned this Aug 31, 2023
@feds01 feds01 requested a review from kontheocharis August 31, 2023 03:38
@feds01 feds01 changed the title parser cleanups parser+source: improvements and cleanups Aug 31, 2023
@feds01 feds01 added the core Relating to the core internals of the compiler. label Aug 31, 2023
kontheocharis
kontheocharis previously approved these changes Aug 31, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be removed

@feds01 feds01 merged commit 3204a8a into main Aug 31, 2023
1 check passed
@feds01 feds01 deleted the parser-cleanups branch August 31, 2023 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Relating to the core internals of the compiler. parser Issues related with parsing sub-system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants