Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RFC for Choice. #52

Merged
merged 1 commit into from
Jul 1, 2024
Merged

Add RFC for Choice. #52

merged 1 commit into from
Jul 1, 2024

Conversation

wanda-phi
Copy link
Member

@wanda-phi wanda-phi commented Mar 5, 2024

@whitequark whitequark added meta:nominated Nominated for discussion on the next relevant meeting area:core RFC affecting APIs in amaranth-lang/amaranth labels Mar 5, 2024
@whitequark
Copy link
Member

whitequark commented Mar 5, 2024

Guard syntax bikeshed:

  1. Just stuff it in the list:
Choice(foo, {
  (1, guard): bar
})
  1. Context manager:
with Choice(foo) as c:
    with c.Case(1, if=guard):
        c.eq(bar)
  1. Explicit Guard primitive:
Choice(foo, {
  Guard(1, if=guard): bar
})

with m.Switch(foo):
    with m.Case(Guard(1, if=guard)):
        ...

(I don't love any of these. They're also all backward compatible.)

@wanda-phi
Copy link
Member Author

Guard syntax bikeshed:

1. Just stuff it in the list:
Choice(foo, {
  (1, guard): bar
})

That won't work (Value is not hashable)

@wanda-phi wanda-phi force-pushed the choice branch 2 times, most recently from 0e4fa0d to ad2a0a0 Compare March 10, 2024 06:05
@whitequark whitequark removed the meta:nominated Nominated for discussion on the next relevant meeting label Mar 18, 2024
@wanda-phi wanda-phi force-pushed the choice branch 2 times, most recently from 1948dc1 to 1edf7dd Compare March 21, 2024 23:30
@whitequark whitequark added the meta:nominated Nominated for discussion on the next relevant meeting label Mar 21, 2024
@whitequark whitequark removed the meta:nominated Nominated for discussion on the next relevant meeting label Mar 28, 2024
@whitequark
Copy link
Member

whitequark commented Jun 27, 2024

In addition, the existing Mux expression is made valid on the left-hand side of an assignment, as if it was lowered as follows:

def Mux(sel, val1, val0):
    return Choice(a).case(1, val1).default(val0)

Shouldn't it be the opposite?

def Mux(sel, val1, val0):
    return Choice(a).case(0, val0).default(val1)

@wanda-phi
Copy link
Member Author

In addition, the existing Mux expression is made valid on the left-hand side of an assignment, as if it was lowered as follows:

def Mux(sel, val1, val0):
    return Choice(a).case(1, val1).default(val0)

Shouldn't it be the opposite?

def Mux(sel, val1, val0):
    return Choice(a).case(0, val0).default(val1)

fixed, thanks

@whitequark whitequark added meta:nominated Nominated for discussion on the next relevant meeting and removed meta:nominated Nominated for discussion on the next relevant meeting labels Jul 1, 2024
@whitequark
Copy link
Member

We have discussed this RFC on the 2024-07-01 weekly meeting. The disposition was to merge.

@whitequark whitequark merged commit a54d3a2 into amaranth-lang:main Jul 1, 2024
@whitequark
Copy link
Member

Thanks @wanda-phi for your work!

@wanda-phi wanda-phi deleted the choice branch July 1, 2024 18:07
whitequark added a commit that referenced this pull request Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:core RFC affecting APIs in amaranth-lang/amaranth
Development

Successfully merging this pull request may close these issues.

2 participants