Skip to content
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

Mark exported fns as extern #1002

Merged
merged 2 commits into from
Jun 22, 2023
Merged

Mark exported fns as extern #1002

merged 2 commits into from
Jun 22, 2023

Conversation

leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Jun 22, 2023

What

Mark exported fns as extern, which by default causes them to be marked as extern "C".

Why

Technically the Rust ABI is an internal detail. For an ABI stability guarantee I think we need to use the C ABI, which we can do by specifying extern.

A quick test with test vectors in this repo show that the resulting binary is identical with and without extern, so it really doesn't seems like it is need today.

Still adding because technically it seems like the correct thing to do even if it has no material impact today.

@leighmcculloch leighmcculloch marked this pull request as ready for review June 22, 2023 00:24
@leighmcculloch leighmcculloch enabled auto-merge (squash) June 22, 2023 00:24
@leighmcculloch leighmcculloch merged commit 7010f22 into main Jun 22, 2023
@leighmcculloch leighmcculloch deleted the c-abi branch June 22, 2023 17:00
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