Stream: git-wasmtime

Topic: wasmtime / issue #6082 ISLE feature request: `when` and `...

view this post on Zulip Wasmtime GitHub notifications bot (Mar 21 2023 at 20:37):

Kmeakin opened issue #6082:

Feature

Add (when <expr>) and (unless <expr>) syntax as shorthand for (if-let $true <expr>) and (if-let $false <expr>) respectively

Benefit

Less to type when adding boolean conditions to rules

Implementation

Should be simple to implement as a syntax sugar rewrite in the ISLE compiler

Alternatives

Leave ISLE unchanged

view this post on Zulip Wasmtime GitHub notifications bot (Mar 21 2023 at 20:38):

Kmeakin edited issue #6082:

Feature

Add (when <expr>) and (unless <expr>) syntax as shorthand for (if-let $true <expr>) and (if-let $false <expr>) respectively

Benefit

Less to type when adding boolean conditions to rules

Implementation

Should be simple to implement as a syntax sugar rewrite in the ISLE compiler. I would be happy to implement it myself.

Alternatives

Leave ISLE unchanged

view this post on Zulip Wasmtime GitHub notifications bot (Mar 21 2023 at 22:59):

cfallin commented on issue #6082:

We've definitely talked about this sort of thing before. I think the root issue is actually that we have ctors that communicate a predicate by their truthy return value as well as other ctors that communicate a predicate by matching or not matching. We should decide one way or another, and then make the existing (if ...) shorthand (which uses the RHS as a match/no-match predicate rather than a truthy predicate) fit that decision.

@jameysharp, IIRC there were good reasons why we started returning values more frequently rather than using the matching status itself -- was it to allow for better islec codegen?

If that's the case, one way forward would be to (i) audit all predicates and convert them from "matchy" to "truthy"; then (ii) make (if x) sugar for (if-let $true x).

This strikes me as more type-safe than the other way around: if we keep (if x) as (if-let _ x) any truthy predicate gets matched when it returns $false (the silent bug we had before), whereas with (if-let $true x) we are likely to get a type error if a matchy predicate (that e.g. returns Unit or passes through a value it took in) is used.

It is a little odd to bake in $true and the bool type to the language (currently these are a $-passthrough symbol and a type defined in the prelude, respectively) but, eh... it's certainly more ergonomic.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 21 2023 at 23:11):

jameysharp commented on issue #6082:

I like this idea. We currently have an (if <expr>) form which is also syntactic sugar for if-let, so you can model these new forms off of that.

Separately, we should probably delete the if form since it doesn't do what you'd expect. It's used in quite a few places though and we'd need to think about how to fix the existing uses.

Here's one thing that's surprising about when and unless, if implemented as syntactic sugar in the way you've described. If a constructor used anywhere in <expr> is declared partial and returns None, then the entire if-let doesn't match. So you could have a pair of rules which are identical except that one uses when and the other uses unless, and find that neither one matches. And you can't tell by looking at a rule if that's going to happen; you'd have to check every single term it references.

In the interests of reducing surprises, I think the desugared if-let should have an internal allow_partial flag, set to false for when/unless and true for a real if-let. Then, in sema.rs, in translate_iflet, pass the allow_partial flag to translate_expr where it currently just passes true for the on_lhs parameter. That'll trigger the existing checks which prohibit partial terms in other circumstances.

@cfallin, using boolean results allows us to write rules that the overlap checker can tell don't overlap without needing to set different priorities on the rules. That in turn sometimes helps generate better code, but the overlap checking was the main reason. As I recall, it was @elliottt who figured out the value of that pattern, when he started to fix overlapping rules.

Because of the value for overlap checking, I think @Kmeakin is right that it's a good idea to have both the when and unless forms. We could quibble about whether when should instead be named if. The very distant memories I have of writing Perl make me feel like that's the "right" counterpart to unless, but in the interests of not having to change everything at once, I think we should preserve the current behavior of if and migrate away from it over time.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 21 2023 at 23:33):

cfallin commented on issue #6082:

I think we should preserve the current behavior of if and migrate away from it over time.

I'm usually very much in favor of this sort of approach (see: ISLE!), but in this case I worry about the cognitive overhead and confusion to new users if if, when and unless all exist at the same time, with confusingly subtle semantic differences. I suspect that flipping the semantics of if (or renaming to when, I don't care much either) and doing one bulk-changeover in the backends would not be too bad -- a fairly mechanical change (find-replace if-let $true to if, and updating predicate definitions).

I do agree that disallowing partial constructors in if/when and unless seems right!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 04 2023 at 00:06):

Kmeakin commented on issue #6082:

Removing if would also allow us to have an (if <cond> <then> <else>) expression

view this post on Zulip Wasmtime GitHub notifications bot (May 12 2024 at 22:25):

jameysharp added the isle label to Issue #6082.

view this post on Zulip Wasmtime GitHub notifications bot (May 12 2024 at 22:25):

github-actions[bot] commented on issue #6082:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "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>

Last updated: Dec 23 2024 at 13:07 UTC