wasmtime / PR #9903 ISLE: improve syntax BNF · git-wasmtime

I could be very wrong about this. But having this kind of infix operator defined in S-expression feels weird and I couldn't find it in the extractor parser, so I think this is a typo?

Wasmtime GitHub notifications bot (Dec 26 2024 at 08:49):

eagr created PR review comment:

technically not a value, could be confusing

Wasmtime GitHub notifications bot (Dec 26 2024 at 08:49):

eagr created PR review comment:

technically it's a field of the variant

Wasmtime GitHub notifications bot (Dec 26 2024 at 10:44):

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

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 (Dec 26 2024 at 17:24):

cfallin created PR review comment:

Actually no -- the full list of whitespace characters is here. The original list was correct.

Wasmtime GitHub notifications bot (Dec 26 2024 at 17:24):

cfallin submitted PR review:

Thanks for the PR! Unfortunately this introduces some inaccuracies and I have a lot of comments -- see below.

Wasmtime GitHub notifications bot (Dec 26 2024 at 17:24):

cfallin created PR review comment:

Eh, there is a reason I had chosen typedecl -- type is too close to ty (the latter reads as an abbreviation of the former with no semantic difference), the existing nonterminal for type expressions, and there is a semantic difference between uses of a type and the form that declares them. Let's stick with typedecl.

Wasmtime GitHub notifications bot (Dec 26 2024 at 17:24):

cfallin created PR review comment:

This is outdated syntax, sorry -- we removed argument polarity a while ago. The whole <expr> case needs to go away and you can inline <pattern> for <pattern-arg> since we don't have the distinction anymore.

Wasmtime GitHub notifications bot (Dec 26 2024 at 17:24):

cfallin created PR review comment:

Well, here I would disagree again. You can call it a "type expression" if you want, but it's a value in the universe of types. It is definitely not a typedef: typedef is, in my mind, another name for the whole form that defines a new type (like a typedef in C/C++ for example). Perhaps "type body" if you want to distinguish the semantic place this occupies -- an expanded form that can be named in a type declaration form's right-hand side but not in-line wherever types are given.

Wasmtime GitHub notifications bot (Dec 27 2024 at 04:34):

eagr submitted PR review.

Wasmtime GitHub notifications bot (Dec 27 2024 at 04:34):

eagr created PR review comment:

Should have double checked first, sorry about that.

Wasmtime GitHub notifications bot (Dec 27 2024 at 05:03):

eagr submitted PR review.

Wasmtime GitHub notifications bot (Dec 27 2024 at 05:03):

eagr created PR review comment:

Yeah, typedef is worse than typevalue.. I did mean something like "type body".

Wasmtime GitHub notifications bot (Dec 27 2024 at 05:06):

eagr edited PR review comment.

Wasmtime GitHub notifications bot (Dec 27 2024 at 05:11):

eagr commented on PR #9903:

Out of curiosity, is there a reason for convert instead of convertor, given the extractor?

Wasmtime GitHub notifications bot (Dec 27 2024 at 05:14):

eagr updated PR #9903.

Wasmtime GitHub notifications bot (Dec 27 2024 at 18:58):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Dec 27 2024 at 19:01):

cfallin commented on PR #9903:

Out of curiosity, is there a reason for convert instead of convertor, given the extractor?

Locally it made sense as "convert T to U using foo"; one could debate the many options further but we already had that discussion three(?) years ago and it's unlikely to change now absent good reason.

Wasmtime GitHub notifications bot (Dec 27 2024 at 19:17):

cfallin merged PR #9903.

Last updated: Jan 24 2025 at 00:11 UTC