Transpile all C code to Rust #3
Open
Conversation
This file contains 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
A build script named `build.rs` is picked up automatically, and I'm going to remove it in one of the following commits anyway.
It causes a compiler warning, and to the best of my knowledge it isn't being used. At least removing it has no adverse effects I can find. In addition, I'm working on transpiling this code to Rust, and the `random` is one thing that didn't carry over correctly.
This was done using the following commands: ``` bear -- cargo build c2rust transpile compile_commands.json ``` The transpiled Rust code doesn't quite work yet, which is why it's not being built yet, and the C code still exists. I will rectify this in the following commits.
It prevents the code from being compiled on stable, and it doesn't seem to be used.
They aren't actually written to, and having them be `*mut _` creates problems when calling them from `lib.rs`.
`libc::c_int` causes errors when compiling to `wasm32-unknown-unknown`. I've checked all definitions in `libc` to make sure the types are equivalent for all platforms.
`libc::c_double` causes errors when compiling to `wasm32-unknown-unknown`. I've checked all definitions in `libc` to make sure the types are equivalent for all platforms.
|
Hey @hporro, have you had a chance to think about this? If you need more time, then no rush! And if you don't want to merge this, I understand. But if this won't get merged, I'd like to know rather sooner than later, so I can make other arrangements. |
|
Hello @hannobraun! I would like to review and test the code before merging it. I really like the transpilation of the code to Rust. The problem is I'm super busy at work this days. Do you mind waiting one week more? I'll have more time since next Thursday. |
|
Thank you for getting back to me, @hporro! Please take your time. I'm not under any pressure to get this merged. I just wanted to know whether this is acceptable, in principle, because I wanted to explore alternatives if it wasn't. |
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.
The C code in this crate is a bit of a pain when it comes to cross-compiling. You need to install all kinds of compiler toolchains in a platform-specific way, while with Rust, you usually just needs a
rustup target add <my-target>and it Just Works. I figured, why not just Rewrite It In Rust, so I did.I did the translation with C2Rust (see commits for details). The generated Rust code needed some manual massaging, but nothing too bad. As far as I can tell, everything works, but I don't have any tests that would exercise all the translated functionality. What I do know, is that Fornjot still works fine using this version (also see hannobraun/fornjot#1278).
In addition, C2Rust has been in development since 2017, and is being developed by a company that specializes in this kind of work. This was the first time that I used it, but I have every reason to believe that it is a solid tool that works well.
I realize that rewriting the whole project in another language might be a controversial proposal. I also realize that the generated Rust code is far from good. However, as far as my own use case is concerned, this is the right step. The previous C code wasn't something that I would have felt comfortable updating anyway (which is not a comment on C or that code, only on my own skill and willingness). If modifications become necessary, the new Rust code provides a much better basis, at least for me.
Given that this C code is old and mature, and it seems unlikely that many updates would have been made to it anyway, I would also be fine just forking this library and moving on with my own Rust-based version (or, as an alternative, contribute the transpiled 3D primitives to
robust, if they'll have them). I figured I should open this pull request before I do that though, to prevent further fragmentation in case you want to accept these changes.