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

Hide the UnsafeCell content of opaque C++ types in regard to unwind safety #1372

Merged
merged 5 commits into from
Aug 30, 2024

Conversation

dtolnay
Copy link
Owner

@dtolnay dtolnay commented Aug 30, 2024

Opaque C++ types used to be automatically RefUnwindSafe prior to #1370, which is a fine default. This PR fixes a regression that manifests like this:

error[E0277]: the type `UnsafeCell<PhantomData<()>>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
   --> tests/test.rs:389:61
    |
389 |     let _ref_unwind_safe = |c: &ffi::C| panic::catch_unwind(|| inspect(c));
    |                                         ------------------- ^^^^^^^^^^^^^ `UnsafeCell<PhantomData<()>>` may contain interior mutability and a reference may not be safely transferrable across a catch_unwind boundary
    |                                         |
    |                                         required by a bound introduced by this call
    |
    = help: within `cxx_test_suite::ffi::C`, the trait `RefUnwindSafe` is not implemented for `UnsafeCell<PhantomData<()>>`, which is required by `{closure@tests/test.rs:389:61: 389:63}: UnwindSafe`
note: required because it appears within the type `cxx::opaque::SyncUnsafeCell<PhantomData<()>>`
   --> src/opaque.rs:24:8
    |
24  | struct SyncUnsafeCell<T>(UnsafeCell<T>);
    |        ^^^^^^^^^^^^^^
note: required because it appears within the type `cxx::private::Opaque`
   --> src/opaque.rs:16:12
    |
16  | pub struct Opaque {
    |            ^^^^^^
note: required because it appears within the type `cxx_test_suite::ffi::C`
   --> tests/ffi/lib.rs:98:14
    |
98  |         type C;
    |              ^
    = note: required for `&cxx_test_suite::ffi::C` to implement `UnwindSafe`
note: required because it's used within this closure
   --> tests/test.rs:389:61
    |
389 |     let _ref_unwind_safe = |c: &ffi::C| panic::catch_unwind(|| inspect(c));
    |                                                             ^^
note: required by a bound in `std::panic::catch_unwind`
   --> $RUSTUP_HOME/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:344:40
    |
344 | pub fn catch_unwind<F: FnOnce() -> R + UnwindSafe, R>(f: F) -> Result<R> {
    |                                        ^^^^^^^^^^ required by this bound in `catch_unwind`

    warning: adding items after statements is confusing, since items exist from the start of the scope
       --> tests/test.rs:391:5
        |
    391 |     fn require_unwind_safe<T: UnwindSafe>() {}
        |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements
        = note: `-W clippy::items-after-statements` implied by `-W clippy::pedantic`
        = help: to override `-W clippy::pedantic` add `#[allow(clippy::items_after_statements)]`

    warning: adding items after statements is confusing, since items exist from the start of the scope
       --> tests/test.rs:394:5
        |
    394 |     fn require_ref_unwind_safe<T: RefUnwindSafe>() {}
        |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements

    warning: binding to `_` prefixed variable with no side-effect
       --> tests/test.rs:388:9
        |
    388 |     let _unwind_safe = |c: UniquePtr<ffi::C>| panic::catch_unwind(|| drop(c));
        |         ^^^^^^^^^^^^
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding
        = note: `-W clippy::no-effect-underscore-binding` implied by `-W clippy::pedantic`
        = help: to override `-W clippy::pedantic` add `#[allow(clippy::no_effect_underscore_binding)]`

    warning: binding to `_` prefixed variable with no side-effect
       --> tests/test.rs:389:9
        |
    389 |     let _ref_unwind_safe = |c: &ffi::C| panic::catch_unwind(|| inspect(c));
        |         ^^^^^^^^^^^^^^^^
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding
@dtolnay dtolnay merged commit aa9abc7 into master Aug 30, 2024
30 checks passed
@dtolnay dtolnay deleted the unwind branch August 30, 2024 22:19
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.

1 participant