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

Gate some commonly unused traits behind a feature #884

Merged
merged 6 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
27 changes: 24 additions & 3 deletions generator/src/generator/namespace/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,22 @@ pub(crate) struct EnumInfo {
pub(super) wire_size: Option<(u8, u8)>,
}

pub(super) fn default_debug_impl(name: &str, out: &mut crate::generator::Output) {
outln!(out, "#[cfg(not(feature = \"extra-traits\"))]");
outln!(out, "impl core::fmt::Debug for {} {{", name);
out.indented(|out| {
outln!(
out,
"fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {{"
);
out.indented(|out| {
outln!(out, "f.debug_struct(\"{}\").finish_non_exhaustive()", name);
});
outln!(out, "}}");
});
outln!(out, "}}");
}

/// Caches to avoid repeating some operations.
#[derive(Default)]
pub(crate) struct Caches {
Expand Down Expand Up @@ -248,9 +264,6 @@ impl Derives {

pub(super) fn to_list(self) -> Vec<&'static str> {
let mut list = Vec::new();
if self.debug {
list.push("Debug");
}
if self.clone {
list.push("Clone");
}
Expand All @@ -260,6 +273,14 @@ impl Derives {
if self.default_ {
list.push("Default");
}
list
}

pub(super) fn extra_traits_list(self) -> Vec<&'static str> {
let mut list = Vec::new();
if self.debug {
list.push("Debug");
}
if self.partial_eq {
list.push("PartialEq");
}
Expand Down
36 changes: 36 additions & 0 deletions generator/src/generator/namespace/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,18 @@ fn emit_request_struct(

let mut derives = Derives::all();
generator.filter_derives_for_fields(&mut derives, &request_def.fields.borrow(), true);
let extras = derives.extra_traits_list();
let derives = derives.to_list();
if !derives.is_empty() {
outln!(out, "#[derive({})]", derives.join(", "));
}
if !extras.is_empty() {
outln!(
out,
"#[cfg_attr(feature = \"extra-traits\", derive({}))]",
extras.join(", ")
);
}
if !gathered.has_fds() {
outln!(
out,
Expand Down Expand Up @@ -467,6 +475,30 @@ fn emit_request_struct(
outln!(out, "}}",);
}

// Implement `Debug` manually if `extra-traits` is not enabled.
outln!(out, "#[cfg(not(feature = \"extra-traits\"))]");
outln!(
out,
"impl{lifetime} core::fmt::Debug for {name}Request{lifetime} {{",
lifetime = struct_lifetime_block,
name = name
);
out.indented(|out| {
outln!(
out,
"fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {{"
);
out.indented(|out| {
outln!(
out,
"f.debug_struct(\"{name}Request\").finish_non_exhaustive()",
name = name
);
});
outln!(out, "}}");
});
outln!(out, "}}");

// Methods implemented on every request
outln!(
out,
Expand Down Expand Up @@ -917,6 +949,10 @@ fn emit_request_struct(
"/// Parse this request given its header, its body, and any fds that go along \
with it"
);
outln!(
out,
"#[cfg(feature = \"extra-traits\")]"
);
if gathered.has_fds() {
outln!(
out,
Expand Down
10 changes: 10 additions & 0 deletions generator/src/generator/namespace/struct_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,18 @@ pub(super) fn emit_struct_type(
if let Some(doc) = doc {
generator.emit_doc(doc, out, Some(&deducible_fields));
}
let extras = derives.extra_traits_list();
let derives = derives.to_list();
if !derives.is_empty() {
outln!(out, "#[derive({})]", derives.join(", "));
}
if !extras.is_empty() {
outln!(
out,
"#[cfg_attr(feature = \"extra-traits\", derive({}))]",
extras.join(", ")
);
}
if !has_fds {
outln!(
out,
Expand All @@ -75,6 +83,8 @@ pub(super) fn emit_struct_type(
}
outln!(out, "}}");

super::helpers::default_debug_impl(name, out);

if generate_try_parse {
let input_name = if !matches!(parse_size_constraint, StructSizeConstraint::None) {
"initial_value"
Expand Down
10 changes: 10 additions & 0 deletions generator/src/generator/namespace/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,21 @@ pub(super) fn emit_switch_type(
for case in switch.cases.iter() {
generator.filter_derives_for_fields(&mut derives, &case.fields.borrow(), false);
}
let extras = derives.extra_traits_list();
let mut derives = derives.to_list();
if switch.kind == xcbdefs::SwitchKind::BitCase {
derives.push("Default");
}
if !derives.is_empty() {
outln!(out, "#[derive({})]", derives.join(", "));
}
if !extras.is_empty() {
outln!(
out,
"#[cfg_attr(feature = \"extra-traits\", derive({}))]",
extras.join(", ")
);
}
outln!(
out,
r#"#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]"#
Expand Down Expand Up @@ -215,6 +223,8 @@ pub(super) fn emit_switch_type(
outln!(out, "}}");
}

super::helpers::default_debug_impl(name, out);

if generate_try_parse {
emit_switch_try_parse(generator, switch, name, &case_infos, switch_expr_type, out);
}
Expand Down
1 change: 1 addition & 0 deletions generator/src/generator/requests_replies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ pub(super) fn generate(out: &mut Output, module: &xcbdefs::Module, mut enum_case
out,
"#[allow(clippy::cognitive_complexity, clippy::single_match)]"
);
outln!(out, "#[cfg(feature = \"extra-traits\")]");
outln!(out, "pub fn parse(");
out.indented(|out| {
outln!(out, "header: RequestHeader,");
Expand Down
4 changes: 3 additions & 1 deletion x11rb-async/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ event-listener = "2.5.3"
futures-lite = "1"
tracing = { version = "0.1", default-features = false }
x11rb = { version = "0.12.0", path = "../x11rb", default-features = false }
x11rb-protocol = { version = "0.12.0", path = "../x11rb-protocol" }
x11rb-protocol = { version = "0.12.0", default-features = false, features = ["std"], path = "../x11rb-protocol" }

[features]
# Enable this feature to enable all the X11 extensions
Expand Down Expand Up @@ -58,6 +58,8 @@ all-extensions = [
"xvmc"
]

extra-traits = ["x11rb-protocol/extra-traits"]
notgull marked this conversation as resolved.
Show resolved Hide resolved

composite = ["x11rb-protocol/composite", "xfixes"]
damage = ["x11rb-protocol/damage", "xfixes"]
dbe = ["x11rb-protocol/dbe"]
Expand Down
5 changes: 4 additions & 1 deletion x11rb-protocol/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ serde = { version = "1", features = ["derive"], optional = true }
criterion = "0.5"

[features]
default = ["std"]
default = ["extra-traits", "std"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm.... do we really want this to be enabled by default?

Well... I guess it does not matter for now. 🤷

std = []

# Enable extra traits for the X11 types
extra-traits = []

# Enable utility functions in `x11rb::resource_manager` for querying the
# resource databases.
resource_manager = ["std"]
Expand Down
19 changes: 17 additions & 2 deletions x11rb-protocol/src/protocol/bigreq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,16 @@
/// 262140 bytes in length. When enabled, if the 16-bit length field is zero, it
/// is immediately followed by a 32-bit length field specifying the length of the
/// request in 4-byte units.
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Clone, Copy, Default)]
#[cfg_attr(feature = "extra-traits", derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash))]

Check warning on line 46 in x11rb-protocol/src/protocol/bigreq.rs

View check run for this annotation

Codecov / codecov/patch

x11rb-protocol/src/protocol/bigreq.rs#L45-L46

Added lines #L45 - L46 were not covered by tests
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct EnableRequest;
#[cfg(not(feature = "extra-traits"))]
impl core::fmt::Debug for EnableRequest {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("EnableRequest").finish_non_exhaustive()
}

Check warning on line 53 in x11rb-protocol/src/protocol/bigreq.rs

View check run for this annotation

Codecov / codecov/patch

x11rb-protocol/src/protocol/bigreq.rs#L51-L53

Added lines #L51 - L53 were not covered by tests
}
impl EnableRequest {
/// Serialize this request into bytes for the provided connection
pub fn serialize(self, major_opcode: u8) -> BufWithFds<[Cow<'static, [u8]>; 1]> {
Expand All @@ -62,6 +69,7 @@
([request0.into()], vec![])
}
/// Parse this request given its header, its body, and any fds that go along with it
#[cfg(feature = "extra-traits")]
pub fn try_parse_request(header: RequestHeader, value: &[u8]) -> Result<Self, ParseError> {
notgull marked this conversation as resolved.
Show resolved Hide resolved
if header.minor_opcode != ENABLE_REQUEST {
return Err(ParseError::InvalidValue);
Expand All @@ -88,13 +96,20 @@
/// # Fields
///
/// * `maximum_request_length` - The maximum length of requests supported by the server, in 4-byte units.
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Clone, Copy, Default)]
#[cfg_attr(feature = "extra-traits", derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash))]

Check warning on line 100 in x11rb-protocol/src/protocol/bigreq.rs

View check run for this annotation

Codecov / codecov/patch

x11rb-protocol/src/protocol/bigreq.rs#L99-L100

Added lines #L99 - L100 were not covered by tests
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct EnableReply {
pub sequence: u16,
pub length: u32,
pub maximum_request_length: u32,
}
#[cfg(not(feature = "extra-traits"))]
impl core::fmt::Debug for EnableReply {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("EnableReply").finish_non_exhaustive()
}

Check warning on line 111 in x11rb-protocol/src/protocol/bigreq.rs

View check run for this annotation

Codecov / codecov/patch

x11rb-protocol/src/protocol/bigreq.rs#L109-L111

Added lines #L109 - L111 were not covered by tests
}
impl TryParse for EnableReply {
fn try_parse(initial_value: &[u8]) -> Result<(Self, &[u8]), ParseError> {
let remaining = initial_value;
Expand Down
Loading
Loading