Fix incorrect fmt::Pointer implementations attempt two

CHANGELOG.md

@@ -104,6 +104,10 @@ 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.
+ ([#381](https://github.com/JelteF/derive_more/pull/381))
 
 ## 0.99.10 - 2020-09-11
 

impl/doc/debug.md

@@ -14,8 +14,28 @@ 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
+use derive_more::Debug;
+
+#[derive(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
@@ -90,8 +110,8 @@ If the top-level `#[debug("...", args...)]` attribute (the one for a whole struc
 and can be trivially substituted with a transparent delegation call to the inner type, then all the additional
 [formatting parameters][1] do work as expected:
 ```rust
-# use derive_more::Debug;
-#
+use derive_more::Debug;
+
 #[derive(Debug)]
 #[debug("{_0:o}")] // the same as calling `Octal::fmt()`
 struct MyOctalInt(i32);
@@ -110,8 +130,8 @@ assert_eq!(format!("{:07?}", MyBinaryInt(2)), "10");
 If, for some reason, transparency in trivial cases is not desired, it may be suppressed explicitly
 either with the [`format_args!()`] macro usage:
 ```rust
-# use derive_more::Debug;
-#
+use derive_more::Debug;
+
 #[derive(Debug)]
 #[debug("{}", format_args!("{_0:o}"))] // `format_args!()` obscures the inner type
 struct MyOctalInt(i32);
@@ -121,8 +141,8 @@ assert_eq!(format!("{:07?}", MyOctalInt(9)), "11");
 ```
 Or by adding [formatting parameters][1] which cause no visual effects:
 ```rust
-# use derive_more::Debug;
-#
+use derive_more::Debug;
+
 #[derive(Debug)]
 #[debug("{_0:^o}")] // `^` is centering, but in absence of additional width has no effect
 struct MyOctalInt(i32);

impl/doc/display.md

@@ -22,9 +22,28 @@ inner variable. If there is no such variable, or there is more than 1, an error
 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 +74,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

@@ -13,7 +13,7 @@ use crate::utils::{
 
 use super::{
  trait_name_to_attribute_name, ContainerAttributes, ContainsGenericsExt as _,
- FmtAttribute,
+ FieldsExt as _, FmtAttribute,
 };
 
 /// Expands a [`fmt::Debug`] derive macro.
@@ -250,9 +250,22 @@ impl<'a> Expansion<'a> {
  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
+ .fields
+ .fmt_args_idents()
+ .into_iter()
+ .any(|field| expr == field)
+ {
+ 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 deref_args = fmt.additional_deref_args(self.fields);
+
+ quote! { derive_more::core::write!(__derive_more_f, #fmt, #(#deref_args),*) }
  });
  };
 
@@ -288,12 +301,16 @@ 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 deref_args = fmt_attr.additional_deref_args(self.fields);
+
+ Ok(quote! {
+ derive_more::__private::DebugTuple::field(
+ #out,
+ &derive_more::core::format_args!(#fmt_attr, #(#deref_args),*),
+ )
+ })
+ }
  None => {
  let ident = format_ident!("_{i}");
  Ok(quote! {
@@ -319,29 +336,38 @@ impl<'a> Expansion<'a> {
  )
  };
  let out = named.named.iter().try_fold(out, |out, field| {
- let field_ident = field.ident.as_ref().unwrap_or_else(|| {
- unreachable!("`syn::Fields::Named`");
- });
- let field_str = field_ident.to_string();
- match FieldAttribute::parse_attrs(&field.attrs, self.attr_name)?
- .map(Spanning::into_inner)
- {
- Some(FieldAttribute::Left(_skip)) => {
- exhaustive = false;
- Ok::<_, syn::Error>(out)
- }
- Some(FieldAttribute::Right(fmt_attr)) => Ok(quote! {
+ let field_ident = field.ident.as_ref().unwrap_or_else(|| {
+ unreachable!("`syn::Fields::Named`");
+ });
+ let field_str = field_ident.to_string();
+ match FieldAttribute::parse_attrs(&field.attrs, self.attr_name)?
+ .map(Spanning::into_inner)
+ {
+ Some(FieldAttribute::Left(_skip)) => {
+ exhaustive = false;
+ Ok::<_, syn::Error>(out)
+ }
+ Some(FieldAttribute::Right(fmt_attr)) => {
+ let deref_args =
+ fmt_attr.additional_deref_args(self.fields);
+
+ 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, #(#deref_args),*
+ ),
  )
- }),
- None => Ok(quote! {
- derive_more::core::fmt::DebugStruct::field(#out, #field_str, &#field_ident)
- }),
+ })
  }
- })?;
+ None => Ok(quote! {
+ derive_more::core::fmt::DebugStruct::field(
+ #out, #field_str, &#field_ident
+ )
+ }),
+ }
+ })?;
  Ok(if exhaustive {
  quote! { derive_more::core::fmt::DebugStruct::finish(#out) }
  } else {

impl/src/fmt/display.rs

@@ -11,7 +11,7 @@ use crate::utils::{attr::ParseMultiple as _, Spanning};
 
 use super::{
  trait_name_to_attribute_name, ContainerAttributes, ContainsGenericsExt as _,
- FmtAttribute,
+ FieldsExt as _, FmtAttribute,
 };
 
 /// Expands a [`fmt::Display`]-like derive macro.
@@ -278,13 +278,30 @@ impl<'a> Expansion<'a> {
  body = match &self.attrs.fmt {
  Some(fmt) => {
  if has_shared_attr {
- quote! { &derive_more::core::format_args!(#fmt) }
+ let deref_args = fmt.additional_deref_args(self.fields);
+
+ quote! { &derive_more::core::format_args!(#fmt, #(#deref_args),*) }
  } else if let Some((expr, trait_ident)) = fmt.transparent_call() {
+ let expr = if self
+ .fields
+ .fmt_args_idents()
+ .into_iter()
+ .any(|field| expr == field)
+ {
+ 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 deref_args = fmt.additional_deref_args(self.fields);
+
+ quote! {
+ derive_more::core::write!(__derive_more_f, #fmt, #(#deref_args),*)
+ }
  }
  }
  None if self.fields.is_empty() => {
@@ -332,8 +349,10 @@ impl<'a> Expansion<'a> {
 
  if has_shared_attr {
  if let Some(shared_fmt) = &self.shared_attr {
+ let deref_args = shared_fmt.additional_deref_args(self.fields);
+
  let shared_body = quote! {
- derive_more::core::write!(__derive_more_f, #shared_fmt)
+ derive_more::core::write!(__derive_more_f, #shared_fmt, #(#deref_args),*)
  };
 
  body = if body.is_empty() {

impl/src/fmt/mod.rs

@@ -9,7 +9,7 @@ pub(crate) mod display;
 mod parsing;
 
 use proc_macro2::TokenStream;
-use quote::{format_ident, ToTokens};
+use quote::{format_ident, quote, ToTokens};
 use syn::{
  parse::{Parse, ParseStream},
  punctuated::Punctuated,
@@ -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,33 @@ impl FmtAttribute {
  })
  }
 
+ /// Returns an [`Iterator`] over the additional formatting arguments doing the dereferencing
+ /// replacement in this [`FmtAttribute`] for those [`Placeholder`] representing the provided
+ /// [`syn::Fields`] and requiring it
+ fn additional_deref_args<'fmt: 'ret, 'fields: 'ret, 'ret>(
+ &'fmt self,
+ fields: &'fields syn::Fields,
+ ) -> impl Iterator<Item = TokenStream> + 'ret {
+ let used_args = Placeholder::parse_fmt_string(&self.lit.value())
+ .into_iter()
+ .filter_map(|placeholder| match placeholder.arg {
+ Parameter::Named(name) => Some(name),
+ _ => None,
+ })
+ .collect::<Vec<_>>();
+
+ fields
+ .fmt_args_idents()
+ .into_iter()
+ .filter_map(move |field_name| {
+ (used_args.iter().any(|arg| field_name == arg)
+ && !self.args.iter().any(|arg| {
+ arg.alias.as_ref().map_or(false, |(n, _)| n == &field_name)
+ }))
+ .then(|| quote! { #field_name = *#field_name })
+ })
+ }
+
  /// Errors in case legacy syntax is encountered: `fmt = "...", (arg),*`.
  fn check_legacy_fmt(input: ParseStream<'_>) -> syn::Result<()> {
  let fork = input.fork();
@@ -523,6 +552,7 @@ where
  }
 }
 
+/// Extension of a [`syn::Type`] and a [`syn::Path`] allowing to travers its type parameters.
 trait ContainsGenericsExt {
  /// Checks whether this definition contains any of the provided `type_params`.
  fn contains_generics(&self, type_params: &[&syn::Ident]) -> bool;
@@ -638,6 +668,26 @@ impl ContainsGenericsExt for syn::Path {
  }
 }
 
+/// Extension of [`syn::Fields`] providing helpers for a [`FmtAttribute`].
+trait FieldsExt {
+ /// Returns an [`Iterator`] over [`syn::Ident`]s representing these [`syn::Fields`] in a
+ /// [`FmtAttribute`] as [`FmtArgument`]s or named [`Placeholder`]s.
+ ///
+ /// [`syn::Ident`]: struct@syn::Ident
+ // TODO: Return `impl Iterator<Item = syn::Ident> + '_` once MSRV is bumped up to 1.75 or
+ // higher.
+ fn fmt_args_idents(&self) -> Vec<syn::Ident>;
+}
+
+impl FieldsExt for syn::Fields {
+ fn fmt_args_idents(&self) -> Vec<syn::Ident> {
+ self.iter()
+ .enumerate()
+ .map(|(i, f)| f.ident.clone().unwrap_or_else(|| format_ident!("_{i}")))
+ .collect()
+ }
+}
+
 #[cfg(test)]
 mod fmt_attribute_spec {
  use itertools::Itertools as _;

impl/src/parsing.rs

@@ -71,6 +71,12 @@ impl Parse for Expr {
  }
 }
 
+impl PartialEq<syn::Ident> for Expr {
+ fn eq(&self, other: &syn::Ident) -> bool {
+ self.ident().map_or(false, |i| i == other)
+ }
+}
+
 impl ToTokens for Expr {
  fn to_tokens(&self, tokens: &mut TokenStream) {
  match self {