Fix incorrect fmt::Pointer implementations attempt two

CHANGELOG.md

@@ -104,6 +104,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Hygiene of macro expansions in presence of custom `core` crate.
   ([#327](https://github.com/JelteF/derive_more/pull/327))
 - Fix documentation of generated methods in `IsVariant` derive.
+- Make `{field:p}` do the expected thing in format strings for `Display` and
+  `Debug`. Also document weirdness around `Pointer` formatting when using
+  expressions, due to field variables being references.
 
 ## 0.99.10 - 2020-09-11
 

impl/doc/debug.md

@@ -14,8 +14,26 @@ This derive macro is a clever superset of `Debug` from standard library. Additio
 You supply a format by placing an attribute on a struct or enum variant, or its particular field:
 `#[debug("...", args...)]`. The format is exactly like in [`format!()`] or any other [`format_args!()`]-based macros.
 
-The variables available in the arguments is `self` and each member of the struct or enum variant, with members of tuple
-structs being named with a leading underscore and their index, i.e. `_0`, `_1`, `_2`, etc.
+The variables available in the arguments is `self` and each member of the
+struct or enum variant, with members of tuple structs being named with a
+leading underscore and their index, i.e. `_0`, `_1`, `_2`, etc. Due to
+ownership/lifetime limitations the member variables are all references to the
+fields, except when used directly in the format string. For most purposes this
+detail doesn't matter, but it is quite important when using `Pointer`
+formatting. If you don't use the `{field:p}` syntax, you have to dereference
+once to get the address of the field itself, instead of the address of the
+reference to the field:
+
+```rust
+#[derive(derive_more::Debug)]
+#[debug("{field:p} {:p}", *field)]
+struct RefInt<'a> {
+    field: &'a i32,
+}
+
+let a = &123;
+assert_eq!(format!("{:?}", RefInt{field: &a}), format!("{a:p} {:p}", a));
+```
 
 
 ### Generic data types

impl/doc/display.md

@@ -14,17 +14,33 @@ For enums, you can either specify it on each variant, or on the enum as a whole.
 For variants that don't have a format specified, it will simply defer to the format of the
 inner variable. If there is no such variable, or there is more than 1, an error is generated.
 
-
-
-
 ## The format of the format
 
 You supply a format by attaching an attribute of the syntax: `#[display("...", args...)]`.
 The format supplied is passed verbatim to `write!`.
 
-The variables available in the arguments is `self` and each member of the variant,
-with members of tuple structs being named with a leading underscore and their index,
-i.e. `_0`, `_1`, `_2`, etc.
+The variables available in the arguments is `self` and each member of the
+struct or enum variant, with members of tuple structs being named with a
+leading underscore and their index, i.e. `_0`, `_1`, `_2`, etc. Due to
+ownership/lifetime limitations the member variables are all references to the
+fields, except when used directly in the format string. For most purposes this
+detail doesn't matter, but it is quite important when using `Pointer`
+formatting. If you don't use the `{field:p}` syntax, you have to dereference
+once to get the address of the field itself, instead of the address of the
+reference to the field:
+
+```rust
+# use derive_more::Display;
+#
+#[derive(Display)]
+#[display("{field:p} {:p}", *field)]
+struct RefInt<'a> {
+    field: &'a i32,
+}
+
+let a = &123;
+assert_eq!(format!("{}", RefInt{field: &a}), format!("{a:p} {:p}", a));
+```
 
 For enums you can also specify a shared format on the enum itself instead of
 the variant. This format is used for each of the variants, and can be
@@ -55,7 +71,7 @@ E.g., for a structure `Foo` defined like this:
 # trait Trait { type Type; }
 #
 #[derive(Display)]
-#[display("{} {} {:?} {:p}", a, b, c, d)]
+#[display("{a} {b} {c:?} {d:p}")]
 struct Foo<'a, T1, T2: Trait, T3> {
     a: T1,
     b: <T2 as Trait>::Type,

impl/src/fmt/debug.rs

@@ -244,15 +244,49 @@ impl<'a> Expansion<'a> {
         Ok(())
     }
 
+    fn field_idents(&self) -> impl Iterator<Item = Ident> + '_ {
+        self.fields
+            .iter()
+            .enumerate()
+            .map(|(i, f)| f.ident.clone().unwrap_or_else(|| format_ident!("_{i}")))
+    }
+
+    fn field_params<'selff, 's>(
+        &'selff self,
+        fmt: &'a FmtAttribute,
+    ) -> impl Iterator<Item = TokenStream> + 's
+    where
+        'a: 's,
+        'selff: 's,
+    {
+        let used_arguments: Vec<String> = fmt.placeholder_names().collect();
+        self.field_idents().filter_map(move |var| {
+            if used_arguments.contains(&var.to_string()) {
+                Some(quote! { #var = *#var })
+            } else {
+                None
+            }
+        })
+    }
+
     /// Generates [`Debug::fmt()`] implementation for a struct or an enum variant.
     ///
     /// [`Debug::fmt()`]: std::fmt::Debug::fmt()
     fn generate_body(&self) -> syn::Result<TokenStream> {
         if let Some(fmt) = &self.attr.fmt {
             return Ok(if let Some((expr, trait_ident)) = fmt.transparent_call() {
-                quote! { derive_more::core::fmt::#trait_ident::fmt(&(#expr), __derive_more_f) }
+                let expr = if self.field_idents().any(|var| {
+                    fmt.placeholder_names()
+                        .any(|arg| arg == var.to_string().as_str())
+                }) {
+                    quote! { #expr }
+                } else {
+                    quote! { &(#expr) }
+                };
+                quote! { derive_more::core::fmt::#trait_ident::fmt(#expr, __derive_more_f) }
             } else {
-                quote! { derive_more::core::write!(__derive_more_f, #fmt) }
+                let field_params = self.field_params(fmt);
+                quote! { derive_more::core::write!(__derive_more_f, #fmt, #(#field_params),*) }
             });
         };
 
@@ -288,12 +322,14 @@ impl<'a> Expansion<'a> {
                             exhaustive = false;
                             Ok::<_, syn::Error>(out)
                         }
-                        Some(FieldAttribute::Right(fmt_attr)) => Ok(quote! {
-                            derive_more::__private::DebugTuple::field(
-                                #out,
-                                &derive_more::core::format_args!(#fmt_attr),
+                        Some(FieldAttribute::Right(fmt_attr)) => {
+                            let field_params = self.field_params(&fmt_attr);
+                            Ok(quote! {
+                                derive_more::__private::DebugTuple::field(
+                                    #out,
+                                    &derive_more::core::format_args!(#fmt_attr, #(#field_params),*),
                             )
-                        }),
+                        })},
                         None => {
                             let ident = format_ident!("_{i}");
                             Ok(quote! {
@@ -330,13 +366,15 @@ impl<'a> Expansion<'a> {
                                 exhaustive = false;
                                 Ok::<_, syn::Error>(out)
                             }
-                            Some(FieldAttribute::Right(fmt_attr)) => Ok(quote! {
+                            Some(FieldAttribute::Right(fmt_attr)) => {
+                            let field_params = self.field_params(&fmt_attr);
+                            Ok(quote! {
                                 derive_more::core::fmt::DebugStruct::field(
                                     #out,
                                     #field_str,
-                                    &derive_more::core::format_args!(#fmt_attr),
+                                    &derive_more::core::format_args!(#fmt_attr, #(#field_params),*),
                                 )
-                            }),
+                            })},
                             None => Ok(quote! {
                                 derive_more::core::fmt::DebugStruct::field(#out, #field_str, &#field_ident)
                             }),

impl/src/fmt/display.rs

@@ -5,7 +5,7 @@ use std::fmt;
 
 use proc_macro2::TokenStream;
 use quote::{format_ident, quote};
-use syn::{ext::IdentExt as _, parse_quote, spanned::Spanned as _};
+use syn::{ext::IdentExt as _, parse_quote, spanned::Spanned as _, Ident};
 
 use crate::utils::{attr::ParseMultiple as _, Spanning};
 
@@ -254,6 +254,31 @@ struct Expansion<'a> {
 }
 
 impl<'a> Expansion<'a> {
+    fn field_idents(&self) -> impl Iterator<Item = Ident> + '_ {
+        self.fields
+            .iter()
+            .enumerate()
+            .map(|(i, f)| f.ident.clone().unwrap_or_else(|| format_ident!("_{i}")))
+    }
+
+    fn field_params<'selff, 's>(
+        &'selff self,
+        fmt: &'a FmtAttribute,
+    ) -> impl Iterator<Item = TokenStream> + 's
+    where
+        'a: 's,
+        'selff: 's,
+    {
+        let used_arguments: Vec<String> = fmt.placeholder_names().collect();
+        self.field_idents().filter_map(move |var| {
+            if used_arguments.contains(&var.to_string()) {
+                Some(quote! { #var = *#var })
+            } else {
+                None
+            }
+        })
+    }
+
     /// Generates [`Display::fmt()`] implementation for a struct or an enum variant.
     ///
     /// # Errors
@@ -278,13 +303,23 @@ impl<'a> Expansion<'a> {
             body = match &self.attrs.fmt {
                 Some(fmt) => {
                     if has_shared_attr {
-                        quote! { &derive_more::core::format_args!(#fmt) }
+                        let field_params = self.field_params(fmt);
+                        quote! { &derive_more::core::format_args!(#fmt, #(#field_params),*) }
                     } else if let Some((expr, trait_ident)) = fmt.transparent_call() {
+                        let expr = if self.field_idents().any(|var| {
+                            fmt.placeholder_names()
+                                .any(|arg| arg == var.to_string().as_str())
+                        }) {
+                            quote! { #expr }
+                        } else {
+                            quote! { &(#expr) }
+                        };
                         quote! {
-                            derive_more::core::fmt::#trait_ident::fmt(&(#expr), __derive_more_f)
+                            derive_more::core::fmt::#trait_ident::fmt(#expr, __derive_more_f)
                         }
                     } else {
-                        quote! { derive_more::core::write!(__derive_more_f, #fmt) }
+                        let field_params = self.field_params(fmt);
+                        quote! { derive_more::core::write!(__derive_more_f, #fmt, #(#field_params),*) }
                     }
                 }
                 None if self.fields.is_empty() => {
@@ -332,8 +367,9 @@ impl<'a> Expansion<'a> {
 
         if has_shared_attr {
             if let Some(shared_fmt) = &self.shared_attr {
+                let field_params = self.field_params(shared_fmt);
                 let shared_body = quote! {
-                    derive_more::core::write!(__derive_more_f, #shared_fmt)
+                    derive_more::core::write!(__derive_more_f, #shared_fmt, #(#field_params),*)
                 };
 
                 body = if body.is_empty() {

impl/src/fmt/mod.rs

@@ -113,14 +113,16 @@ impl Parse for FmtAttribute {
     fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
         Self::check_legacy_fmt(input)?;
 
-        Ok(Self {
+        let mut parsed = Self {
             lit: input.parse()?,
             comma: input
                 .peek(token::Comma)
                 .then(|| input.parse())
                 .transpose()?,
             args: input.parse_terminated(FmtArgument::parse, token::Comma)?,
-        })
+        };
+        parsed.args.pop_punct();
+        Ok(parsed)
     }
 }
 
@@ -273,6 +275,15 @@ impl FmtAttribute {
         })
     }
 
+    fn placeholder_names(&self) -> impl Iterator<Item = String> {
+        Placeholder::parse_fmt_string(&self.lit.value())
+            .into_iter()
+            .filter_map(|placeholder| match placeholder.arg {
+                Parameter::Named(name) => Some(name),
+                _ => None,
+            })
+    }
+
     /// Errors in case legacy syntax is encountered: `fmt = "...", (arg),*`.
     fn check_legacy_fmt(input: ParseStream<'_>) -> syn::Result<()> {
         let fork = input.fork();