Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

Conversation

@edsrzf
Copy link
Contributor

@edsrzf edsrzf commented Sep 14, 2022

This re-works the lexer to use a buffer (peek_buf method) for lookahead instead of a single byte. This allows us to use one big pattern match for most things instead of having sub-branches within each case.

There's definitely a chance I've broken something, but all the existing tests pass.

I've also pulled out some of the larger cases into separate methods and factored out common number tokenization logic.

I really want to re-work how the column and line tracking is done but when I tried I ended up going down a rabbit hole, and will look at that as a separate piece.


let kind = match char {
b'@' => {
let kind = match self.peek_buf() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kinda wonder what this type of match actually compiles down to, but assume we're not too worried about performance at this stage. It may be worth revisiting later.

It makes the code pretty nice IMHO.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you were truly interested, you could look at similar code in https://godbolt.org/ for the assembly output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I do intend to at some stage. But for now I think I'm happy with the nicer code.

@ryangjchandler
Copy link
Contributor

This is definitely a worthwhile refactor - like you say, performance isn't of huge importance to me right now. Once we've got everything working correctly, performance can be a priority.

Even in it's current state, it's tonnes faster than a PHP implementation.

@ryangjchandler ryangjchandler merged commit 3ab37c2 into php-rust-tools:main Sep 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

2 participants