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

Optimise atom_manager! implementation #914

Merged
merged 2 commits into from
Feb 15, 2024
Merged

Optimise atom_manager! implementation #914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39be186
Optimise atom_manager! implementation
psychon Feb 3, 2024
f431b67
Test & fix atom_manager! macro hygiene
psychon Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 19 additions & 17 deletions x11rb/src/x11_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,19 @@ pub use x11rb_protocol::x11_utils::{
/// #[derive(Debug)]
/// /// A handle to a response from the X11 server.
/// struct AtomCollectionCookie<'c, C: ConnectionExt> {
/// phantom: std::marker::PhantomData<&'c C>,
/// _NET_WM_NAME: Cookie<'c, C, InternAtomReply>,
/// _NET_WM_ICON: Cookie<'c, C, InternAtomReply>,
/// ATOM_WITH_SPACES: Cookie<'c, C, InternAtomReply>,
/// WHATEVER: Cookie<'c, C, InternAtomReply>,
/// // please treat the actual members as private
/// # phantom: std::marker::PhantomData<&'c C>,
/// # _NET_WM_NAME: Cookie<'c, C, InternAtomReply>,
/// # _NET_WM_ICON: Cookie<'c, C, InternAtomReply>,
/// # ATOM_WITH_SPACES: Cookie<'c, C, InternAtomReply>,
/// # WHATEVER: Cookie<'c, C, InternAtomReply>,
/// }
///
/// impl AtomCollection {
/// pub fn new<C: ConnectionExt>(
/// conn: &C,
/// ) -> Result<AtomCollectionCookie<'_, C>, ConnectionError> {
/// // This is just an example for readability; the actual code is more efficient.
/// Ok(AtomCollectionCookie {
/// phantom: std::marker::PhantomData,
/// _NET_WM_NAME: conn.intern_atom(false, b"_NET_WM_NAME")?,
Expand All @@ -70,6 +72,7 @@ pub use x11rb_protocol::x11_utils::{
/// C: ConnectionExt,
/// {
/// pub fn reply(self) -> Result<AtomCollection, ReplyError> {
/// // This is just an example for readability; the actual code is different.
/// Ok(AtomCollection {
/// _NET_WM_NAME: self._NET_WM_NAME.reply()?.atom,
/// _NET_WM_ICON: self._NET_WM_ICON.reply()?.atom,
Expand All @@ -94,10 +97,8 @@ macro_rules! atom_manager {
#[derive(Debug)]
$(#[$cookie_meta])*
$vis struct $cookie_name<'a, C: $crate::protocol::xproto::ConnectionExt> {
phantom: std::marker::PhantomData<&'a C>,
$(
$field_name: $crate::cookie::Cookie<'a, C, $crate::protocol::xproto::InternAtomReply>,
)*
__private_phantom: ::std::marker::PhantomData<&'a C>,
__private_cookies: ::std::vec::Vec<$crate::cookie::Cookie<'a, C, $crate::protocol::xproto::InternAtomReply>>,
}

// Replies
Expand All @@ -114,23 +115,24 @@ macro_rules! atom_manager {
$vis fn new<C: $crate::protocol::xproto::ConnectionExt>(
_conn: &C,
) -> ::std::result::Result<$cookie_name<'_, C>, $crate::errors::ConnectionError> {
let names = [
$($crate::__atom_manager_atom_value!($field_name$(: $atom_value)?),)*
];
let cookies: ::std::result::Result<::std::vec::Vec<_>, _>
= names.into_iter().map(|name| _conn.intern_atom(false, name)).collect();
Ok($cookie_name {
phantom: std::marker::PhantomData,
$(
$field_name: _conn.intern_atom(
false,
$crate::__atom_manager_atom_value!($field_name$(: $atom_value)?),
)?,
)*
__private_phantom: ::std::marker::PhantomData,
__private_cookies: cookies?,
})
}
}

impl<'a, C: $crate::protocol::xproto::ConnectionExt> $cookie_name<'a, C> {
$vis fn reply(self) -> ::std::result::Result<$struct_name, $crate::errors::ReplyError> {
let mut replies = self.__private_cookies.into_iter();
Ok($struct_name {
$(
$field_name: self.$field_name.reply()?.atom,
$field_name: replies.next().expect("new() should have constructed a Vec of the correct size").reply()?.atom,
)*
})
}
Expand Down
54 changes: 35 additions & 19 deletions x11rb/tests/x11_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use std::{collections::HashMap, io::IoSlice};
use x11rb::{
use ::std::{
collections::HashMap, io::IoSlice, option::Option as RealOption, result::Result as RealResult,
vec::Vec as RealVec,
};
use ::x11rb::{
atom_manager,
connection::{
BufWithFds, DiscardMode, ReplyOrError, RequestConnection, RequestKind, SequenceNumber,
Expand All @@ -11,6 +14,19 @@ use x11rb::{
x11_utils::{ExtensionInformation, Serialize, TryParse, TryParseFd},
};

// Just to be annoying, overwrite some standard types.
// We want to make sure atom_manager! does not use these directly.
mod std {}
mod x11rb {}
#[allow(dead_code)]
type Vec = ();
#[allow(dead_code)]
type Result = ();
#[allow(dead_code)]
type PhantomData = ();
#[allow(dead_code)]
type Option = ();

atom_manager! {
Atoms: AtomsCookies {
FIRST,
Expand All @@ -24,21 +40,21 @@ struct AtomFakeConnection {
// Mapping from byte string to the corresponding atom value and sequence number.
// If no entry is found, sending the InternAtom request fails.
// If the entry does not fit into u16, fetching the reply fails.
atoms_and_cookies: HashMap<Vec<u8>, u32>,
atoms_and_cookies: HashMap<RealVec<u8>, u32>,
}

impl RequestConnection for AtomFakeConnection {
type Buf = Vec<u8>;
type Buf = RealVec<u8>;

fn send_request_with_reply<R>(
&self,
bufs: &[IoSlice],
_fds: Vec<RawFdContainer>,
) -> Result<Cookie<Self, R>, ConnectionError>
_fds: RealVec<RawFdContainer>,
) -> RealResult<Cookie<Self, R>, ConnectionError>
where
R: TryParse,
{
let bytes: Vec<u8> = bufs.iter().flat_map(|buf| buf.iter().copied()).collect();
let bytes: RealVec<u8> = bufs.iter().flat_map(|buf| buf.iter().copied()).collect();
// request type and only-if-exists flag
assert_eq!(bytes[..2], [INTERN_ATOM_REQUEST, 0]);
// We ignore the length field
Expand All @@ -55,8 +71,8 @@ impl RequestConnection for AtomFakeConnection {
fn send_request_with_reply_with_fds<R>(
&self,
_bufs: &[IoSlice],
_fds: Vec<RawFdContainer>,
) -> Result<CookieWithFds<Self, R>, ConnectionError>
_fds: RealVec<RawFdContainer>,
) -> RealResult<CookieWithFds<Self, R>, ConnectionError>
where
R: TryParseFd,
{
Expand All @@ -66,8 +82,8 @@ impl RequestConnection for AtomFakeConnection {
fn send_request_without_reply(
&self,
_bufs: &[IoSlice],
_fds: Vec<RawFdContainer>,
) -> Result<VoidCookie<Self>, ConnectionError> {
_fds: RealVec<RawFdContainer>,
) -> RealResult<VoidCookie<Self>, ConnectionError> {
unimplemented!()
}

Expand All @@ -78,21 +94,21 @@ impl RequestConnection for AtomFakeConnection {
fn prefetch_extension_information(
&self,
_extension_name: &'static str,
) -> Result<(), ConnectionError> {
) -> RealResult<(), ConnectionError> {
unimplemented!();
}

fn extension_information(
&self,
_extension_name: &'static str,
) -> Result<Option<ExtensionInformation>, ConnectionError> {
) -> RealResult<RealOption<ExtensionInformation>, ConnectionError> {
unimplemented!()
}

fn wait_for_reply_or_raw_error(
&self,
sequence: SequenceNumber,
) -> Result<ReplyOrError<Vec<u8>>, ConnectionError> {
) -> RealResult<ReplyOrError<RealVec<u8>>, ConnectionError> {
let sequence_u16 = match u16::try_from(sequence) {
Ok(value) => value,
Err(_) => return Err(ConnectionError::UnsupportedExtension),
Expand All @@ -110,21 +126,21 @@ impl RequestConnection for AtomFakeConnection {
fn wait_for_reply(
&self,
_sequence: SequenceNumber,
) -> Result<Option<Vec<u8>>, ConnectionError> {
) -> RealResult<RealOption<RealVec<u8>>, ConnectionError> {
unimplemented!()
}

fn wait_for_reply_with_fds_raw(
&self,
_sequence: SequenceNumber,
) -> Result<ReplyOrError<BufWithFds<Vec<u8>>, Vec<u8>>, ConnectionError> {
) -> RealResult<ReplyOrError<BufWithFds<RealVec<u8>>, RealVec<u8>>, ConnectionError> {
unimplemented!()
}

fn check_for_raw_error(
&self,
_sequence: SequenceNumber,
) -> Result<Option<Vec<u8>>, ConnectionError> {
) -> RealResult<RealOption<RealVec<u8>>, ConnectionError> {
unimplemented!()
}

Expand All @@ -136,11 +152,11 @@ impl RequestConnection for AtomFakeConnection {
unimplemented!()
}

fn parse_error(&self, _error: &[u8]) -> Result<x11rb::x11_utils::X11Error, ParseError> {
fn parse_error(&self, _error: &[u8]) -> RealResult<::x11rb::x11_utils::X11Error, ParseError> {
unimplemented!()
}

fn parse_event(&self, _event: &[u8]) -> Result<x11rb::protocol::Event, ParseError> {
fn parse_event(&self, _event: &[u8]) -> RealResult<::x11rb::protocol::Event, ParseError> {
unimplemented!()
}
}
Expand Down
Loading