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

Generate unsafe extern blocks (Edition 2024) #2901

Closed
marmitar opened this issue Aug 18, 2024 · 5 comments · Fixed by #3015
Closed

Generate unsafe extern blocks (Edition 2024) #2901

marmitar opened this issue Aug 18, 2024 · 5 comments · Fixed by #3015

Comments

@marmitar
Copy link

marmitar commented Aug 18, 2024

I know Edition 2024 is very experimental at the moment, so this is more of a heads-up than an actual issue.

Description

Rust has just implemented RFC 3484, requiring all extern blocks to be marked unsafe extern in Edition 2024. Older editions don't yet compile this unsafe extern construct (as of the latest stable, rustc-1.80.0), but they will eventually accept the unsafe keyword for compatibility reasons. This crate needs some way of generating bindings with this future requirements, either via Builder configuration, or by detecting the current edition.

I've implemented a minimal (but complete) example that triggers the error here: bindgen-unsafe-extern.

Input C/C++ Header

void cool_function(int i, char c);

Bindgen Invocation

bindgen::Builder::default()
    .header("src/cool.h")
    .generate()
    .unwrap()
    .write_to_file("src/cool.rs")
    .unwrap()

Actual Results

Generated file:

extern "C" {
    pub fn cool_function(i: ::std::os::raw::c_int, c: ::std::os::raw::c_char);
}

Compiler error:

error: extern blocks must be unsafe
 --> src/cool.rs:3:1
  |
3 | / extern "C" {
4 | |     pub fn cool_function(i: ::std::os::raw::c_int, c: ::std::os::raw::c_char);
5 | | }
  | |_^

Expected Results

For Edition 2024, the generated file should be:

unsafe extern "C" {
    pub fn cool_function(i: ::std::os::raw::c_int, c: ::std::os::raw::c_char);
}
@GKFX
Copy link
Contributor

GKFX commented Aug 18, 2024

It's worth noting that Edition 2024 safe extern functions, plus either the proposed [[safe]] attribute for C (N2659) or an equivalent, would solve #2774.

@ojeda
Copy link
Contributor

ojeda commented Aug 18, 2024

The proposed [[safe]] attribute did not get enough support from the committee back then, but lately memory safety is talked about more in WG14 and is in the latest charter, so perhaps things change. Rust safe items within unsafe extern blocks could help there.

Or perhaps a C implementation could add something like that to improve interop with Rust for their users... :)

@ehuss
Copy link
Contributor

ehuss commented Nov 11, 2024

Just FYI, unsafe extern is now stabilized in all editions in 1.82.0 (2024-10-17). And just a heads up that we are still aiming to have the 2024 edition hit stable in 1.85 (2025-02-20).

uklotzde pushed a commit to HMIProject/open62541-sys that referenced this issue Nov 15, 2024
## Description

This replaces the declaration of `extern "C"` blocks with `unsafe extern
"C"` as will be required by Rust 2024. Remove this when
rust-lang/rust-bindgen#2901 is merged and
[bindgen](https://crates.io/crates/bindgen) can generate these blocks
automatically as required.
@pvdrz
Copy link
Contributor

pvdrz commented Nov 30, 2024

once #3002 is merged, bindgen will have support for editions. However, it seems to me that this should be enabled regardless of the edition for every rust target greater or equal than 1.82 as not using unsafe extern will generate a warning for all the editions before 2024 anyway. Is that correct?

@ojeda
Copy link
Contributor

ojeda commented Dec 1, 2024

Yeah, as far as I can see, the lint is not enabled by default (rust_2024_compatibility or missing_unsafe_on_extern) but it can be used starting with 1.82 even for 2015, so it could help those that want to enable the warning.

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

Successfully merging a pull request may close this issue.

5 participants