From 84445d512014bea241af471a9bf7083019da13e2 Mon Sep 17 00:00:00 2001 From: Bernardo Uriarte Date: Sun, 4 Sep 2022 18:37:45 +0200 Subject: [PATCH] propagate `include_all_fields` option --- .../salsa-2022-macros/src/salsa_struct.rs | 91 +++++----- components/salsa-2022/src/debug.rs | 158 +++++++++++++----- components/salsa-2022/src/event.rs | 50 ++++-- components/salsa-2022/src/key.rs | 16 +- salsa-2022-tests/tests/debug.rs | 4 +- .../tests/warnings/needless_borrow.rs | 7 +- 6 files changed, 215 insertions(+), 111 deletions(-) diff --git a/components/salsa-2022-macros/src/salsa_struct.rs b/components/salsa-2022-macros/src/salsa_struct.rs index 482f6d83..bc4521bd 100644 --- a/components/salsa-2022-macros/src/salsa_struct.rs +++ b/components/salsa-2022-macros/src/salsa_struct.rs @@ -97,14 +97,11 @@ impl SalsaStruct { self.fields.iter() } - /// Iterator over fields that are part of structs identity. - /// - /// If this is an input, empty iterator. - pub(crate) fn identity_fields(&self) -> Vec<&SalsaField> { + pub(crate) fn is_identity_field(&self, field: &SalsaField) -> bool { match self.kind { - SalsaStructKind::Input => Vec::new(), - SalsaStructKind::Tracked => self.all_fields().filter(|f| f.has_id_attr).collect(), - SalsaStructKind::Interned => self.all_fields().collect(), + SalsaStructKind::Input => false, + SalsaStructKind::Tracked => field.has_id_attr, + SalsaStructKind::Interned => true, } } @@ -314,55 +311,55 @@ impl SalsaStruct { let ident_string = ident.to_string(); // `::salsa::debug::helper::SalsaDebug` will use `DebugWithDb` or fallbak to `Debug` - - let field_debug_impl = |field: &SalsaField| -> TokenStream { - let field_name_string = field.name().to_string(); - let field_getter = field.get_name(); - let field_ty = field.ty(); - - parse_quote_spanned! {field.field.span() => - .field( - #field_name_string, - &::salsa::debug::helper::SalsaDebug::<#field_ty, #db_type>::salsa_debug( - #[allow(clippy::needless_borrow)] - &self.#field_getter(_db), - _db - ) - ) - } - }; - - let identity_fields = self - .identity_fields() - .into_iter() - .map(field_debug_impl) - .collect::(); - - let all_fields = self + let fields = self .all_fields() .into_iter() - .map(field_debug_impl) + .map(|field| -> TokenStream { + let field_name_string = field.name().to_string(); + let field_getter = field.get_name(); + let field_ty = field.ty(); + + if self.is_identity_field(field){ + parse_quote_spanned! {field.field.span() => + debug_struct = debug_struct.field( + #field_name_string, + &::salsa::debug::helper::SalsaDebug::<#field_ty, #db_type>::salsa_debug( + #[allow(clippy::needless_borrow)] + &self.#field_getter(_db), + _db, + _include_all_fields + ) + ); + } + }else{ + parse_quote_spanned! {field.field.span() => + if _include_all_fields { + debug_struct = debug_struct.field( + #field_name_string, + &::salsa::debug::helper::SalsaDebug::<#field_ty, #db_type>::salsa_debug( + #[allow(clippy::needless_borrow)] + &self.#field_getter(_db), + _db, + true + ) + ); + } + } + } + + }) .collect::(); // `use ::salsa::debug::helper::Fallback` is needed for the fallback to `Debug` impl parse_quote_spanned! {ident.span()=> impl ::salsa::DebugWithDb<#db_type> for #ident { - fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>, _db: &#db_type) -> ::std::fmt::Result { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>, _db: &#db_type, _include_all_fields: bool) -> ::std::fmt::Result { #[allow(unused_imports)] use ::salsa::debug::helper::Fallback; - f.debug_struct(#ident_string) - .field("[salsa id]", &self.0.as_u32()) - #identity_fields - .finish() - } - - fn fmt_all(&self, f: &mut ::std::fmt::Formatter<'_>, _db: &#db_type) -> ::std::fmt::Result { - #[allow(unused_imports)] - use ::salsa::debug::helper::Fallback; - f.debug_struct(#ident_string) - .field("[salsa id]", &self.0.as_u32()) - #all_fields - .finish() + let mut debug_struct = &mut f.debug_struct(#ident_string); + debug_struct = debug_struct.field("[salsa id]", &self.0.as_u32()); + #fields + debug_struct.finish() } } } diff --git a/components/salsa-2022/src/debug.rs b/components/salsa-2022/src/debug.rs index c97d8d2e..02476a37 100644 --- a/components/salsa-2022/src/debug.rs +++ b/components/salsa-2022/src/debug.rs @@ -12,7 +12,18 @@ pub trait DebugWithDb { DebugWith { value: BoxRef::Ref(self), db, - all_fields: false, + include_all_fields: false, + } + } + + fn debug_with<'me, 'db>(&'me self, db: &'me Db, include_all_fields: bool) -> DebugWith<'me, Db> + where + Self: Sized + 'me, + { + DebugWith { + value: BoxRef::Ref(self), + db, + include_all_fields, } } @@ -26,7 +37,7 @@ pub trait DebugWithDb { DebugWith { value: BoxRef::Ref(self), db, - all_fields: true, + include_all_fields: true, } } @@ -37,7 +48,7 @@ pub trait DebugWithDb { DebugWith { value: BoxRef::Box(Box::new(self)), db, - all_fields: false, + include_all_fields: false, } } @@ -51,26 +62,22 @@ pub trait DebugWithDb { DebugWith { value: BoxRef::Box(Box::new(self)), db, - all_fields: true, + include_all_fields: true, } } - /// Should only read fields that are part of the identity, which means: - /// - for [#\[salsa::input\]](salsa_2022_macros::input) no fields - /// - for [#\[salsa::tracked\]](salsa_2022_macros::tracked) only fields with `#[id]` attribute - /// - for [#\[salsa::interned\]](salsa_2022_macros::interned) any field - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result; - - /// Unlike [DebugWithDb::fmt], may read any field. - fn fmt_all(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { - self.fmt(f, db) - } + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result; } pub struct DebugWith<'me, Db: ?Sized> { value: BoxRef<'me, dyn DebugWithDb + 'me>, db: &'me Db, - all_fields: bool, + include_all_fields: bool, } enum BoxRef<'me, T: ?Sized> { @@ -91,11 +98,7 @@ impl std::ops::Deref for BoxRef<'_, T> { impl std::fmt::Debug for DebugWith<'_, D> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if self.all_fields { - DebugWithDb::fmt_all(&*self.value, f, self.db) - } else { - DebugWithDb::fmt(&*self.value, f, self.db) - } + DebugWithDb::fmt(&*self.value, f, self.db, self.include_all_fields) } } @@ -103,8 +106,13 @@ impl DebugWithDb for &T where T: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { - T::fmt(self, f, db) + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { + T::fmt(self, f, db, include_all_fields) } } @@ -112,8 +120,13 @@ impl DebugWithDb for Box where T: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { - T::fmt(self, f, db) + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { + T::fmt(self, f, db, include_all_fields) } } @@ -121,8 +134,13 @@ impl DebugWithDb for Rc where T: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { - T::fmt(self, f, db) + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { + T::fmt(self, f, db, include_all_fields) } } @@ -130,8 +148,13 @@ impl DebugWithDb for Arc where T: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { - T::fmt(self, f, db) + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { + T::fmt(self, f, db, include_all_fields) } } @@ -139,8 +162,13 @@ impl DebugWithDb for Vec where T: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { - let elements = self.iter().map(|e| e.debug(db)); + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { + let elements = self.iter().map(|e| e.debug_with(db, include_all_fields)); f.debug_list().entries(elements).finish() } } @@ -149,8 +177,13 @@ impl DebugWithDb for Option where T: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { - let me = self.as_ref().map(|v| v.debug(db)); + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { + let me = self.as_ref().map(|v| v.debug_with(db, include_all_fields)); std::fmt::Debug::fmt(&me, f) } } @@ -160,8 +193,18 @@ where K: DebugWithDb, V: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { - let elements = self.iter().map(|(k, v)| (k.debug(db), v.debug(db))); + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { + let elements = self.iter().map(|(k, v)| { + ( + k.debug_with(db, include_all_fields), + v.debug_with(db, include_all_fields), + ) + }); f.debug_map().entries(elements).finish() } } @@ -171,10 +214,15 @@ where A: DebugWithDb, B: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { f.debug_tuple("") - .field(&self.0.debug(db)) - .field(&self.1.debug(db)) + .field(&self.0.debug_with(db, include_all_fields)) + .field(&self.1.debug_with(db, include_all_fields)) .finish() } } @@ -185,11 +233,16 @@ where B: DebugWithDb, C: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { f.debug_tuple("") - .field(&self.0.debug(db)) - .field(&self.1.debug(db)) - .field(&self.2.debug(db)) + .field(&self.0.debug_with(db, include_all_fields)) + .field(&self.1.debug_with(db, include_all_fields)) + .field(&self.2.debug_with(db, include_all_fields)) .finish() } } @@ -198,8 +251,13 @@ impl DebugWithDb for HashSet where V: DebugWithDb, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { - let elements = self.iter().map(|e| e.debug(db)); + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { + let elements = self.iter().map(|e| e.debug_with(db, include_all_fields)); f.debug_list().entries(elements).finish() } } @@ -213,7 +271,11 @@ pub mod helper { use std::{fmt, marker::PhantomData}; pub trait Fallback { - fn salsa_debug<'a, 'b>(a: &'a T, _db: &'b Db) -> &'a dyn fmt::Debug { + fn salsa_debug<'a, 'b>( + a: &'a T, + _db: &'b Db, + _include_all_fields: bool, + ) -> &'a dyn fmt::Debug { a } } @@ -222,8 +284,12 @@ pub mod helper { impl, Db: ?Sized> SalsaDebug { #[allow(dead_code)] - pub fn salsa_debug<'a, 'b: 'a>(a: &'a T, db: &'b Db) -> DebugWith<'a, Db> { - a.debug(db) + pub fn salsa_debug<'a, 'b: 'a>( + a: &'a T, + db: &'b Db, + include_all_fields: bool, + ) -> DebugWith<'a, Db> { + a.debug_with(db, include_all_fields) } } diff --git a/components/salsa-2022/src/event.rs b/components/salsa-2022/src/event.rs index fdbef715..ca2b9b68 100644 --- a/components/salsa-2022/src/event.rs +++ b/components/salsa-2022/src/event.rs @@ -28,10 +28,15 @@ impl DebugWithDb for Event where Db: ?Sized + Database, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { f.debug_struct("Event") .field("runtime_id", &self.runtime_id) - .field("kind", &self.kind.debug(db)) + .field("kind", &self.kind.debug_with(db, include_all_fields)) .finish() } } @@ -149,11 +154,19 @@ impl DebugWithDb for EventKind where Db: ?Sized + Database, { - fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { + fn fmt( + &self, + fmt: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { match self { EventKind::DidValidateMemoizedValue { database_key } => fmt .debug_struct("DidValidateMemoizedValue") - .field("database_key", &database_key.debug(db)) + .field( + "database_key", + &database_key.debug_with(db, include_all_fields), + ) .finish(), EventKind::WillBlockOn { other_runtime_id, @@ -161,11 +174,17 @@ where } => fmt .debug_struct("WillBlockOn") .field("other_runtime_id", other_runtime_id) - .field("database_key", &database_key.debug(db)) + .field( + "database_key", + &database_key.debug_with(db, include_all_fields), + ) .finish(), EventKind::WillExecute { database_key } => fmt .debug_struct("WillExecute") - .field("database_key", &database_key.debug(db)) + .field( + "database_key", + &database_key.debug_with(db, include_all_fields), + ) .finish(), EventKind::WillCheckCancellation => fmt.debug_struct("WillCheckCancellation").finish(), EventKind::WillDiscardStaleOutput { @@ -173,20 +192,29 @@ where output_key, } => fmt .debug_struct("WillDiscardStaleOutput") - .field("execute_key", &execute_key.debug(db)) - .field("output_key", &output_key.debug(db)) + .field( + "execute_key", + &execute_key.debug_with(db, include_all_fields), + ) + .field("output_key", &output_key.debug_with(db, include_all_fields)) .finish(), EventKind::DidDiscard { key } => fmt .debug_struct("DidDiscard") - .field("key", &key.debug(db)) + .field("key", &key.debug_with(db, include_all_fields)) .finish(), EventKind::DidDiscardAccumulated { executor_key, accumulator, } => fmt .debug_struct("DidDiscardAccumulated") - .field("executor_key", &executor_key.debug(db)) - .field("accumulator", &accumulator.debug(db)) + .field( + "executor_key", + &executor_key.debug_with(db, include_all_fields), + ) + .field( + "accumulator", + &accumulator.debug_with(db, include_all_fields), + ) .finish(), } } diff --git a/components/salsa-2022/src/key.rs b/components/salsa-2022/src/key.rs index c443b7bb..8b71811d 100644 --- a/components/salsa-2022/src/key.rs +++ b/components/salsa-2022/src/key.rs @@ -38,7 +38,12 @@ impl crate::debug::DebugWithDb for DependencyIndex where Db: ?Sized + Database, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + _include_all_fields: bool, + ) -> std::fmt::Result { db.fmt_index(*self, f) } } @@ -68,9 +73,14 @@ impl crate::debug::DebugWithDb for DatabaseKeyIndex where Db: ?Sized + Database, { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { + fn fmt( + &self, + f: &mut std::fmt::Formatter<'_>, + db: &Db, + include_all_fields: bool, + ) -> std::fmt::Result { let i: DependencyIndex = (*self).into(); - DebugWithDb::fmt(&i, f, db) + DebugWithDb::fmt(&i, f, db, include_all_fields) } } diff --git a/salsa-2022-tests/tests/debug.rs b/salsa-2022-tests/tests/debug.rs index 62311cd5..03487846 100644 --- a/salsa-2022-tests/tests/debug.rs +++ b/salsa-2022-tests/tests/debug.rs @@ -51,8 +51,6 @@ fn input() { // all fields let actual = format!("{:?}", complex_struct.debug_all(&db)); - let expected = expect![[ - r#"ComplexStruct { [salsa id]: 0, my_input: MyInput { [salsa id]: 0 }, not_salsa: NotSalsa { field: "it's salsa time" } }"# - ]]; + let expected = expect![[r#"ComplexStruct { [salsa id]: 0, my_input: MyInput { [salsa id]: 0, field: 22 }, not_salsa: NotSalsa { field: "it's salsa time" } }"#]]; expected.assert_eq(&actual); } diff --git a/salsa-2022-tests/tests/warnings/needless_borrow.rs b/salsa-2022-tests/tests/warnings/needless_borrow.rs index 502d23e2..92cd0677 100644 --- a/salsa-2022-tests/tests/warnings/needless_borrow.rs +++ b/salsa-2022-tests/tests/warnings/needless_borrow.rs @@ -7,7 +7,12 @@ struct Jar(TokenTree); enum Token {} impl salsa::DebugWithDb for Token { - fn fmt(&self, _f: &mut std::fmt::Formatter<'_>, _db: &dyn Db) -> std::fmt::Result { + fn fmt( + &self, + _f: &mut std::fmt::Formatter<'_>, + _db: &dyn Db, + _include_all_fields: bool, + ) -> std::fmt::Result { unreachable!() } }