search

immunant / c2rust

Migrate C code to Rust
https://c2rust.com/
Other
3.79k stars 219 forks source link

Optionally derive `Debug` for structs that allow it #1040

Open dgherzka opened 8 months ago

dgherzka commented 8 months ago

This is allowed for any struct that does not contain any unions recursively. The behavior is optional and can be enabled with --derive Debug.

dgherzka commented 8 months ago

I actually meant to open this PR against my fork, not upstream. It may be kind of rough, but it would be helpful to know whether I'm at least on the right track!

dgherzka commented 8 months ago

Because I opened this against the wrong repo, it also contains https://github.com/immunant/c2rust/pull/1030. Sorry, I'm a bit of a github noob.

dgherzka commented 8 months ago

The platform differences in va_list are making it hard to unit test this. On Linux, the struct (S3) has a lifetime parameter, but the return type of the translated fn that returns it doesn't have one, so the build fails (that must be a bug, right?). It's also unsafe to construct the object, so it can't just be a static because nothing will wrap it in unsafe {}. I'll see if just declaring pointers helps.

kkysen commented 8 months ago

Because I opened this against the wrong repo, it also contains #1030. Sorry, I'm a bit of a github noob.

Usually I stack PRs, so PR #1 is merging into main from branch1 and PR #2 is merging into branch1 from branch2. Then when I merge PR #1 and delete branch1, it updates PR #2 to be merging into main from branch2. That's when the branches belong to the repo, though, and I'm not sure how it works for forks. When #1030 merges, though, this PR should fix itself, though you may need to rebase.

dgherzka commented 8 months ago

This is now rebased and no longer includes the commits that are already in #1030.

dgherzka commented 7 months ago

Latest push changes this CLI argument and rebases the branch.

dgherzka commented 6 months ago

Marking as a draft while I make the fixes. Thanks for all the feedback!