Sam Mohr (Jun 22 2024 at 21:40): Sam Mohr (Jun 22 2024 at 21:40):Bit of an insane question, but is implementing Task as a built-in currently possible, but not implemented? It seems like once that and module params are implemented, the ecosystem is now able to share effectful code through pure Roc packages. If it's possible, would someone be able to help me enumerate the work required to implement it? If not, what's blocking it?
Agus Zubiaga (Jun 22 2024 at 21:41):I don't think it's blocked really. In fact, I think @Isaac Van Doren had already started looking into it.
Agus Zubiaga (Jun 22 2024 at 21:43): Sam Mohr (Jun 22 2024 at 21:45):Oh, nice! I forgot about this. I didn't see anything on @Isaac Van Doren 's fork/branches, but I'd love to work with him if possible on implementing it
Luke Boswell (Jun 22 2024 at 21:54):Yeah not blocked, would be a massive benefit to have it :partying_face:
Sam Mohr (Jun 22 2024 at 21:54):My thoughts exactly
Isaac Van Doren (Jun 22 2024 at 22:26):@Sam Mohr I haven’t made much progress so you’re welcome to start a branch! I’d be happy to collaborate also if it makes sense to.
We need the functions generated for the hosted module in the platform (defined in effect_module.rs) to implement the task module in the builtins. I’m not sure how to go about exposing these functions to the builtins. Maybe they need to be rewritten in Zig?
Sam Mohr (Jun 22 2024 at 22:57):Yes, I'd expect as much, though we'll have to see when it gets written
Sam Mohr (Jun 22 2024 at 22:58):I'll look into it tomorrow and see what I find
Sam Mohr (Jun 22 2024 at 22:58):Thanks for the tip!
Isaac Van Doren (Jun 22 2024 at 22:58):Sounds good!
Sam Mohr (Jun 24 2024 at 20:22):I've made good progress on the built-in Task implementation, which mainly has involved copying the Task module from basic-cli to crates/compiler/builtins/roc/Task.roc. Just to make sure I'm on the right path:
Effect and InternalTask, and now only Task remains. I implemented it as Effect was implemented, which means Task := {} -> Result ok err.effect_module.rs to task_module.rs and now for all foreign function definitions in the hosted module, I'm expecting them to return Task instead of EffectEffect.after, Effect.always, etc. because they acted as a bridge to Task that's no longer needed if it's in the std library (so I removed those functions)generates Effect with [...] section from our syntax and our codegen. We already have to update all hosted modules anyway to return Tasks instead, so breaking their parsing doesn't seem too crazy.basic-cli to return Tasks instead, and if that works, then I'll add testing and then release at least a draft PRDoes anyone disagree with this game plan? It might be too much for one PR (ignoring the PRs for the platforms), which would mean I will try to break up into multiple PR's, but otherwise this isn't that bad of a change to make since all of the infrastructure is already there.
Luke Boswell (Jun 24 2024 at 20:39):Sounds great. I'm happy to hack away on platforms and stage PRs when we're ready.
Sam Mohr (Jun 24 2024 at 20:42):If you're bored, you could start making PR's per platform now that declare Tasks instead of Effects for the FFI, but I'd recommend waiting until I test basic-cli integration just in case, so you don't waste your time
Sam Mohr (Jun 24 2024 at 20:58):Also, do we have a name we're considering for the hosted module instead of Effect? It seems like PlatformTask is a good name to imply that these are tasks that come from the platform. That's what I'm going to go with in the meantime
Luke Boswell (Jun 24 2024 at 20:58):It's a fair amount of work to make a change like this and I can chip in a little.
I'll wait until there's an E2E example, that will be a good point to replicate.
Ayaz Hafiz (Jun 25 2024 at 03:28):This sounds fine, I think lumping in one PR is fine. Landing this sooner is a good idea
Ayaz Hafiz (Jun 25 2024 at 03:29):is there any reason for hosted modules in the long term? i understand the need in the interim and any name seems fine, but I think there is no need to have hosted modules at all once this work is completed.
Sam Mohr (Jun 25 2024 at 03:32):Yeah, the suggestion from Richard was to use module params to declare the FFI (I think), rendering hosted module no longer needed
Ayaz Hafiz (Jun 25 2024 at 03:33):right, understood, thank you
Sam Mohr (Jun 25 2024 at 03:33):So I can't do it in the PR, but it'll be a welcome change.
Sam Mohr (Jun 25 2024 at 03:33):Should also be pretty easy since everything will already be changed to Tasks
Sam Mohr (Jun 25 2024 at 07:45):I have created two PRs, the main one in the language repo, and a basic-cli one as a platform update demo. They are both in draft mode because pretty much all of the implementation work is done, but I am running into a couple issues. The main one is that Roc says the inferred types of our generated FFI function bodies don't match their type definitions for the Task return types:
── TYPE MISMATCH in examples/cli/false-interpreter/platform/PlatformTask.roc ───
Something is off with the body of the getLine definition:
This Task.Task opaque wrapping has the type:
Task *
But the type annotation on getLine says it should be:
Task Str
If anyone has time to look at what the issue is (I expect it's in the newly-renamed task_module.rs, which should be returning a manually-built Task {} -> Result ok err but obviously something is wrong), that would be appreciated.
Sam Mohr (Jun 25 2024 at 07:46):The PR for the language looks very big, and that's because it is, but it includes a lot of minor file reformats from the new module syntax, so it's a little inflated
Sam Mohr (Jun 25 2024 at 07:57):And again, I'm happy to split this up, but for now I just wanted to get it into a draft PR so that someone else could run it if they wanted to
Sam Mohr (Jun 25 2024 at 07:59):Here's the command I ran for the above output: cargo run --bin roc -- check examples/cli/false-interpreter/False.roc
Sam Mohr (Jun 25 2024 at 08:23):@Folkert de Vries if you have free time sometime soon, could you see if you can see anything?
Sam Mohr (Jun 25 2024 at 08:27):My current theory is that we allow inference of the Task's inner closure's type and that leads to the Task *, but I don't know if setting the type during canonicalization actually is respected or if it's "paved over" during type-checking
Sam Mohr (Jun 25 2024 at 08:28):This is the code I'm referring to: https://github.com/roc-lang/roc/blob/d47a073634e79d2db69a67fd08a12b714d46b514/crates/compiler/can/src/effect_module.rs#L1401
Sam Mohr (Jun 25 2024 at 08:28):So I'm gonna try changing it here and seeing if I can get things to work
Sam Mohr (Jun 25 2024 at 08:29):If you don't have time though, don't worry about it.
Sam Mohr (Jun 26 2024 at 09:15):Okay, I fixed the issue with type misalignment, I now understand the type system a whole lot better!
Sam Mohr (Jun 26 2024 at 09:16):I'm now running into other issues with respect to building our test platforms, but it's progress
Sam Mohr (Jun 26 2024 at 09:34):Just kidding, I thought it was hanging, but it was waiting for user input :upside_down:
Luke Boswell (Jun 26 2024 at 09:50):haha, know the feeling
Richard Feldman (Jun 26 2024 at 10:50):@Sam Mohr btw a helpful tip when making changes to builtins: if you break something in a builtin .roc file, cargo check can hang because it tries to build the builtins when compiling the rest of the compiler.
to see the error message, do cargo check -vvv ("very, very verbose") - which will print out the compile error message that occurred when trying to build the .roc files for the builtins
Sam Mohr (Jun 26 2024 at 10:51):Huh... that is really good to know
Sam Mohr (Jun 26 2024 at 10:52):Not the case with this last issue (not yet mentioned in this thread)
Sam Mohr (Jun 26 2024 at 10:52):But good for the back pocket
Sam Mohr (Jun 26 2024 at 10:53):I seem to have messed up the load_internal/src/file.rs "graph" of steps somehow, it runs out of steps after solving types, I believe, and then loops forever instead of exiting
Sam Mohr (Jun 26 2024 at 10:53):Once that's fixed, then I expect everything will be working
Sam Mohr (Jun 26 2024 at 10:54):I have Tasks as a built-in working with the examples/cli/effects platform
Sam Mohr (Jun 27 2024 at 19:02):Just a quick update on this, the issue I was running into with was that there were many import pf.Task statements in the basic-cli platform. I've tested with other built-ins, and it seems like import pf.<builtin> hangs, probably because built-in imports are only ignored in the current app. Here's a repro:
app [main] { pf: platform "https://github.com/roc-lang/basic-cli/releases/download/0.10.0/vNe6s9hWzoTZtFmNkvEICPErI9ptji_ySjicO6CkucY.tar.br" }
import pf.Task
import pf.List
import pf.Stdout
main =
Stdout.line! "Hello, world!"
If I change List to List2, I get an error that it isn't found, and if I remove it, the app runs, but otherwise it hangs. I'll make an issue if I can't find an existing one for this.
Sam Mohr (Jun 27 2024 at 19:12):That being resolved, pretty much all of the CI tests are passing for basic-cli barring a couple for an odd reason. After changing the return type of roc_fx_args in platform/src/lib.rs to RocResult<RocList<RocStr>, ()>, calling the method deterministically returns one of the error tags for a different error. The Rust code cannot fail, and the type annotation on the Roc side is args : Task (List Str) *, so it shouldn't fail either. I believe I have to update the glue code, because it seems like a silent failure in the Rust -> Roc decoding, but it would be nice to have type checking between the Roc and Rust FFI if at all possible.
Sam Mohr (Jun 27 2024 at 19:14):I think I'll make an issue for that as well, since changing the return type of a Rust function doesn't cause any errors during Roc runtime. It may not be an easy add, but it means we have weaker type safety than Elm's ports, which this is somewhat analogous to.
Sam Mohr (Jun 27 2024 at 19:14):Once this issue is fixed, I'll un-draft the PR's and ask for help reviewing this change.
Agus Zubiaga (Jun 27 2024 at 19:24):Sam Mohr said:
I think I'll make an issue for that as well, since changing the return type of a Rust function doesn't cause any errors during Roc runtime. It may not be an easy add, but it means we have weaker type safety than Elm's ports, which this is somewhat analogous to.
Hm, I don't know how we could do that without slowing down every IO interaction. I feel like the goal should be to make glue more reliable, probably via roundtrip property testing.
Sam Mohr (Jun 27 2024 at 19:24):I mean at compilation, not during runtime
Sam Mohr (Jun 27 2024 at 19:24):During runtime would be terrible
Agus Zubiaga (Jun 27 2024 at 19:27):Oh, I'm not sure what that'd look like, but that's probably for another topic :smiley:
Sam Mohr (Jun 27 2024 at 19:28):This weekend if I remember I'll post in #ideas
Luke Boswell (Jun 30 2024 at 21:43):Making a list of things to land #6836
Luke Boswell (Jun 30 2024 at 21:54):I lumped basic-webserver in with the required group, because I think we need to ensure it is working OK before we make this change. Most of the basic-cli changes to replace glue types will translate directly across.
Luke Boswell (Jul 01 2024 at 01:18): Luke Boswell (Jul 01 2024 at 02:08):Upgrade and pre-release of basic-ssg READY :check:
Luke Boswell (Jul 01 2024 at 02:19):Stage pre-release URL platforms in roc-lang/roc
Luke Boswell (Jul 01 2024 at 08:08):Upgrade and pre-release of basic-webserver PR #58 READY FOR REVIEW :pray:
Sam Mohr (Jul 03 2024 at 09:01):We are chugging along with the Task as a built-in PRs, and I think I finally found the source of my FFI decoding issues. With the current Roc code in the smores56:builtin-task branch, typing the args Task as Task (Dict Str Str) * in the hosted module gives this in the CLI:
smores@smortress:~/dev/roc$ cargo run --bin roc ../basic-cli/examples/args.roc
Finished dev [unoptimized + debuginfo] target(s) in 0.15s
Running `target/debug/roc ../basic-cli/examples/args.roc`
🔨 Rebuilding platform...
Parsing args...
Program exited with error:
(StdoutErr Interrupted)
But if I change the Task declaration to args : Task (Dict Str Str) {} (a.k.a. make the type static by removing the wildcard), then the code works:
smores@smortress:~/dev/roc$ cargo run --bin roc ../basic-cli/examples/args.roc -- div
Finished dev [unoptimized + debuginfo] target(s) in 0.15s
Running `target/debug/roc ../basic-cli/examples/args.roc -- div`
🔨 Rebuilding platform...
Parsing args...
[src/lib.rs:379:9] std::env::args_os().map(|os_str|
RocStr::from(os_str.to_string_lossy().borrow())).collect() = [
"/proc/self/fd/3",
"div",
]
Error: Required option -n/--dividend is missing.
Usage:
args-example div -n/--dividend NUM -d/--divisor NUM
My theory is that RocResult is trying to decode my Result data [StdoutErr err, Exit I32 Str] as a Result data [] since the FFI definition returns RocResult<RocDict<RocStr, RocStr>, ()>. The extra bytes mean we interpret the underlying struct totally incorrectly. There's probably more testing to be done here, though I'm not entirely sure about how to do that yet.
If this indeed our issues right now, then for now I'm going to change the basic-cli hosted module to only declare static types, and in the long term we should consider interpretting Task ok * as Task ok [], or better yet interpretting Result ok err as Result ok err, even if the error gets combined into a bigger tag union.
Sam Mohr (Jul 03 2024 at 09:03):Or maybe this is just a wildcard * implementation issue, I know that there are lots of issues surrounding it at the moment.
Luke Boswell (Jul 03 2024 at 10:57):We have made a lot of progress and are very close. :smiley:
Agus Zubiaga (Jul 03 2024 at 15:52):Sam Mohr said:
Just a quick update on this, the issue I was running into with was that there were many
import pf.Taskstatements in thebasic-cliplatform. I've tested with other built-ins, and it seems likeimport pf.<builtin>hangs
Fixed in: https://github.com/roc-lang/roc/pull/6863
Luke Boswell (Jul 19 2024 at 11:22):I spent some time this evening updating the PR's for builtin-task (just merging really).
It looks like there is one or two minor issues/failures on the roc PR, but otherwise it's looking almost ready for a testing release.
I updated basic-cli and that is also looking good. There is an issue/segfault with Arg.list which I think may be something I broke or a known issue... :thinking: besides that it's also almost ready for a pre-release.
Anton (Jul 19 2024 at 12:10):There is an issue/segfault with
Arg.list...
Is that with builtin-task or without?
Luke Boswell (Jul 19 2024 at 12:13):Builtin-task
Anton (Jul 19 2024 at 14:39):I'm also now experiencing the segfault without builtin-task using the basic-cli 0.13 pre-release (built with the 0.13.x branch).
Anton (Jul 19 2024 at 15:33):Many other examples are failing as well, --linker=legacy makes the issue disappear, this could be musl related
Anton (Jul 19 2024 at 15:36):Valgrind errors:
==65301== Command: examples/LeastSquares/main
==65301==
==65301== Jump to the invalid address stated on the next line
==65301== at 0x0: ???
==65301== by 0x34E762: ??? (in /home/username/gitrepos/examples2/examples/examples/LeastSquares/main)
==65301== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==65301==
==65301==
==65301== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==65301== Bad permissions for mapped region at address 0x0
==65301== at 0x0: ???
==65301== by 0x34E762: ??? (in /home/username/gitrepos/examples2/examples/examples/LeastSquares/main)
==65301==
==65301== HEAP SUMMARY:
==65301== in use at exit: 0 bytes in 0 blocks
==65301== total heap usage: 0 allocs, 0 frees, 0 bytes allocated
Yeah, I think it is musl. I have been building locally and haven't hit this
Anton (Jul 19 2024 at 17:44):Screenshot_20240719_194128.png
Interesting diff between the unique (over both files) functions in linux-x64.rh for 0.12 and 0.13 releases
Luke Boswell (Jul 19 2024 at 19:20):Well we ripped out all the roc glue generated types and replaced them with roc std and handrolled interfaces, also bumped the toolchain significantly. Maybe that explains the size difference?
Luke Boswell (Jul 19 2024 at 19:25):For the segfault.. you just gave me an idea. What if it's a dev backend issue?
So I'm not 100% on how we used to generate surgical hosts. But we updated the preprocess host subcommand.
We generate the host using a dylib generated by roc. It currently just uses roc build --lib, but does that default to dev backend on linux and maybe if we added --optimize it would be different?
I figured it wouldn't matter because the stubbed app is what gets replaced. But maybe it confuses things somehow?
Anton (Jul 20 2024 at 09:12):bumped the toolchain ...
The rust toolchain is on 1.79 for both 0.12 and 0.13.
Anton (Jul 20 2024 at 09:17):I'll check out those leads...
I'd say the missing memset in 0.13 is definitely suspicious/problematic
Brendan Hansknecht (Jul 20 2024 at 15:35):Missing memset?
Anton (Jul 20 2024 at 15:57):Yeah, linux-x64.rh for 0.13 does not contain memset, but the same file does for 0.12
Brendan Hansknecht (Jul 20 2024 at 15:58):Rust getting smarter at removing things if you use musl?
Brendan Hansknecht (Jul 20 2024 at 15:59):The issue with musl is that toolchains tend so assume everything is static and can be cleaned up and hidden even if you use it.
Brendan Hansknecht (Jul 20 2024 at 16:00):This is probably one of the biggest reasons we need effect interpretters with all these dependency functions passed in as pointers.
Richard Feldman (Jul 20 2024 at 18:01):I don't think those are coupled, for what it's worth
Richard Feldman (Jul 20 2024 at 18:02):we could get started on passing allocator functions (and the like) in from the host anytime, really
Brendan Hansknecht (Jul 20 2024 at 19:31):Yeah, but unless we also pass all effects in from the host with the allocators, we still have the linker issues.
Richard Feldman (Jul 20 2024 at 19:35):totally, but given that both are a requirement, it's worth noting that we can start on the one anytime - and then the other one becomes the only remaining blocker to fixing the linking issue
Brendan Hansknecht (Jul 20 2024 at 19:37):Ah yeah! that definitely could be started at any time.
Anton (Jul 23 2024 at 17:45):Yeah, linux-x64.rh for 0.13 does not contain memset, but the same file does for 0.12
We were also hitting an error earlier (still happens) running cargo build without --release inside build.roc:
error: linking with `cc` failed: exit status: 1
|
= note: LC_ALL="C" PATH="/home/username/.rustup/toolchains/1.79.0-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/bin:
[...]
"/home/username/gitrepos/basic-cli/target/debug/deps/host-1af30843a687a450" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs"
= note: /usr/bin/ld: /home/username/gitrepos/basic-cli/target/debug/deps/libroc_host-2562b64005f3ca50.rlib(roc_host-2562b64005f3ca50.1t441est2wxz5auf.rcgu.o): in function `rust_main':
/home/username/gitrepos/basic-cli/crates/roc_host/src/lib.rs:339: undefined reference to `roc__mainForHost_1_exposed_size'
/usr/bin/ld: /home/username/gitrepos/basic-cli/crates/roc_host/src/lib.rs:349: undefined reference to `roc__mainForHost_1_exposed_generic'
/usr/bin/ld: /home/username/gitrepos/basic-cli/target/debug/deps/libroc_host-2562b64005f3ca50.rlib(roc_host-2562b64005f3ca50.1t441est2wxz5auf.rcgu.o): in function `roc_host::call_the_closure':
/home/username/gitrepos/basic-cli/crates/roc_host/src/lib.rs:368: undefined reference to `roc__mainForHost_0_caller'
collect2: error: ld returned 1 exit status
= note: some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified
= note: use the `-l` flag to specify native libraries to link
= note: use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-link-lib)
error: could not compile `roc_host_bin` (bin "host") due to 1 previous error
It does work with cargo build --release. There's a decent chance that if we fix this we fix the musl memset issue too.
Anton (Jul 23 2024 at 17:50):cargo build --release no longer works if I remove:
[profile.release]
lto = true
strip = "debuginfo"
codegen-units = 1
Getting close now... :)
Anton (Jul 24 2024 at 17:32):I found it :tada: :tada:
links=app was in crates/roc_host_bin/Cargo.toml but should be in crates/roc_host/Cargo.toml :expressionless:
Brendan Hansknecht (Jul 24 2024 at 18:18):I wonder if that fixes some of our other strange segfault related errors
Luke Boswell (Jul 29 2024 at 06:18):I merged latest main for both roc PR and basic-cli PR and have all :check:'s.
@Sam Mohr I think these are good to go. You still have these marked as Draft -- can you please have a look and if you are happy with these update Ready for Review?
Anton (Jul 29 2024 at 08:27):Just a heads up, Sam mentioned earlier that he was going on vacation so he may still be vacationing :p
Sam Mohr (Jul 29 2024 at 12:37):@Anton yeah, I went from Japan to now a week-long choir retreat, but I planned to have 4 hours each day of free time, so I can finally get back to working on Roc
Anton (Jul 29 2024 at 12:38):Cool, no pressure though :)
Luke Boswell (Jul 31 2024 at 21:39):@Sam Mohr do have to have all the effects be lambdas? Is there an issue on linux? it was working ok for me on my mac.
Sam Mohr (Jul 31 2024 at 21:39):So the change is coming from how the type system works
Sam Mohr (Jul 31 2024 at 21:40):The old way had the return types as [...]_
Sam Mohr (Jul 31 2024 at 21:40):Which meant that you couldn't actually use the same Task with two different error types, the compiler would infer a specific one
Sam Mohr (Jul 31 2024 at 21:41):So it felt like you could just do Arg.list!, but that was a false promise
Sam Mohr (Jul 31 2024 at 21:42):This constraint comes from the fact that a static Task ok * currently cannot be integrated into a larger error tag union
Sam Mohr (Jul 31 2024 at 21:42):Since it's a top level definition, which can only have one specialization
Sam Mohr (Jul 31 2024 at 21:44):Now, since they're all thunks, the * can actually be expanded to another tag union, meaning we can just call Arg.list! {} in context in multiple different functions with different error unions
Richard Feldman (Jul 31 2024 at 21:44):Sam Mohr said:
Since it's a top level definition, which can only have one specialization
I think that's only true for some types but not others fwiw
Richard Feldman (Jul 31 2024 at 21:44):e.g. top-level Num * can have multiple specializations
Sam Mohr (Jul 31 2024 at 21:45):When I tried just using Task ok * I got type errors
Richard Feldman (Jul 31 2024 at 21:45):yeah, some types are special-cased
Richard Feldman (Jul 31 2024 at 21:45):I think it might actually just be Num * right now
Richard Feldman (Jul 31 2024 at 21:45):we've planned to expand that list for awhile, but haven't yet
Luke Boswell (Jul 31 2024 at 21:46):I think we should try and resolve the underlying type issue if possible. It was probably ok for the Arg.list workaround, but I'm concerned if we are doing this accross the board, and then may change it back again. There will be a lot of code that breaks.
Sam Mohr (Jul 31 2024 at 21:47):If we can special case Result ok * and Task ok * that would fix this need for thunks
Sam Mohr (Jul 31 2024 at 21:48):@Luke Boswell I wasn't considering it was possible because I thought it was a constraint of the type system, but if what Rich is saying is right, then I totally agree, let's do the right way
Richard Feldman (Jul 31 2024 at 21:49):@Ayaz Hafiz do we have written down anywhere what the "expand this by induction to other types" project would entail?
Sam Mohr (Jul 31 2024 at 21:50):I can look at the Num impl tonight after rehearsal
Ayaz Hafiz (Aug 01 2024 at 03:17):i really would not recommend trying to expand this to other value types right now
Ayaz Hafiz (Aug 01 2024 at 03:18):i understand that wrapping in lambdas can be annoying but i think the only other reasonable way to compile this is to desugar all polymorphic value types to lambdas after type checking, which is not easy (it's the reason it was removed)
Ayaz Hafiz (Aug 01 2024 at 03:20):there is a simple way to do it, but the existing implementation in mono and the backends is not amenable to it IMO
Luke Boswell (Aug 01 2024 at 03:29):It sounds like a reasonable trade-off then.
Task ok * will be lambdas, even if they normally wouldn't take any arguments. Sam Mohr (Aug 01 2024 at 13:31):Okay, just saw this. I found where we determine generalizability in the compiler/constrain module here. Though as Ayaz points out, just marking * tag unions as generalizable isn't nearly where the work ends.
Sam Mohr (Aug 01 2024 at 13:32):So for now, I have all tests except for those based on the specific basic-cli version we use working in the task branch. So I think it's ready for review.
Sam Mohr (Aug 01 2024 at 13:36):I've marked the roc-lang PR as ready for review https://github.com/roc-lang/roc/pull/6836, and I'll update the summary to facilitate reviewing it, since even though the change isn't very complicated, there are a lot of files that are touched.
Luke Boswell (Aug 13 2024 at 06:09):I'm just wondering what the status on this change is?
I understand it's basically ready (looks like theres a recent conflict now), and we just need to coordinate a breaking change to make it happen and switch over. Or does this need further review? Ayaz has looked at it a few times and Sam has resolved the comments.
I'd love to help get this change over the line.
Sam Mohr (Aug 13 2024 at 06:09):Thanks for bringing this up! I was thinking about it earlier today
Sam Mohr (Aug 13 2024 at 06:10):I'll work on resolving those conflicts now
Luke Boswell (Aug 13 2024 at 06:11):I can help with basic-webserver, and merge the latest main with API changes into that branch
Sam Mohr (Aug 13 2024 at 06:11):I think once I do that, @Anton (or someone with the same perms) will need to make a test release of basic-cli, then we can ensure the roc-lang PR works
Luke Boswell (Aug 13 2024 at 06:12):I think we can make a testing release of roc by making a PR and uncommenting the right part in CI script, we just can't upload the binaries without Anton but he can do that later if it all passes.
Luke Boswell (Aug 13 2024 at 06:12):We need roc before we'll get a passing release of basic-cli right?
Sam Mohr (Aug 13 2024 at 06:13):Well, Roc can't be merged without updating the tests in Roc that use hardcoded basic-cli versions
Sam Mohr (Aug 13 2024 at 06:13):Unless we assert "it'll be fine" and just do it anyway
Sam Mohr (Aug 13 2024 at 06:13):Which feels a little iffy to me, but I'd be okay with
Luke Boswell (Aug 13 2024 at 06:14):I mean, we can make a PR to build a testing release of Roc -- I don't think that runs CI tests or anything
Luke Boswell (Aug 13 2024 at 06:15):I think we can do it like https://github.com/roc-lang/roc/pull/6908
Sam Mohr (Aug 13 2024 at 06:16):That would be great!
Luke Boswell (Aug 13 2024 at 06:18):It looks like we just make a PR from the task as builtin branch, and uncomment/comment the line in those five yaml files.
Sam Mohr (Aug 13 2024 at 06:52):@Luke Boswell just to be clear, I'd make a branch on top of my own builtin-task branch, do the YAML flips as Anton does in #6908, and then have him upload the created binary as a test release, which we'd then use to test basic-cli?
Sam Mohr (Aug 13 2024 at 06:53):After which we'd merge the basic-cli PR and then the roc PR after that?
Anton (Aug 13 2024 at 08:57):I'd love to help get this change over the line.
Yeah, I want to get this done but I really want to fix github.com/roc-lang/examples/pull/192 first
Anton (Aug 13 2024 at 09:02):btw instead of building Roc releases, it should be a lot easier to use nix to use a specific Roc commit like here. It's not that important that ubuntu CI fails as long as nix CI passes for now.
Luke Boswell (Aug 13 2024 at 09:38):btw instead of building Roc releases, it should be a lot easier to use nix to use a specific Roc commit
@Anton can we use this to merge builtin task into main though? I thought we needed a release so that we can reference that and have CI pass before we can merge the branch into main.
Anton (Aug 13 2024 at 09:40):@Anton can we use this to merge builtin task into main though?
No, for the final merge you'll need a release. But you'll want to make sure basic-cli and basic-webserver tests pass (using nix to get that specific roc commit) before you make that release.
Luke Boswell (Aug 22 2024 at 00:10):@Anton -- sorry I missed your comment on using an LLM to update the basic-webserver PR until after I fixed it.
I did a quick pass over all three PRs, and from what I can tell the roc PR :check:, and basic-cli PR :check: are good to go. There is just this LLVM issue blocking basic-webserver :cross_mark: (and therefore the Task as builtin breaking change). The fix for this will be an update to the roc PR.
+ roc build --prebuilt-platform --linker=legacy examples/dir.roc
thread 'main' panicked at /Users/luke/.cargo/git/checkouts/inkwell-55729a4c43239568/d1a5963/src/values/enums.rs:302:13:
Found StructValue(StructValue { struct_value: Value { name: "load_result", address: 0x6000030b1040, is_const: false, is_null: false, is_undef: false, llvm_value: " %load_result = load { [0 x i64], [3 x i64], i8, [7 x i8] }, ptr %result_value, align 8, !dbg !10", llvm_type: "{ [0 x i64], [3 x i64], i8, [7 x i8] }" } }) but expected PointerValue variant
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@Brendan Hansknecht -- does this LLVM look like an easy fix? I suspect there's just a typo somewhere in the task and builtin PR.
Brendan Hansknecht (Aug 22 2024 at 00:14):It looks like we have an already loaded struct but didn't realize so. As such, we are trying to load the struct from a pointer even though we already have the struct loaded.
Brendan Hansknecht (Aug 22 2024 at 00:14):Probably a minor bug
Anton (Aug 23 2024 at 09:08):sorry I missed your comment on using an LLM to update the basic-webserver PR until after I fixed it.
No problem, I got started on it but it requires some time to set up all the plumbing code, so I wanted to do it manually for now anyway
Brendan Hansknecht (Aug 24 2024 at 05:59):More detailed error and backtrace:
thread 'main' panicked at crates/compiler/gen_llvm/src/llvm/build.rs:3776:17:
"PlatformTask_task_closure_stderrLine_146823d2bf8fc0c097442048ad932e5338966c8ef5244f2d062b7bbb52d84c7": StructValue(StructValue { struct_value: Value { name: "load_result", address: 0x6000029b9c40, is_const: false, is_null: false, is_undef: false, llvm_value: " %load_result = load { [0 x i64], [3 x i64], i8, [7 x i8] }, ptr %result_value, align 8, !dbg !10", llvm_type: "{ [0 x i64], [3 x i64], i8, [7 x i8] }" } })
Union(NonRecursive([[InLayout(STR)], [InLayout(UNIT)]]))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
(base) ➜ roc git:(task-builtins) ✗ RUST_BACKTRACE=1 ROC_CHECK_MONO_IR=1 cargo run ../roc-webserver/examples/dir.roc
Finished dev [unoptimized + debuginfo] target(s) in 0.29s
Running `target/debug/roc ../roc-webserver/examples/dir.roc`
🔨 Rebuilding platform...
thread 'main' panicked at crates/compiler/gen_llvm/src/llvm/build.rs:3776:17:
"PlatformTask_task_closure_stderrLine_146823d2bf8fc0c097442048ad932e5338966c8ef5244f2d062b7bbb52d84c7": StructValue(StructValue { struct_value: Value { name: "load_result", address: 0x60000008d880, is_const: false, is_null: false, is_undef: false, llvm_value: " %load_result = load { [0 x i64], [3 x i64], i8, [7 x i8] }, ptr %result_value, align 8, !dbg !10", llvm_type: "{ [0 x i64], [3 x i64], i8, [7 x i8] }" } })
Union(NonRecursive([[InLayout(STR)], [InLayout(UNIT)]]))
stack backtrace:
0: rust_begin_unwind
at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/std/src/panicking.rs:647:5
1: core::panicking::panic_fmt
at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/panicking.rs:72:14
2: build_return
at ./crates/compiler/gen_llvm/src/llvm/build.rs:3776:17
3: build_exp_stmt
at ./crates/compiler/gen_llvm/src/llvm/build.rs:3232:13
4: build_exp_stmt
at ./crates/compiler/gen_llvm/src/llvm/build.rs:3451:21
5: build_exp_stmt
at ./crates/compiler/gen_llvm/src/llvm/build.rs:3213:26
6: build_proc
at ./crates/compiler/gen_llvm/src/llvm/build.rs:6261:16
7: build_procedures_help
at ./crates/compiler/gen_llvm/src/llvm/build.rs:5803:13
8: build_procedures
at ./crates/compiler/gen_llvm/src/llvm/build.rs:5557:25
9: gen_from_mono_module_llvm
at ./crates/compiler/build/src/program.rs:239:5
10: gen_from_mono_module
at ./crates/compiler/build/src/program.rs:125:47
11: build_loaded_file
at ./crates/compiler/build/src/program.rs:924:61
12: build_file
at ./crates/compiler/build/src/program.rs:735:5
13: build
at ./crates/cli/src/lib.rs:904:27
14: main
at ./crates/cli/src/main.rs:48:17
Caused by stderrLine somehow. Expected a pointer for somthing passed by reference. Instead got a tag with a struct in it.
Brendan Hansknecht (Aug 24 2024 at 06:00):They type looks like it may be equivalent to Result Str []
Brendan Hansknecht (Aug 24 2024 at 06:01):I'm assuming this is a bug in the signature generation of the new task hosted module equivalent.
Brendan Hansknecht (Aug 24 2024 at 08:10):Just to get this out of private chats and give an update:
Fundamentally the issue is caused by the platform api getting expanded by open tag unions and type variables. This leads to the platform static api having a different type than the final type that roc is generating.
In the specific case above we have something like this:
stdoutLine : Str -> Task {} *
What this ends up becoming in basic webserver is:
stdoutLine : Str -> Task {} [ServerErr Str]
Thus the return type of [ C Str, C {} ]. Err case of Str and Ok case of nothing.
We are fundamentally missing a way to make sure that the platform api is static and never expands to support extra tag union variants through open tag merging. This would also be hit with the platform api returning an error union that gets expanded. Cause Task Str [MyError] might end up expanding to Task Str [MyError, YourError Str] which would change the layout and break the type.
Brendan Hansknecht (Aug 24 2024 at 08:15):Do we have any way to lock down platform type variables and then programmatically expand them?
Like we really want stdoutLine : Str -> Task {} [] to mean that stdoutLine is infallible and the platform host function will return a Result {} []. We then want to force roc to deal with mapping the Result {} [] to a Result {} [ServerErr Str] such that the platform api stays restricted but the resulting type can be used as expected.
Richard Feldman (Aug 24 2024 at 11:24):hm, could we make the hosted version be Str -> Task {} {}?
Richard Feldman (Aug 24 2024 at 11:24):that doesn't express what we want it to express, but it certainly wouldn't expand!
Brendan Hansknecht (Aug 24 2024 at 16:10):I mean we can, but it is a general problem that I think we need to solve. Any platform returning a tag union or a * from host to app could break based on how it is used.
Brendan Hansknecht (Aug 24 2024 at 16:12):Also, you have to make the {} error case back to something that can merge with tags to make it happily work with await and error accumulation
Brendan Hansknecht (Aug 24 2024 at 16:13):I mean in this case you can do |> Task.onErr \_ -> Task.ok {}, but that isn't a general solution.
Richard Feldman (Aug 24 2024 at 16:15):yeah I mainly meant it as a way to unblock in the short term
Richard Feldman (Aug 24 2024 at 16:15):not as a good long term solution :big_smile:
Brendan Hansknecht (Aug 24 2024 at 16:16):I guess you could technically do this.
For everything infallible:
|> Task.err \{} -> crash "unreachable"
For everything fallible like
Task Str [Eof]
You can do:
|> Task.mapErr \err ->
when err is
# manually list out all cases
Eof -> Eof
We theoretically could even do this programmatically. Would need to ban * and [] in the API. Cause I don't know of a way to map those.
Brendan Hansknecht (Aug 24 2024 at 16:19):But maybe there is a way to cleanly map all types.
Luke Boswell (Aug 25 2024 at 11:28):Made the updates I think we need for basic-webserver.
I think I found a new bug, but I could have done something silly. I've just smashed out these changes and need take a much closer look to see, but definitely haven't seen this issue before.
$ roc build platform/libapp.roc
🔨 Rebuilding platform...
thread 'main' panicked at crates/compiler/gen_llvm/src/llvm/build.rs:3391:30:
called `Result::unwrap()` on an `Err` value: AlignmentError("The src_align_bytes argument to build_memcpy was not a power of 2.")
I think I know the issue.
Brendan Hansknecht (Aug 25 2024 at 17:38):0 isn't a valid alignment, but we generate it.
Brendan Hansknecht (Aug 25 2024 at 17:39):A zero sized type still has an alignment of 1.
Brendan Hansknecht (Aug 25 2024 at 18:49):I think this should fix it, but haven't tested on basic-webserver: https://github.com/roc-lang/roc/pull/7024
branch is align-1 if you want to test locallly.
Luke Boswell (Aug 25 2024 at 21:06):Ok, so I think we're :check: across the board now -- when testing locally using nix on my mac. :tada:
All that remains now I think some CI magic and release coordination etc.
The PR's are pointing at the latest commit on the builtin-task branch on roc.
Sam Mohr (Aug 25 2024 at 21:09):WOW
Sam Mohr (Aug 25 2024 at 21:09):I'm gonna be free in an hour for a few hours, and then tonight for a few hours after that. Or we can try to coordinate some other day soon?
Sam Mohr (Aug 25 2024 at 21:10):Or go without me! Whatever works
Luke Boswell (Aug 25 2024 at 21:10):It's all over to Anton from here I think.
Sam Mohr (Aug 25 2024 at 21:10):Thanks for all the work from @Luke Boswell , @Brendan Hansknecht , and @Anton , you've all been instrumental to this massive effort!
Luke Boswell (Aug 25 2024 at 21:12):Actually, could you give the webserver PR a once over. I made a bunch of fixes very last night, I'm going to be sitting in an airport waiting for a flight in an hour or so.
Sam Mohr (Aug 25 2024 at 21:25):Sure
Sam Mohr (Aug 25 2024 at 21:31):@Luke Boswell I left a few comments a couple days ago, but it's otherwise good, and now approved
Brendan Hansknecht (Aug 25 2024 at 22:21):@Sam Mohr
Any chance you can attempt to produce an error if a platform tasks has an unbound [] or *? Like preferably we would automatically generate the mapping I mentioned about to avoid accidentally expanding types, but this is definitely a case where it will easily lead to bugs and forces platforms to have worse apis (for example, all of our strings being used as error types for basic CLI instead of tags. That is a terrible API to workaround bugs like this cause by expanding tags).
Sam Mohr (Aug 25 2024 at 22:22):Let me try to do that once I get through security in 20 minutes
Brendan Hansknecht (Aug 25 2024 at 22:22):Like landing this with the edge cases is fine, but I really think we need a proper fix here. At a minimum we really need proper errors to guide a platform author
Sam Mohr (Aug 25 2024 at 22:23):Yeah, I was internally worried about the PR size, but it's already so big, why not make it cleaner to avoid more and more breaking changes
Luke Boswell (Aug 25 2024 at 22:30):Brendan Hansknecht said:
for example, all of our strings being used as error types for basic CLI instead of tags. That is a terrible API to workaround bugs like this cause by expanding tags).
Can we please break this into a separate branch and PR? (on top of builtin-task)
I've got everything happy on the builtin-task branch, and we can merge this later as a non-breaking change in roc.
Not a big problem, just have to do another round of nix flake updates and running all the tests.
Sam Mohr (Aug 25 2024 at 22:33):It seems like just waiting for everything to merge would be easier, then. How bad would doing it right after merging this be, @Brendan Hansknecht ?
Brendan Hansknecht (Aug 25 2024 at 22:37):Feel free to merge. I just don't want this to be forgotten
Brendan Hansknecht (Aug 25 2024 at 22:38):It just a really easy edge to hit and the worse part is the platform author may not be aware the bug exists until a user writes complex enough code to break things
Sam Mohr (Aug 25 2024 at 22:39):I will DEFINITELY handle it in the next 48 hours
Luke Boswell (Aug 25 2024 at 22:43):Anton will be online in 12hrs or so... if the PR is merging into the builtin-task branch he may be able to include it in the release. We will need a pre-release of basic-cli first anyway before either of the roc PR's could pass CI.
My thoughts are if we keep it in a separate branch/PR then Anton can decide what will be easiest.
For similar changes (against breaking releases) he has asked me to do it this way.
Sam Mohr (Aug 25 2024 at 22:45):Okay, on it
Luke Boswell (Aug 26 2024 at 04:37):I think I found a blocking issue, just looking into it now. It only comes up when you go to build a URL package for the platform.
Maybe we should add a check in our CI for this on basic-cli/webserver (just using .tar.gz).
14:34:04 ~/Documents/GitHub/basic-webserver release $ roc build --bundle .tar.br platform/main.roc
Compressing with Brotli at maximum quality level…
(Note: Brotli compression can take awhile! Using --bundle .tar.gz takes less time, but usually produces a significantly larger output file. Brotli is generally worth the up-front wait if this is a file people will be downloading!)
thread 'main' panicked at crates/packaging/src/tarball.rs:279:9:
not yet implemented
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Blocking, but seemingly unrelated. Do you disagree?
Sam Mohr (Aug 26 2024 at 04:38):That seems like a good thing to test, unless it would significantly lengthen our CI testing wait
Luke Boswell (Aug 26 2024 at 04:39):Well it has to be related I think, it's a panic when parsing a platform module header
Luke Boswell (Aug 26 2024 at 04:54):Ok, maybe it's not a problem... and I had just done something strange locally
Luke Boswell (Aug 26 2024 at 04:57):Yeah, sorry I think that was a false alarm.
Luke Boswell (Aug 26 2024 at 04:57):I must have messed something up with versions of Roc
Sam Mohr (Aug 26 2024 at 04:58):Yeah, I was gonna point out that you used roc, not /Users/mrcoolguy/roc/target/release/roc
Sam Mohr (Aug 26 2024 at 04:59):Testing that now
Luke Boswell (Aug 26 2024 at 04:59):Yeah, that was the giveaway actually... because my nix shell normally looks like 192-168-1-103:basic-cli luke$
Luke Boswell (Aug 26 2024 at 04:59):So the version of roc I have my OS pointing to is definitely long since out of date
Sam Mohr (Aug 26 2024 at 04:59):Yep, it broke with global roc, and worked with the Task version
Sam Mohr (Aug 26 2024 at 05:00):I use rocup, and because of the recent unblocking efforts, the latest binary is from August 23rd
Sam Mohr (Aug 26 2024 at 05:00):Would recommend rocup
Luke Boswell (Aug 26 2024 at 05:02):I'm switching between branches so much I couldn't imagine doing things without nix now.
Sam Mohr (Aug 26 2024 at 05:02):That's the way to be, yes
Sam Mohr (Aug 26 2024 at 05:03):Not to get too off topic, but: I use rocup for general package dev because it's easier, and nix for everything else (e.g. language/platform dev)
Luke Boswell (Aug 26 2024 at 05:14):https://github.com/lukewilliamboswell/roc-htmx-tailwindcss-demo/pull/1
Upgraded the roc-htmx-tailwindcss demo app using a release I made and it's working well :smiley: 
Sam Mohr (Aug 26 2024 at 05:21):Approved!
Luke Boswell (Aug 26 2024 at 05:59):I'm trying to update rtl to builtin task.. and seeing a cryptic type error that may or may not be related...
15:58:55 ~/Documents/GitHub/rtl builtin-task $ roc check rtl.roc
── TYPE MISMATCH in rtl.roc ────────────────────────────────────────────────────
This 1st argument to this function has an unexpected type:
1│ app [main] {
This Utc.now value is a:
({} -> Task Utc.Utc *)
But this function needs its 1st argument to be:
Task a b
Tip: Add type annotations to functions or values to help you figure
this out.
────────────────────────────────────────────────────────────────────────────────
1 error and 14 warnings found in 48 ms
main =
start = Utc.now! {}
cliParser =
{ Cli.combine <-
<---- snipped---->
The implication there is that the desugaring for the ! bangs are not adding regions, so they show their type errors at the start of the document
Sam Mohr (Aug 26 2024 at 06:18):If you add explicit annotations to the definitions, do you see anything more helpful?
Luke Boswell (Aug 26 2024 at 06:26):It doesn't really help
main : Task {} [Exit I32 Str]_
main =
start : Utc
start = Utc.now! {}
cliParser : Cli.CliParser { maybeInputDir : _, maybeOutputDir : _, maybeExtension : _ }
cliParser =
{ Cli.combine <-
$ roc check rtl.roc
── TYPE MISMATCH in rtl.roc ────────────────────────────────────────────────────
This 1st argument to this function has an unexpected type:
1│ app [main] {
This Utc.now value is a:
({} -> Task Utc.Utc *)
But this function needs its 1st argument to be:
Task a b
Tip: Add type annotations to functions or values to help you figure
this out.
────────────────────────────────────────────────────────────────────────────────
1 error and 14 warnings found in 46 ms
I'll check it out
Sam Mohr (Aug 26 2024 at 06:43):There's another Utc.now! on line 94 that needs {} as an arg
Sam Mohr (Aug 26 2024 at 06:44):@Luke Boswell
Luke Boswell (Aug 26 2024 at 06:50)::sweat_smile:
Luke Boswell (Aug 26 2024 at 06:51):Oh man, I'd really like regions in that error message
Sam Mohr (Aug 26 2024 at 06:52):Yeah...
Anton (Aug 26 2024 at 18:54):PRs have been reviewed, I'll build the TESTING release tomorrow :rocket:
Sam Mohr (Aug 26 2024 at 22:06):I've made the requested changes on the PR, we're looking good
Anton (Aug 27 2024 at 18:18):I ran into No borrow signature for LambdaName on roc build ./examples/Tasks/main.rocwhen testing the examples with the latest basic-cli pre-release.
I'm done for today, so feel free to take a stab at it
Sam Mohr (Aug 27 2024 at 18:20):I'll check it out later
Anton (Aug 27 2024 at 18:22):I forgot to do roc check, I need to add that to the error message:
❯ roc check ./examples/Tasks/main.roc
── TYPE MISMATCH in ./examples/Tasks/main.roc ──────────────────────────────────
This 1st argument to this function has an unexpected type:
21│ startTime = Utc.now!
^^^^^^^^
This Utc.now value is a:
({} -> Task Utc.Utc *)
But this function needs its 1st argument to be:
Task a b
Tip: Add type annotations to functions or values to help you figure
this out.
── TYPE MISMATCH in ./examples/Tasks/main.roc ──────────────────────────────────
This 1st argument to this function has an unexpected type:
47│ endTime = Utc.now!
^^^^^^^^
This Utc.now value is a:
({} -> Task Utc.Utc *)
But this function needs its 1st argument to be:
Task a b
Tip: Add type annotations to functions or values to help you figure
this out.
────────────────────────────────────────────────────────────────────────────────
2 errors and 0 warnings found in 67 ms
I think I saw a similar issue recently, but I can't find it anymore
Sam Mohr (Aug 27 2024 at 18:23):Easy fix, just add a {} because they're thunks
Sam Mohr (Aug 27 2024 at 18:23):I can do it
Anton (Aug 27 2024 at 18:25):Haha, I glossed over the tip (I wrote that one too :p ):
Tip: This can happen when you call a function with less arguments than it expects.
Like `Arg.list!` instead of `Arg.list! {}`
Could someone actually give me contributor access for roc/examples?
Sam Mohr (Aug 27 2024 at 18:40):I don't believe I can help with this PR without it
Richard Feldman (Aug 27 2024 at 19:33):done!
Sam Mohr (Aug 27 2024 at 19:58):Thanks!
Anton (Aug 30 2024 at 18:12):It's nice that hello world is now shorter and also does not contain a new concept (for beginners) Task:
app [main] { pf: platform "https://github.com/roc-lang/basic-cli/releases/download/0.15.0/SlwdbJ-3GR7uBWQo6zlmYWNYOxnvo8r6YABXD-45UOw.tar.br" }
import pf.Stdout
main =
Stdout.line! "Hello, World!"
Last updated: Jul 05 2025 at 12:14 UTC