Stream: git-wasmtime

Topic: wasmtime / PR #9108 ISLE: lexer simplifications

Wasmtime GitHub notifications bot (Aug 11 2024 at 21:34):

Kmeakin edited PR #9108.

Wasmtime GitHub notifications bot (Aug 11 2024 at 21:36):

Kmeakin edited PR #9108:

• Instead of pushing every byte in a lexed integer into a Vec<u8>, use a slice of the original buffer, and only reallocate if there are underscores that need to be removed
• Don't track line/column in Pos, so don't need to update line/column in advance_pos()
  

Wasmtime GitHub notifications bot (Aug 11 2024 at 21:48):

Kmeakin edited PR #9108:

• Instead of pushing every byte in a lexed integer into a Vec<u8>, use a slice of the original buffer, and only reallocate if there are underscores that need to be removed
• Don't track line/column in Pos, so don't need to update line/column in advance_pos()
• Store text being lexed as &str rather than &[u8] to avoid checking substrings for UTF-8 validty
  

Wasmtime GitHub notifications bot (Aug 11 2024 at 21:49):

Kmeakin updated PR #9108.

Wasmtime GitHub notifications bot (Aug 11 2024 at 21:49):

Kmeakin updated PR #9108.

Wasmtime GitHub notifications bot (Aug 11 2024 at 21:54):

Kmeakin updated PR #9108.

Wasmtime GitHub notifications bot (Aug 11 2024 at 22:05):

Kmeakin updated PR #9108.

Wasmtime GitHub notifications bot (Aug 11 2024 at 22:51):

Kmeakin updated PR #9108.

Wasmtime GitHub notifications bot (Aug 11 2024 at 22:52):

Kmeakin edited PR #9108:

• Instead of pushing every byte in a lexed integer into a Vec<u8>, use a slice of the original buffer, and only reallocate if there are underscores that need to be removed
• Don't track line/column in Pos, so don't need to update line/column in advance_pos()
• Store text being lexed as &str rather than &[u8] to avoid checking substrings for UTF-8 validity
• Don't try to parse integers twice
  

Wasmtime GitHub notifications bot (Aug 12 2024 at 01:04):

github-actions[bot] commented on PR #9108:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "isle"

Thus the following users have been cc'd because of the following labels:

• cfallin: isle
• fitzgen: isle

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Aug 12 2024 at 17:02):

pchickey commented on PR #9108:

Hi, thanks for these contributions! If I can nerd-snipe you into some unrelated lexer work, we have a logos lexer over in the wasm-wave crate that we need to be rewritten entirely in safe rust. If you are interested in learning more, ping me over on https://github.com/bytecodealliance/wasmtime/pull/8872

Wasmtime GitHub notifications bot (Aug 14 2024 at 17:03):

fitzgen submitted PR review:

This is fantastic, thanks!

Apologies for the slow review turn around, I've been having ISP issues.

One comment below that I'd like to get your take on, but shouldn't be a blocker for landing this. Can address it in a follow up, if necessary.

Wasmtime GitHub notifications bot (Aug 14 2024 at 17:03):

fitzgen submitted PR review:

This is fantastic, thanks!

Apologies for the slow review turn around, I've been having ISP issues.

One comment below that I'd like to get your take on, but shouldn't be a blocker for landing this. Can address it in a follow up, if necessary.

Wasmtime GitHub notifications bot (Aug 14 2024 at 17:03):

fitzgen created PR review comment:

Why not fn peek_char(&self) -> Option<char>? I guess a byte is mildly more performant, in theory if LLVM doesn't clean things up, but that doesn't seem super compelling compared to the simplification of "we are dealing with strings, so we should also deal with chars".

Wasmtime GitHub notifications bot (Aug 14 2024 at 17:14):

Kmeakin updated PR #9108.

Wasmtime GitHub notifications bot (Aug 14 2024 at 17:19):

fitzgen merged PR #9108.

Last updated: Dec 23 2024 at 12:05 UTC