Repository: knurling-rs/defmt
Pull request #178 (2020-09-22 13:35:52)

derive: fix handling of generic bounds
This fixes bounds on generic parameters getting duplicated into the wrong place of the generated impl, which closes https://github.com/knurling-rs/defmt/issues/176.

This also makes the custom derive emit bounds on *type parameters* instead of *field types*, since the latter can apparently run into very obscure and impossible to debug compiler bugs like https://github.com/rust-lang/rust/issues/77052. This matches the behavior of Rust's built-in derives.


Patch:
diff --git a/book/src/format.md b/book/src/format.md
index 7517e4c..945b823 100644
--- a/book/src/format.md
+++ b/book/src/format.md
@@ -22,55 +22,4 @@ enum Request {
 }
 ```
 
-NOTE: for generic structs and enums the `derive` macro adds `Format` bounds to the *types of the generic fields* rather than to all the generic (input) parameters of the struct / enum.
-Built-in `derive` attributes like `#[derive(Debug)]` use the latter approach.
-To our knowledge `derive(Format)` approach is more accurate in that it doesn't over-constrain the generic type parameters.
-The different between the two approaches is depicted below:
-
-``` rust
-# extern crate defmt;
-# use defmt::Format;
-
-#[derive(Format)]
-struct S<'a, T> {
-   x: Option<&'a T>,
-   y: u8,
-}
-```
-
-``` rust
-# extern crate defmt;
-# use defmt::Format;
-
-// `Format` produces this implementation
-impl<'a, T> Format for S<'a, T>
-where
-    Option<&'a T>: Format // <- main difference
-{
-    // ..
-    # fn format(&self, f: &mut defmt::Formatter) {}
-}
-
-#[derive(Debug)]
-struct S<'a, T> {
-   x: Option<&'a T>,
-   y: u8,
-}
-```
-
-``` rust
-# use std::fmt::Debug;
-# struct S<'a, T> {
-#    x: Option<&'a T>,
-#    y: u8,
-# }
-
-// `Debug` produces this implementation
-impl<'a, T> Debug for S<'a, T>
-where
-    T: Debug // <- main difference
-{
-    // ..
-    # fn fmt(&self, f: &mut core::fmt::Formatter) -> std::fmt::Result { Ok(()) }
-}
-```
+NOTE: Like built-in derives like `#[derive(Debug)]`, `#[derive(Format)]` will add `Format` bounds to the generic type parameters of the struct.
diff --git a/macros/src/lib.rs b/macros/src/lib.rs
index 1dd19e7..87e985c 100644
--- a/macros/src/lib.rs
+++ b/macros/src/lib.rs
@@ -4,13 +4,15 @@ use proc_macro::{Span, TokenStream};
 use defmt_parser::Fragment;
 use proc_macro2::{Ident as Ident2, Span as Span2, TokenStream as TokenStream2};
 use quote::{format_ident, quote};
+use syn::GenericParam;
+use syn::WhereClause;
 use syn::{
     parse::{self, Parse, ParseStream},
     parse_macro_input,
     punctuated::Punctuated,
     spanned::Spanned as _,
     Data, DeriveInput, Expr, Fields, FieldsNamed, FieldsUnnamed, ItemFn, ItemStruct, LitInt,
-    LitStr, ReturnType, Token, Type,
+    LitStr, ReturnType, Token, Type, WherePredicate,
 };
 
 #[proc_macro_attribute]
@@ -175,7 +177,7 @@ impl MLevel {
 // `#[derive(Format)]`
 #[proc_macro_derive(Format)]
 pub fn format(ts: TokenStream) -> TokenStream {
-    let input = parse_macro_input!(ts as DeriveInput);
+    let mut input = parse_macro_input!(ts as DeriveInput);
     let span = input.span();
 
     let ident = input.ident;
@@ -254,30 +256,22 @@ pub fn format(ts: TokenStream) -> TokenStream {
         }
     }
 
-    let params = input.generics.params;
-    let predicates = if params.is_empty() {
-        vec![]
-    } else {
-        // `Format` bounds for non-native field types
-        let mut preds = field_types
-            .into_iter()
-            .map(|ty| quote!(#ty: defmt::Format))
-            .collect::<Vec<_>>();
-        // extend with the where clause from the struct/enum declaration
-        if let Some(where_clause) = input.generics.where_clause {
-            preds.extend(
-                where_clause
-                    .predicates
-                    .into_iter()
-                    .map(|pred| quote!(#pred)),
-            )
+    let where_clause = input.generics.make_where_clause();
+    let mut where_clause: WhereClause = where_clause.clone();
+    let (impl_generics, type_generics, _) = input.generics.split_for_impl();
+
+    // Extend where-clause with `Format` bounds for type parameters.
+    for param in &input.generics.params {
+        if let GenericParam::Type(ty) = param {
+            let ident = &ty.ident;
+            where_clause
+                .predicates
+                .push(syn::parse::<WherePredicate>(quote!(#ident: defmt::Format).into()).unwrap());
         }
-        preds
-    };
+    }
+
     quote!(
-        impl<#params> defmt::Format for #ident<#params>
-        where #(#predicates),*
-        {
+        impl #impl_generics defmt::Format for #ident #type_generics #where_clause {
             fn format(&self, f: &mut defmt::Formatter) {
                 #(#exprs)*
             }


Test patch:
diff --git a/tests/encode.rs b/tests/encode.rs
index 76f0ad8..384b115 100644
--- a/tests/encode.rs
+++ b/tests/encode.rs
@@ -757,3 +757,42 @@ fn format_slice_enum_generic_struct() {
         ],
     );
 }
+
+#[test]
+fn derive_with_bounds() {
+    #[derive(Format)]
+    struct S<T: Copy> {
+        val: T,
+    }
+
+    #[derive(Format)]
+    struct S2<'a: 'b, 'b> {
+        a: &'a u8,
+        b: &'b u8,
+    }
+
+    let index = fetch_string_index();
+    check_format_implementation(
+        &S { val: 0 },
+        &[
+            index,         // "S {{ val: {:?} }}"
+            inc(index, 1), // "{:i32}"
+            0,
+            0,
+            0,
+            0,
+        ],
+    );
+
+    let index = fetch_string_index();
+    check_format_implementation(
+        &S2 { a: &1, b: &2 },
+        &[
+            index,         // "S2 { a: {:?}, b: {:?} }}"
+            inc(index, 1), // "{:u8}"
+            1,
+            inc(index, 2), // "{:u8}"
+            2,
+        ],
+    );
+}