Skip to content

Conversation

@jschwe
Copy link
Member

@jschwe jschwe commented Nov 18, 2025

Refactor cc_flags() to return Vec, so that we can also add dynamic flags here.
This makes it easier to keep the flags in sync across bindings and compilation

Refactor cc_flags() to return Vec<String>, so that we
can also add dynamic flags here.
This makes it easier to keep the flags in sync
across bindings and compilation

Signed-off-by: Jonathan Schwender <[email protected]>
@jschwe
Copy link
Member Author

jschwe commented Nov 18, 2025

@sagudev What do you think about this? It seems that the flags we set diverge somewhat between bindings and build(), so I would like to put as much as possible in cc_flags.
There is more that could be moved into the function, but I first wanted to hear a general opinion before i spend more time.

@sagudev
Copy link
Member

sagudev commented Nov 18, 2025

@sagudev What do you think about this? It seems that the flags we set diverge somewhat between bindings and build(), so I would like to put as much as possible in cc_flags. There is more that could be moved into the function, but I first wanted to hear a general opinion before i spend more time.

I would love that. I think I already tried that in the past but hit roadblocks, hopefully you will succeed.

BTW there was also issue in bindgen I think about reusing clang ran for compilation and analysis so that would be especially interesting (although maybe more for mozangle where we do full compilation using cc-rs), but still.

@jschwe
Copy link
Member Author

jschwe commented Nov 18, 2025

BTW there was also issue in bindgen I think about reusing clang ran for compilation and analysis so that would be especially interesting (although maybe more for mozangle where we do full compilation using cc-rs), but still.

Thats an interesting thought! I found the issue and opened rust-lang/cc-rs#1614, since the suggestion to add an export flag feature to the cc-rs build does seem like a great idea, if upstream accepts it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants