Stream: contributing

Topic: Bool toNum builtin

Jared Ramirez (Dec 20 2024 at 20:46):

Hey y'all, it's been a while since I've contributed to Roc but I have the next couple of weeks off so I'd like to get back into it a bit!

I see that @Sam Mohr create this issue to add a builtin an hour ago. This seems like a straightforward one to get started with. Since the issue was created so recently, I'm assuming it's up for grabs, but if it's not or someone thinks there's something better for me to work on, please let me know!

I'm planning on following this similar PR which adds other some builtins function!

Richard Feldman (Dec 20 2024 at 20:47):

nice, go for it! :smiley:

Anthony Bullard (Dec 20 2024 at 20:47):

You should assume if there is not an assignee, it's up for grabs!

Jared Ramirez (Dec 20 2024 at 23:16):

Okay, so small wrinkle:

Adding Bool.toNum : Bool -> Num * requires importing Num in Bool, causing a cyclical dependency (because Num already imports Bool).

One possible solution here is moving both the definitions of the types Num and Bool to a third module, then importing that from both Num and Bool. These modules could then re-export their respective types. This seems like it could have broader implications though.

What do y'all think? Has there been any thought into this kind of thing before?

Anthony Bullard (Dec 20 2024 at 23:17):

You don't need to import Num because it's a Builtin

Anthony Bullard (Dec 20 2024 at 23:17):

At least that's my understanding that it is also the case even in the Builtins package

Anthony Bullard (Dec 20 2024 at 23:18):

I'm wrong

Anthony Bullard (Dec 20 2024 at 23:19):

Don't listen to me

Anthony Bullard (Dec 20 2024 at 23:20):

I think we might then want to have Num.fromBool : Bool -> Num *

Anthony Bullard (Dec 20 2024 at 23:21):

To avoid the cycle

Jared Ramirez (Dec 20 2024 at 23:21):

Yeah, that makes sense to me!

Jared Ramirez (Dec 20 2024 at 23:24):

What's the typical process to reach consensus on something like this?

Jared Ramirez (Dec 20 2024 at 23:24):

Or we just wait for folks to weigh in?

Richard Feldman (Dec 20 2024 at 23:30):

Num.fromBool seems fine! :thumbs_up:

Jared Ramirez (Dec 21 2024 at 00:11):

Great PR is up here!

Brendan Hansknecht (Dec 21 2024 at 00:18):

Looks great, but I'm on mobile and can't give approval.

Part of me wants to guarantee it will optimize to not generate a branch, but llvm should always optimize that to not generate a branch. Just will be a branch in dev backend and normal builds

Luke Boswell (Dec 21 2024 at 00:19):

It's going to need mono snapshots regenerated i think... but I kicked off CI anyway

Luke Boswell (Dec 21 2024 at 01:06):

@Jared Ramirez are you able to update the mono snapshots?

Jared Ramirez (Dec 21 2024 at 02:42):

Yup, I can do it tomorrow! What’s the command to do so?

Anton (Dec 21 2024 at 10:58):

    cargo test -p test_mono -p uitest --no-fail-fast
    git add -u
    git commit -S -m "update mono tests"
    git push origin YOUR_BRANCH_NAME

Jared Ramirez (Dec 23 2024 at 16:42):

Great, updated the snapshots

Last updated: Jul 06 2025 at 12:14 UTC