Merge pull request #884 from psychon/notgull/less-codegen

Gate some commonly unused traits behind a feature

.github/workflows/CI.yml

@@ -9,7 +9,7 @@ on:
 
 env:
   CARGO_TERM_COLOR: always
-  MOST_FEATURES: all-extensions cursor image tracing tracing-subscriber/env-filter
+  MOST_FEATURES: all-extensions cursor extra-traits image request-parsing tracing tracing-subscriber/env-filter
   # According to code coverage changes, sometimes $XENVIRONMENT is set and
   # sometimes not. Try to make this consistent to stabilise coverage reports.
   # Example: https://app.codecov.io/gh/psychon/x11rb/compare/726/changes
@@ -60,6 +60,12 @@ jobs:
        - name: clippy x11rb without features
          run: cargo clippy -p x11rb --all-targets -- -D warnings ${{ matrix.clippy_args }}
 
+       - name: clippy x11rb without default features
+         run: cargo clippy -p x11rb --no-default-features --all-targets -- -D warnings ${{ matrix.clippy_args }}
+
+       - name: clippy x11rb-protocol with request parsing
+         run: cargo clippy -p x11rb-protocol --all-targets --features request-parsing -- -D warnings ${{ matrix.clippy_args }}
+
        - name: clippy x11rb with allow-unsafe-code but without dl-libxcb
          run: cargo clippy -p x11rb --all-targets --features "allow-unsafe-code" -- -D warnings ${{ matrix.clippy_args }}
 

generator/src/generator/namespace/helpers.rs

@@ -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 {
@@ -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");
         }
@@ -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");
         }

generator/src/generator/namespace/request.rs

@@ -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,
@@ -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,
@@ -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 = \"request-parsing\")]"
+        );
         if gathered.has_fds() {
             outln!(
                 out,

generator/src/generator/namespace/struct_type.rs

@@ -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,
@@ -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"

generator/src/generator/namespace/switch.rs

@@ -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))]"#
@@ -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);
     }
@@ -386,6 +396,10 @@ fn emit_switch_try_parse(
                 )
             })
             .collect::<Vec<_>>();
+        outln!(
+            out.indent(),
+            "#[cfg_attr(not(feature = \"request-parsing\"), allow(dead_code))]"
+        );
         outln!(
             out.indent(),
             "fn try_parse(value: &[u8], {}) -> Result<(Self, &[u8]), ParseError> {{",

generator/src/generator/requests_replies.rs

@@ -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 = \"request-parsing\")]");
         outln!(out, "pub fn parse(");
         out.indented(|out| {
             outln!(out, "header: RequestHeader,");

x11rb-async/Cargo.toml

@@ -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
@@ -58,6 +58,9 @@ all-extensions = [
     "xvmc"
 ]
 
+# Enable extra traits on protocol types.
+extra-traits = ["x11rb-protocol/extra-traits"]
+
 composite = ["x11rb-protocol/composite", "xfixes"]
 damage = ["x11rb-protocol/damage", "xfixes"]
 dbe = ["x11rb-protocol/dbe"]

x11rb-protocol/Cargo.toml

@@ -21,9 +21,17 @@ serde = { version = "1", features = ["derive"], optional = true }
 criterion = "0.5"
 
 [features]
-default = ["std"]
+default = ["extra-traits", "std"]
 std = []
 
+# Enable extra traits for the X11 types
+extra-traits = []
+
+# Enable parsing for requests.
+# 
+# This adds a lot of extra code that isn't used in the common case.
+request-parsing = []
+
 # Enable utility functions in `x11rb::resource_manager` for querying the
 # resource databases.
 resource_manager = ["std"]

x11rb-protocol/src/protocol/bigreq.rs

@@ -42,9 +42,16 @@ pub const ENABLE_REQUEST: u8 = 0;
 /// 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))]
 #[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()
+    }
+}
 impl EnableRequest {
     /// Serialize this request into bytes for the provided connection
     pub fn serialize(self, major_opcode: u8) -> BufWithFds<[Cow<'static, [u8]>; 1]> {
@@ -62,6 +69,7 @@ impl EnableRequest {
         ([request0.into()], vec![])
     }
     /// Parse this request given its header, its body, and any fds that go along with it
+    #[cfg(feature = "request-parsing")]
     pub fn try_parse_request(header: RequestHeader, value: &[u8]) -> Result<Self, ParseError> {
         if header.minor_opcode != ENABLE_REQUEST {
             return Err(ParseError::InvalidValue);
@@ -88,13 +96,20 @@ impl crate::x11_utils::ReplyRequest for EnableRequest {
 /// # 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))]
 #[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()
+    }
+}
 impl TryParse for EnableReply {
     fn try_parse(initial_value: &[u8]) -> Result<(Self, &[u8]), ParseError> {
         let remaining = initial_value;