Skip to content

Commit

Permalink
Use a wrapper type implementing Sync, instead of static mut (#700)
Browse files Browse the repository at this point in the history
* Use a wrapper type implementing `Sync`, instead of `static mut`

`static mut` is generally best avoided. It seems to only be necessary
currently to allow the use of non-`Sync` types, since pointers are not
sync.

This also removes the `NULLPTR` constant in favor of using
`std::ptr::null`, uses `.as_ptr()` instead of casting, and uses the
arrray repeating syntax to define `types_null`. None of those should
impact behavior or the public API.

`*_requests` and `*_events` static are no longer public now. This is
theoretically a breaking change, but shouldn't really impact anything.

* Suppress `clippy::test_attr_in_doctest` lint

* Address nightly warnings
  • Loading branch information
ids1024 authored Apr 2, 2024
1 parent eb7a57b commit ea03873
Show file tree
Hide file tree
Showing 9 changed files with 452 additions and 422 deletions.
4 changes: 1 addition & 3 deletions wayland-backend/src/rs/client_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ use super::{
wire::MessageParseError,
};

pub use crate::types::client::{InvalidId, NoWaylandLib, WaylandError};

#[derive(Debug, Clone)]
struct Data {
client_destroyed: bool,
Expand Down Expand Up @@ -407,7 +405,7 @@ impl InnerBackend {
// Prepare the message in a debug-compatible way
let args = args.into_iter().map(|arg| {
if let Argument::NewId(ObjectId { id: p }) = arg {
if !p.id == 0 {
if p.id != 0 {
panic!("The newid provided when sending request {}@{}.{} is not a placeholder.", object.interface.name, id.id, message_desc.name);
}
if let Some((child_id, child_serial, child_interface)) = child {
Expand Down
1 change: 0 additions & 1 deletion wayland-backend/src/rs/wire.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Types and routines used to manipulate arguments from the wire format

use std::collections::VecDeque;
use std::convert::TryInto;
use std::ffi::CStr;
use std::os::unix::io::RawFd;
use std::os::unix::io::{BorrowedFd, OwnedFd};
Expand Down
2 changes: 0 additions & 2 deletions wayland-backend/src/sys/client_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ use smallvec::SmallVec;

use wayland_sys::{client::*, common::*, ffi_dispatch};

pub use crate::types::client::{InvalidId, NoWaylandLib, WaylandError};

use super::{free_arrays, RUST_MANAGED};

use super::client::*;
Expand Down
1 change: 1 addition & 0 deletions wayland-backend/src/sys/server_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use wayland_sys::{common::*, ffi_dispatch, server::*};

use super::{free_arrays, server::*, RUST_MANAGED};

#[allow(unused_imports)]
pub use crate::types::server::{Credentials, DisconnectReason, GlobalInfo, InitError, InvalidId};

scoped_thread_local! {
Expand Down
2 changes: 2 additions & 0 deletions wayland-backend/tests/rs_sys_impls.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::test_attr_in_doctest)]

//! Tests to ensure the rust and sys types implement the same traits.

/// A macro used to assert a type defined in both the rust and sys implementations of wayland-backend
Expand Down
2 changes: 2 additions & 0 deletions wayland-scanner/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Unreleased

- Use wrapper type implementing `Sync` instead of `static mut`s.

## 0.31.1 -- 2024-01-29

- Include an `std::convert::Infallible` in hidden `__phantom_lifetime` enum variants,
Expand Down
38 changes: 18 additions & 20 deletions wayland-scanner/src/c_interfaces.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::cmp;
use std::iter::repeat;

use proc_macro2::{Literal, TokenStream};
use quote::{format_ident, quote};
Expand Down Expand Up @@ -27,14 +26,13 @@ pub(crate) fn generate_interfaces_prefix(protocol: &Protocol) -> TokenStream {

let types_null_len = Literal::usize_unsuffixed(longest_nulls);

let nulls = repeat(quote! { NULLPTR as *const wayland_backend::protocol::wl_interface })
.take(longest_nulls);

quote! {
const NULLPTR: *const std::os::raw::c_void = 0 as *const std::os::raw::c_void;
static mut types_null: [*const wayland_backend::protocol::wl_interface; #types_null_len] = [
#(#nulls,)*
];
use std::ptr::null;
struct SyncWrapper<T>(T);
unsafe impl<T> Sync for SyncWrapper<T> {}
static types_null: SyncWrapper<[*const wayland_backend::protocol::wl_interface; #types_null_len]> = SyncWrapper([
null::<wayland_backend::protocol::wl_interface>(); #types_null_len
]);
}
}

Expand All @@ -47,24 +45,24 @@ pub(crate) fn generate_interface(interface: &Interface) -> TokenStream {
let version_value = Literal::i32_unsuffixed(interface.version as i32);
let request_count_value = Literal::i32_unsuffixed(interface.requests.len() as i32);
let requests_value = if interface.requests.is_empty() {
quote! { NULLPTR as *const wayland_backend::protocol::wl_message }
quote! { null::<wayland_backend::protocol::wl_message>() }
} else {
let requests_ident = format_ident!("{}_requests", interface.name);
quote! { unsafe { &#requests_ident as *const _ } }
quote! { #requests_ident.0.as_ptr() }
};
let event_count_value = Literal::i32_unsuffixed(interface.events.len() as i32);
let events_value = if interface.events.is_empty() {
quote! { NULLPTR as *const wayland_backend::protocol::wl_message }
quote! { null::<wayland_backend::protocol::wl_message>() }
} else {
let events_ident = format_ident!("{}_events", interface.name);
quote! { unsafe { &#events_ident as *const _ } }
quote! { #events_ident.0.as_ptr() }
};

quote! {
#requests
#events

pub static mut #interface_ident: wayland_backend::protocol::wl_interface = wayland_backend::protocol::wl_interface {
pub static #interface_ident: wayland_backend::protocol::wl_interface = wayland_backend::protocol::wl_interface {
name: #name_value as *const u8 as *const std::os::raw::c_char,
version: #version_value,
request_count: #request_count_value,
Expand All @@ -89,15 +87,15 @@ fn gen_messages(interface: &Interface, messages: &[Message], which: &str) -> Tok
let array_values = msg.args.iter().map(|arg| match (arg.typ, &arg.interface) {
(Type::Object, &Some(ref inter)) | (Type::NewId, &Some(ref inter)) => {
let interface_ident =format_ident!("{}_interface", inter);
quote! { unsafe { &#interface_ident as *const wayland_backend::protocol::wl_interface } }
quote! { &#interface_ident as *const wayland_backend::protocol::wl_interface }
}
_ => quote! { NULLPTR as *const wayland_backend::protocol::wl_interface },
_ => quote! { null::<wayland_backend::protocol::wl_interface>() },
});

Some(quote! {
static mut #array_ident: [*const wayland_backend::protocol::wl_interface; #array_len] = [
static #array_ident: SyncWrapper<[*const wayland_backend::protocol::wl_interface; #array_len]> = SyncWrapper([
#(#array_values,)*
];
]);
})
}
});
Expand All @@ -118,17 +116,17 @@ fn gen_messages(interface: &Interface, messages: &[Message], which: &str) -> Tok
wayland_backend::protocol::wl_message {
name: #name_value as *const u8 as *const std::os::raw::c_char,
signature: #signature_value as *const u8 as *const std::os::raw::c_char,
types: unsafe { &#types_ident as *const _ },
types: #types_ident.0.as_ptr(),
}
}
});

quote! {
#(#types_arrays)*

pub static mut #message_array_ident: [wayland_backend::protocol::wl_message; #message_array_len] = [
static #message_array_ident: SyncWrapper<[wayland_backend::protocol::wl_message; #message_array_len]> = SyncWrapper([
#(#message_array_values,)*
];
]);
}
}

Expand Down
Loading

0 comments on commit ea03873

Please sign in to comment.