Stream: git-wasmtime

Topic: wasmtime / PR #9529 ISLE: Add block comments


view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 16:33):

Kmeakin requested wasmtime-compiler-reviewers for a review on PR #9529.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 16:33):

Kmeakin requested elliottt for a review on PR #9529.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 16:33):

Kmeakin opened PR #9529 from Kmeakin:km/isle/block-comments to bytecodealliance:main:

Solves https://github.com/bytecodealliance/wasmtime/issues/3542.

Also adjusts the lexical syntax to be closer to wasm while I was at it ("\f" is no longer considered whitespace, but no-one uses that character anyway)

We could also align the line-comment syntax with wasm (require two semicolons to start a line comment instead of one), but that would IMO create too much churn

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 16:34):

Kmeakin edited PR #9529:

Solves https://github.com/bytecodealliance/wasmtime/issues/3542.

Also adjusts the lexical syntax to be closer to wasm while I was at it: "\r" can be used to terminate a line-comment, and "\f" is no longer considered whitespace, but no-one uses that character anyway.

We could also align the line-comment syntax with wasm (require two semicolons to start a line comment instead of one), but that would IMO create too much churn

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 16:35):

Kmeakin updated PR #9529.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 16:52):

Kmeakin updated PR #9529.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 17:11):

cfallin submitted PR review:

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 17:11):

cfallin created PR review comment:

Can we add a test for the nested block comment case as well?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 17:11):

cfallin created PR review comment:

It looks like the parser below supports nested block comments; can we represent this in the BNF?

(Maybe something like <block-char> += <block-comment>, i.e., a nested comment is one <block-char> in the parse tree)

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 17:26):

Kmeakin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 17:26):

Kmeakin created PR review comment:

The nesting is represented by the | <block-comment> alternative. I copied pretty much directly from the wasm spec: https://webassembly.github.io/spec/core/text/lexical.html#comments

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 17:39):

Kmeakin updated PR #9529.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 17:49):

Kmeakin updated PR #9529.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 19:45):

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

Subscribe to Label Action

cc @cfallin, @fitzgen

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

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

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

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 23:14):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 23:14):

cfallin created PR review comment:

Ah, I missed the last option -- very sorry. Thanks, this looks right!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 23:15):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 31 2024 at 23:30):

cfallin merged PR #9529.


Last updated: Dec 23 2024 at 12:05 UTC