Stream: git-wasmtime

Topic: wasmtime / PR #4074 ISLE: add support for implicit `=x` v...

Wasmtime GitHub notifications bot (Apr 26 2022 at 23:24):

cfallin opened PR #4074 from isle-var-bind-or-match to main:

Currently, a variable can be named in two different ways in an ISLE
pattern. One can write a pattern like (T x y) that binds the two
args of T with the subpatterns x and y, each of which match
anything and capture the value as a bound variable. Or, one can write
a pattern like (T x =x), where the first arg pattern x captures
the value in x and the second arg pattern =x matches only the same
value that was already captured.

It turns out (thanks to @fitzgen for this insight here [1]) that this
distinction can actually be inferred easily: if x isn't bound, then
mentioning it binds it; otherwise, it matches only the already-bound
variable. There's no concern about ordering (one mention binding
vs. the other) because (i) the value is equal either way, and (ii) the
types at both sites must be the same.

This language tweak seems like it should simplify things nicely! We
can remove the =x syntax later if we want, but this PR doesn't do
so.

[1] https://github.com/bytecodealliance/wasmtime/pull/4071#discussion_r859111513

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Apr 26 2022 at 23:24):

cfallin requested fitzgen for a review on PR #4074.

Wasmtime GitHub notifications bot (Apr 27 2022 at 00:12):

cfallin updated PR #4074 from isle-var-bind-or-match to main.

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:13):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:13):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:13):

fitzgen created PR review comment:

Can we keep a set/map of already-bound variables? Doing a linear search here seems suboptimal.

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:16):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:16):

cfallin created PR review comment:

Ah, I think I originally wrote it this way for two reasons:

• It allows us to push/pop bindings when we enter and leave a lexical sub-scope (a let) -- we need to record the nesting order somehow, so a map to accelerate this would have to be maintained using that linear order on the side;
• It makes it easier to refactor to allow for shadowing later (which is a feature I think we might want).

In practice with a handful of bindings it shouldn't really be a perf concern, I think...

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:20):

cfallin created PR review comment:

(If you still want us to do something better here I'm happy to think about it more, though!)

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:20):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:22):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:22):

fitzgen created PR review comment:

Eh, if it isn't a quick fix then lets leave it and deal with it if/when it actually becomes a problem

Wasmtime GitHub notifications bot (Apr 27 2022 at 20:25):

cfallin merged PR #4074.

Last updated: Nov 22 2024 at 16:03 UTC