-
-
Notifications
You must be signed in to change notification settings - Fork 366
Add roc glue to zig toolchain
#9083
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
Draft
Giesch
wants to merge
41
commits into
main
Choose a base branch
from
giesch/roc-glue
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+3,683
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* this does reflection similar to Luke's draft PR, but then just prints to stdout
* use a stubbed out roc app to allow compiling with the reflected platform
83d0745 to
f3e2ad2
Compare
* this fixes an error parsing roc-ray
* this is required after the merge from main
* and remove unused target/arch logging
Previously rocGlue caught all errors internally and returned void, causing the CLI to exit with code 0 even when errors occurred. This made CI test failures hard to diagnose since the actual error was hidden. Changes: - Return GlueError!void instead of void - Return the error after printing error message - Add try at call site to propagate errors Co-Authored-By: Claude Opus 4.5 <[email protected]>
The previous implementation used target.query.isNativeOs() which returns false when a target is explicitly specified (e.g., -Dtarget=x86_64-linux-musl), even if that target matches the host OS. This caused CI failures because the glue host library build was skipped when running with explicit musl targets. By checking target.result.os.tag == builtin.target.os.tag instead, we correctly detect when the resolved target OS matches the host, regardless of whether the target was explicitly specified or left as native. This fix allows: - Glue tests to pass in CI with -Dtarget=x86_64-linux-musl on Linux - Coverage builds to work with explicit compatible targets - Fuzzing warnings to only appear for actual cross-compilation Co-Authored-By: Claude Opus 4.5 <[email protected]>
The DebugGlue test was failing on multiple CI platforms without printing any error information. This change adds stderr/stdout output on failure to help diagnose the actual issue. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Type modules require nominal types (:=) not type aliases (:). This was causing CI failures on platforms that built fresh. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Two fixes for the glue command on Windows: 1. Escape backslashes in platform paths when writing to the synthetic app's Roc string literal. Windows paths like C:\Users\... were being interpreted as invalid escape sequences (\U, \d, etc.). 2. Properly return an error when the glue spec exits with non-zero code. Previously, the command would succeed even if the glue spec failed, causing tests to pass but not produce output files. Co-Authored-By: Claude Opus 4.5 <[email protected]>
* this duplicates the partial fix from this PR #9116 there appear to be other segfaults as well
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is currently still slop that I'm iterating on. The plan is to open the PR once C and Zig codegen are working, and then come back for Rust in another PR.
This adds an
experimental-gluecommand (name based onexperimental-lsp) that generates native platform glue code based on a Roc platform'smain.roc. The design is based on how this was done in the old Rust compiler, and Luke's example PR here, with some changes to fit into the current state of the Zig compiler. Those differences are also the reasons for theexperimental-prefix - we probably want to change them.Basic Idea (from the Rust version)
The
roc gluecommand will take a 'glue spec', a Roc script that exposes a pure function that receives build-time-reflected type information about the platform, and returns a list of native-code files to generate. Those generated files will be a type-safe API to implement effects for the given Roc platform. We'll eventually have glue specs for C, Zig, and Rust, and the idea is that it's easy for some developer to write their own for Swift or whatever.Differences from Rust compiler version
The Rust compiler compiled that glue spec to a dylib, and called it directly. We can't compile to a dylib yet in the Zig toolchain. (It sounds like that's planned to be added eventually, though). So this compiles the glue script to an executable with a glue platform, and passes the type information as a JSON-encoded CLI arg (then decoded in Zig by the glue platform). That's a bit janky and we should do the dylib thing.
The Rust compiler version type-checked the Roc platform main in a direct way to get the type information to pass to the glue script. I don't understand the compiler well enough to see how to do that off the bat, so this version is compiling a 'stub' Roc app with no type signatures and entry points that always crash, and then inspecting the output of that build. That's a bit janky and we should have a more direct 'get types for glue' API of some kind.
If either of those are blockers for merging this, that'd make sense to me. I don't really know how to address them (yet), though.
Huge thanks to Luke for the starting implementation and answering a bunch of questions.