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.

Sign up for GitHub

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

Jump to bottom

Provide C FFI types via core::ffi, not just in std #94503

Merged
merged 3 commits into from
Mar 2, 2022
Merged

Provide C FFI types via core::ffi, not just in std #94503

merged 3 commits into from
Mar 2, 2022

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Mar 1, 2022 •
edited
Loading

Tracking issue: #94501

The ability to interoperate with C code via FFI is not limited to crates
using std; this allows using these types without std.

The existing types in std::os::raw become type aliases for the ones in
core::ffi. This uses type aliases rather than re-exports, to allow the
std types to remain stable while the core types are unstable.

This also moves the currently unstable NonZero_ variants and
c_size_t/c_ssize_t/c_ptrdiff_t types to core::ffi, while leaving
them unstable.

Historically, we didn't do this because these types are target-dependent.
However, core itself is also target-dependent. core should not call
any OS services, but it knows the target and the target's ABI.

@rust-highfive
Copy link
Collaborator

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 1, 2022
@joshtriplett joshtriplett mentioned this pull request Mar 1, 2022
Closed
3 tasks
@joshtriplett
Copy link
Member Author

r? @Amanieu

@rust-highfive rust-highfive assigned Amanieu and unassigned kennytm Mar 1, 2022
@joshtriplett joshtriplett added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Mar 1, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
Amanieu
Amanieu reviewed Mar 1, 2022
View reviewed changes
Copy link
Member

@Amanieu Amanieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see some additional C types here, specifically all the ones that are defined in stddef.h and stdint.h. These two headers are part of the freestanding subset of C which has no OS dependencies.

Specifically I would like to see the C99 fixed-width integer types and:

  • wchar_t
  • char8_t, char16_t, char32_t
  • max_align_t

The idea is that anyone interface with C code should be able to blindly use the equivalent c_* type without having to think too much about what the exact equivalent Rust type is.

library/core/src/ffi.rs Show resolved Hide resolved
library/core/src/ffi.rs Show resolved Hide resolved
@joshtriplett
Copy link
Member Author

joshtriplett commented Mar 1, 2022 •
edited
Loading

@Amanieu I'd absolutely love to add more C FFI types, but I really don't want this PR to do anything more than move the types we already have. Similarly, the ssize_t and similar types are feature-gated for various reasons, and this PR leaves them feature-gated, so moving them shouldn't be conditional on deciding if they should be un-feature-gated.

I'm trying to make zero semantic changes here apart from the move, to make this easy to review and approve.

@joshtriplett joshtriplett force-pushed the core-ffi-c branch 2 times, most recently from 0974b39 to 06db9fa Compare March 2, 2022 00:09
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@joshtriplett
Add a copy of cfg_if to core's internal_macros.rs
0f505c6 
core can't depend on external crates the way std can. Rather than revert
usage of cfg_if, add a copy of it to core. This does not export our
copy, even unstably; such a change could occur in a later commit.
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Amanieu
Copy link
Member

Amanieu commented Mar 2, 2022

r=me once CI passes

@joshtriplett joshtriplett force-pushed the core-ffi-c branch 2 times, most recently from 459f3f3 to 34ada45 Compare March 2, 2022 00:45
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@joshtriplett
Provide C FFI types via core::ffi, not just in std
335c960 
The ability to interoperate with C code via FFI is not limited to crates
using std; this allows using these types without std.

The existing types in `std::os::raw` become type aliases for the ones in
`core::ffi`. This uses type aliases rather than re-exports, to allow the
std types to remain stable while the core types are unstable.

This also moves the currently unstable `NonZero_` variants and
`c_size_t`/`c_ssize_t`/`c_ptrdiff_t` types to `core::ffi`, while leaving
them unstable.
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@joshtriplett
Temporarily make CStr not a link in the c_char docs
75c3e9c 
When CStr moves to core with an alias in std, this can link to
`crate::ffi::CStr`. However, linking in the reverse direction (from core
to std) requires a relative path, and that path can't work from both
core::ffi and std::os::raw (different number of `../` traversals
required).
@joshtriplett
Copy link
Member Author

@bors r=@Amanieu

@bors
Copy link
Contributor

bors commented Mar 2, 2022

📌 Commit 75c3e9c has been approved by Amanieu

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2022
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 2, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 2, 2022
@bors
Auto merge of rust-lang#94514 - matthiaskrgr:rollup-pdzn82h, r=matthi…
2a280de 
…askrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#94464 (Suggest adding a new lifetime parameter when two elided lifetimes should match up for traits and impls.)
 - rust-lang#94476 (7 - Make more use of `let_chains`)
 - rust-lang#94478 (Fix panic when handling intra doc links generated from macro)
 - rust-lang#94482 (compiler: fix some typos)
 - rust-lang#94490 (Update books)
 - rust-lang#94496 (tests: accept llvm intrinsic in align-checking test)
 - rust-lang#94498 (9 - Make more use of `let_chains`)
 - rust-lang#94503 (Provide C FFI types via core::ffi, not just in std)
 - rust-lang#94513 (update Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3ea9eeb into rust-lang:master Mar 2, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 2, 2022
@semarie semarie mentioned this pull request Mar 2, 2022
Merged
@joshtriplett joshtriplett deleted the core-ffi-c branch March 2, 2022 11:03
This was referenced Mar 2, 2022
Merged
Closed
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Apr 26, 2022
Closed
@ojeda ojeda mentioned this pull request Jun 29, 2022
Open
3 tasks
@ojeda ojeda mentioned this pull request May 20, 2024
Open
81 tasks
@taiki-e taiki-e mentioned this pull request Sep 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Amanieu Amanieu Amanieu left review comments

Assignees

@Amanieu Amanieu

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.61.0
Development

Successfully merging this pull request may close these issues.
7 participants
@joshtriplett @rust-highfive @rust-log-analyzer @Amanieu @bors @kennytm @rustbot