From 520343e37d890e0a4b0c6e1427e8164c43ce1c7d Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 10 Nov 2024 12:57:26 -0800 Subject: [PATCH 1/3] Add test of Debug and Display of paths DisplayDebug currently works but DebugDisplay does not. error[E0277]: `PathBuf` doesn't implement `std::fmt::Display` --> tests/test_path.rs:36:13 | 32 | #[derive(Error, Debug)] | ----- in this derive macro expansion ... 36 | #[error("debug:{0:?} display:{0}")] | ^^^^^^^^^^^^^^^^^^^^^^^^^ `PathBuf` cannot be formatted with the default formatter; call `.display()` on it | = help: the trait `std::fmt::Display` is not implemented for `PathBuf` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: call `.display()` or `.to_string_lossy()` to safely print paths, as they may contain non-Unicode data = help: the trait `std::fmt::Display` is implemented for `Var<'_, T>` = note: this error originates in the macro `$crate::format_args` which comes from the expansion of the derive macro `Error` (in Nightly builds, run with -Z macro-backtrace for more info) --- tests/test_path.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_path.rs b/tests/test_path.rs index 16881ae..5bb6972 100644 --- a/tests/test_path.rs +++ b/tests/test_path.rs @@ -29,6 +29,14 @@ pub struct UnsizedError { pub tail: str, } +#[derive(Error, Debug)] +pub enum BothError { + #[error("display:{0} debug:{0:?}")] + DisplayDebug(PathBuf), + #[error("debug:{0:?} display:{0}")] + DebugDisplay(PathBuf), +} + fn assert(expected: &str, value: T) { assert_eq!(expected, value.to_string()); } From dc0359eeecf778da2038805431c61010e7aa957e Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 10 Nov 2024 12:51:48 -0800 Subject: [PATCH 2/3] Defer binding_value construction --- impl/src/fmt.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/impl/src/fmt.rs b/impl/src/fmt.rs index e99c695..cac0ee3 100644 --- a/impl/src/fmt.rs +++ b/impl/src/fmt.rs @@ -85,15 +85,11 @@ impl Display<'_> { } _ => continue, }; - let binding_value = match &member { - MemberUnraw::Unnamed(index) => format_ident!("_{}", index), - MemberUnraw::Named(ident) => ident.to_local(), - }; - let mut wrapped_binding_value = quote!(::thiserror::__private::Var(#binding_value)); let end_spec = match read.find('}') { Some(end_spec) => end_spec, None => return Ok(()), }; + let mut bonus_display = false; let bound = match read[..end_spec].chars().next_back() { Some('?') => Trait::Debug, Some('o') => Trait::Octal, @@ -105,10 +101,7 @@ impl Display<'_> { Some('E') => Trait::UpperExp, Some(_) => Trait::Display, None => { - has_bonus_display = true; - wrapped_binding_value = quote_spanned! {span=> - #binding_value.as_display() - }; + bonus_display = true; Trait::Display } }; @@ -127,11 +120,21 @@ impl Display<'_> { formatvar = IdentUnraw::new(format_ident!("_{}", formatvar.to_string())); } out += &formatvar.to_string(); - if macro_named_args.insert(member) { - bindings.push((formatvar.to_local(), wrapped_binding_value)); - } else { + if !macro_named_args.insert(member.clone()) { // Already added to bindings by a previous use. + continue; } + let binding_value = match &member { + MemberUnraw::Unnamed(index) => format_ident!("_{}", index), + MemberUnraw::Named(ident) => ident.to_local(), + }; + let wrapped_binding_value = if bonus_display { + quote_spanned!(span=> #binding_value.as_display()) + } else { + quote!(::thiserror::__private::Var(#binding_value)) + }; + has_bonus_display |= bonus_display; + bindings.push((formatvar.to_local(), wrapped_binding_value)); } out += read; From 63882935be42fbd89e7076392a4d5330e2120332 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 10 Nov 2024 12:55:08 -0800 Subject: [PATCH 3/3] Support Display and Debug of same path in error message --- impl/src/fmt.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/impl/src/fmt.rs b/impl/src/fmt.rs index cac0ee3..82fc68e 100644 --- a/impl/src/fmt.rs +++ b/impl/src/fmt.rs @@ -4,7 +4,7 @@ use crate::scan_expr::scan_expr; use crate::unraw::{IdentUnraw, MemberUnraw}; use proc_macro2::{Delimiter, TokenStream, TokenTree}; use quote::{format_ident, quote, quote_spanned}; -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeSet, HashMap}; use std::iter; use syn::ext::IdentExt; use syn::parse::discouraged::Speculative; @@ -34,7 +34,7 @@ impl Display<'_> { let mut infinite_recursive = false; let mut implied_bounds = BTreeSet::new(); let mut bindings = Vec::new(); - let mut macro_named_args = HashSet::new(); + let mut macro_named_args = BTreeSet::new(); self.requires_fmt_machinery = self.requires_fmt_machinery || fmt.contains('}'); @@ -112,15 +112,22 @@ impl Display<'_> { out += &member.to_string(); continue; } + let formatvar_prefix = if bonus_display { + "__display" + } else { + "__field" + }; let mut formatvar = IdentUnraw::new(match &member { - MemberUnraw::Unnamed(index) => format_ident!("__field{}", index), - MemberUnraw::Named(ident) => format_ident!("__field_{}", ident.to_string()), + MemberUnraw::Unnamed(index) => format_ident!("{}{}", formatvar_prefix, index), + MemberUnraw::Named(ident) => { + format_ident!("{}_{}", formatvar_prefix, ident.to_string()) + } }); while user_named_args.contains(&formatvar) { formatvar = IdentUnraw::new(format_ident!("_{}", formatvar.to_string())); } out += &formatvar.to_string(); - if !macro_named_args.insert(member.clone()) { + if !macro_named_args.insert(formatvar.clone()) { // Already added to bindings by a previous use. continue; }