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

Tracking Issue for core_ffi_c #94501

Closed
1 of 3 tasks
joshtriplett opened this issue Mar 1, 2022 · 13 comments
Closed
1 of 3 tasks

Tracking Issue for core_ffi_c #94501

joshtriplett opened this issue Mar 1, 2022 · 13 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@joshtriplett
Copy link
Member

joshtriplett commented Mar 1, 2022

Feature gate: #![feature(core_ffi_c)]

This is a tracking issue for providing the aliases for C types (c_int, c_char, etc) via core::ffi, not just via std::os::raw. The ability to interoperate with C code via FFI is not limited to crates using std; this allows using these types without std.

Public API

// core::ffi

pub type c_char = ...;
pub type c_short = ...;
pub type c_ushort = ...;
pub type c_int = ...;
pub type c_uint = ...;
// ...

The existing types in std::os::raw will become type aliases for the ones in core::ffi. This will use 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.

Steps / History

@joshtriplett joshtriplett added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Mar 1, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 2, 2022
Provide C FFI types via core::ffi, not just in std

Tracking issue: rust-lang#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.
@bstrie
Copy link
Contributor

bstrie commented Mar 2, 2022

To clarify, was #94503 able to avoid insta-stability of the new items? There seems to be some confusion as to whether or not type aliases respect stability markers.

@joshtriplett
Copy link
Member Author

joshtriplett commented Mar 3, 2022

@bstrie It appears so: core::ffi::c_ulong shows up as unstable, while std::os::raw::c_ulong shows up as stable.

@madsmtm
Copy link
Contributor

madsmtm commented Jun 7, 2022

The introductory docs for core currently says:

this library is platform-agnostic

So that should probably be modified slightly if we go with this change? (Which I am greatly in favour of btw)

@SimonSapin
Copy link
Contributor

Should these items also be available in std::ffi, in order to keep std a superset of core? (Though in this case it’s redundant with std::os::raw)

@joshtriplett
Copy link
Member Author

@SimonSapin They were, but that was reverted in 07ea143 because it caused breakage; we can re-add it as re-exports when we stabilize this.

@joshtriplett
Copy link
Member Author

The introductory docs for core currently says:

this library is platform-agnostic

So that should probably be modified slightly if we go with this change? (Which I am greatly in favour of btw)

core is platform-agnostic: it's skeptical about platforms but doesn't deny their existence

More seriously: I think the key detail here is that core doesn't offer any interfaces to platform functionality; it doesn't call into the platform. I think offering portable types whose layout is platform-specific doesn't invalidate that. core also has interfaces which use usize, whose size varies by platform.

@joshtriplett
Copy link
Member Author

Following up from Zulip:

@rust-lang/libs-api, shall we stabilize the core::ffi::c_* types?

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Jun 12, 2022

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jun 12, 2022
@BurntSushi
Copy link
Member

To be clear, this also includes adding these types to std::ffi too?

@joshtriplett
Copy link
Member Author

@BurntSushi Yes.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jun 21, 2022
@rfcbot
Copy link

rfcbot commented Jun 21, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot
Copy link

rfcbot commented Jul 1, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jul 1, 2022
@joshtriplett
Copy link
Member Author

Stabilized in #98315

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Sep 8, 2022
ojeda added a commit to ojeda/linux that referenced this issue Nov 23, 2024
Each `bindgen` release may upgrade the list of Rust targets. For instance,
currently, in their master branch [1], the latest ones are:

    Nightly => {
        vectorcall_abi: #124485,
        ptr_metadata: #81513,
        layout_for_ptr: #69835,
    },
    Stable_1_77(77) => { offset_of: #106655 },
    Stable_1_73(73) => { thiscall_abi: #42202 },
    Stable_1_71(71) => { c_unwind_abi: #106075 },
    Stable_1_68(68) => { abi_efiapi: #105795 },

By default, the highest stable release in their list is used, and users
are expected to set one if they need to support older Rust versions
(e.g. see [2]).

Thus, over time, new Rust features are used by default, and at some
point, it is likely that `bindgen` will emit Rust code that requires a
Rust version higher than our minimum (or perhaps enabling an unstable
feature). Currently, there is no problem because the maximum they have,
as seen above, is Rust 1.77.0, and our current minimum is Rust 1.78.0.

Therefore, set a Rust target explicitly now to prevent going forward in
time too much and thus getting potential build failures at some point.

Since we also support a minimum `bindgen` version, and since `bindgen`
does not support passing unknown Rust target versions, we need to use
the list of our minimum `bindgen` version, rather than the latest. So,
since `bindgen` 0.65.1 had this list [3], we need to use Rust 1.68.0:

    /// Rust stable 1.64
    ///  * `core_ffi_c` ([Tracking issue](rust-lang/rust#94501))
    => Stable_1_64 => 1.64;
    /// Rust stable 1.68
    ///  * `abi_efiapi` calling convention ([Tracking issue](rust-lang/rust#65815))
    => Stable_1_68 => 1.68;
    /// Nightly rust
    ///  * `thiscall` calling convention ([Tracking issue](rust-lang/rust#42202))
    ///  * `vectorcall` calling convention (no tracking issue)
    ///  * `c_unwind` calling convention ([Tracking issue](rust-lang/rust#74990))
    => Nightly => nightly;

    ...

    /// Latest stable release of Rust
    pub const LATEST_STABLE_RUST: RustTarget = RustTarget::Stable_1_68;

Thus add the `--rust-target 1.68` parameter. Add a comment as well
explaining this.

An alternative would be to use the currently running (i.e. actual) `rustc`
and `bindgen` versions to pick a "better" Rust target version. However,
that would introduce more moving parts depending on the user setup and
is also more complex to implement.

Cc: Christian Poveda <git@pvdrz.com>
Cc: Emilio Cobos Álvarez <emilio@crisal.io>
Link: https://github.com/rust-lang/rust-bindgen/blob/21c60f473f4e824d4aa9b2b508056320d474b110/bindgen/features.rs#L97-L105 [1]
Link: rust-lang/rust-bindgen#2960 [2]
Link: https://github.com/rust-lang/rust-bindgen/blob/7d243056d335fdc4537f7bca73c06d01aae24ddc/bindgen/features.rs#L131-L150 [3]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Nov 25, 2024
Each `bindgen` release may upgrade the list of Rust targets. For instance,
currently, in their master branch [1], the latest ones are:

    Nightly => {
        vectorcall_abi: #124485,
        ptr_metadata: #81513,
        layout_for_ptr: #69835,
    },
    Stable_1_77(77) => { offset_of: #106655 },
    Stable_1_73(73) => { thiscall_abi: #42202 },
    Stable_1_71(71) => { c_unwind_abi: #106075 },
    Stable_1_68(68) => { abi_efiapi: #105795 },

By default, the highest stable release in their list is used, and users
are expected to set one if they need to support older Rust versions
(e.g. see [2]).

Thus, over time, new Rust features are used by default, and at some
point, it is likely that `bindgen` will emit Rust code that requires a
Rust version higher than our minimum (or perhaps enabling an unstable
feature). Currently, there is no problem because the maximum they have,
as seen above, is Rust 1.77.0, and our current minimum is Rust 1.78.0.

Therefore, set a Rust target explicitly now to prevent going forward in
time too much and thus getting potential build failures at some point.

Since we also support a minimum `bindgen` version, and since `bindgen`
does not support passing unknown Rust target versions, we need to use
the list of our minimum `bindgen` version, rather than the latest. So,
since `bindgen` 0.65.1 had this list [3], we need to use Rust 1.68.0:

    /// Rust stable 1.64
    ///  * `core_ffi_c` ([Tracking issue](rust-lang/rust#94501))
    => Stable_1_64 => 1.64;
    /// Rust stable 1.68
    ///  * `abi_efiapi` calling convention ([Tracking issue](rust-lang/rust#65815))
    => Stable_1_68 => 1.68;
    /// Nightly rust
    ///  * `thiscall` calling convention ([Tracking issue](rust-lang/rust#42202))
    ///  * `vectorcall` calling convention (no tracking issue)
    ///  * `c_unwind` calling convention ([Tracking issue](rust-lang/rust#74990))
    => Nightly => nightly;

    ...

    /// Latest stable release of Rust
    pub const LATEST_STABLE_RUST: RustTarget = RustTarget::Stable_1_68;

Thus add the `--rust-target 1.68` parameter. Add a comment as well
explaining this.

An alternative would be to use the currently running (i.e. actual) `rustc`
and `bindgen` versions to pick a "better" Rust target version. However,
that would introduce more moving parts depending on the user setup and
is also more complex to implement.

Cc: Christian Poveda <git@pvdrz.com>
Cc: Emilio Cobos Álvarez <emilio@crisal.io>
Link: https://github.com/rust-lang/rust-bindgen/blob/21c60f473f4e824d4aa9b2b508056320d474b110/bindgen/features.rs#L97-L105 [1]
Link: rust-lang/rust-bindgen#2960 [2]
Link: https://github.com/rust-lang/rust-bindgen/blob/7d243056d335fdc4537f7bca73c06d01aae24ddc/bindgen/features.rs#L131-L150 [3]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants