diff --git a/components/salsa-2022-macros/src/input.rs b/components/salsa-2022-macros/src/input.rs index 5b83efe6..98975bc7 100644 --- a/components/salsa-2022-macros/src/input.rs +++ b/components/salsa-2022-macros/src/input.rs @@ -1,4 +1,4 @@ -use crate::salsa_struct::{SalsaField, SalsaStruct, SalsaStructKind}; +use crate::salsa_struct::{SalsaField, SalsaStruct}; use proc_macro2::{Literal, TokenStream}; /// For an entity struct `Foo` with fields `f1: T1, ..., fN: TN`, we generate... @@ -10,9 +10,7 @@ pub(crate) fn input( args: proc_macro::TokenStream, input: proc_macro::TokenStream, ) -> proc_macro::TokenStream { - match SalsaStruct::new(SalsaStructKind::Input, args, input) - .and_then(|el| InputStruct(el).generate_input()) - { + match SalsaStruct::new(args, input).and_then(|el| InputStruct(el).generate_input()) { Ok(s) => s.into(), Err(err) => err.into_compile_error().into(), } diff --git a/components/salsa-2022-macros/src/interned.rs b/components/salsa-2022-macros/src/interned.rs index a53ff5f1..59b5e7b7 100644 --- a/components/salsa-2022-macros/src/interned.rs +++ b/components/salsa-2022-macros/src/interned.rs @@ -1,4 +1,4 @@ -use crate::salsa_struct::{SalsaStruct, SalsaStructKind}; +use crate::salsa_struct::SalsaStruct; use proc_macro2::TokenStream; // #[salsa::interned(jar = Jar0, data = TyData0)] @@ -13,9 +13,7 @@ pub(crate) fn interned( args: proc_macro::TokenStream, input: proc_macro::TokenStream, ) -> proc_macro::TokenStream { - match SalsaStruct::new(SalsaStructKind::Interned, args, input) - .and_then(|el| InternedStruct(el).generate_interned()) - { + match SalsaStruct::new(args, input).and_then(|el| InternedStruct(el).generate_interned()) { Ok(s) => s.into(), Err(err) => err.into_compile_error().into(), } diff --git a/components/salsa-2022-macros/src/salsa_struct.rs b/components/salsa-2022-macros/src/salsa_struct.rs index 5b20b761..c6f7b331 100644 --- a/components/salsa-2022-macros/src/salsa_struct.rs +++ b/components/salsa-2022-macros/src/salsa_struct.rs @@ -29,50 +29,72 @@ use crate::options::{AllowedOptions, Options}; use proc_macro2::{Ident, Span, TokenStream}; use syn::spanned::Spanned; -pub(crate) enum SalsaStructKind { - Input, - Tracked, - Interned, -} - pub(crate) struct SalsaStruct { - kind: SalsaStructKind, args: Options, struct_item: syn::ItemStruct, + customizations: Vec, fields: Vec, } +#[derive(PartialEq, Eq, Debug, Copy, Clone)] +pub enum Customization { + DebugWithDb, +} + const BANNED_FIELD_NAMES: &[&str] = &["from", "new"]; impl SalsaStruct { pub(crate) fn new( - kind: SalsaStructKind, args: proc_macro::TokenStream, input: proc_macro::TokenStream, ) -> syn::Result { let struct_item = syn::parse(input)?; - Self::with_struct(kind, args, struct_item) + Self::with_struct(args, struct_item) } pub(crate) fn with_struct( - kind: SalsaStructKind, args: proc_macro::TokenStream, struct_item: syn::ItemStruct, ) -> syn::Result { let args: Options = syn::parse(args)?; - let fields = Self::extract_options(&struct_item)?; + let customizations = Self::extract_customizations(&struct_item)?; + let fields = Self::extract_fields(&struct_item)?; Ok(Self { - kind, args, struct_item, + customizations, fields, }) } + fn extract_customizations(struct_item: &syn::ItemStruct) -> syn::Result> { + Ok(struct_item + .attrs + .iter() + .map(|attr| { + if attr.path.is_ident("customize") { + // FIXME: this should be a comma separated list but I couldn't + // be bothered to remember how syn does this. + let args: syn::Ident = attr.parse_args()?; + if args.to_string() == "DebugWithDb" { + Ok(vec![Customization::DebugWithDb]) + } else { + Err(syn::Error::new_spanned(args, "unrecognized customization")) + } + } else { + Ok(vec![]) + } + }) + .collect::>, _>>()? + .into_iter() + .flatten() + .collect()) + } + /// Extract out the fields and their options: /// If this is a struct, it must use named fields, so we can define field accessors. /// If it is an enum, then this is not necessary. - pub(crate) fn extract_options(struct_item: &syn::ItemStruct) -> syn::Result> { + fn extract_fields(struct_item: &syn::ItemStruct) -> syn::Result> { match &struct_item.fields { syn::Fields::Named(n) => Ok(n .named @@ -93,13 +115,6 @@ impl SalsaStruct { self.fields.iter() } - pub(crate) fn is_identity_field(&self, field: &SalsaField) -> bool { - match self.kind { - SalsaStructKind::Input | SalsaStructKind::Tracked => field.has_id_attr, - SalsaStructKind::Interned => true, - } - } - /// Names of all fields (id and value). /// /// If this is an enum, empty vec. @@ -163,12 +178,14 @@ impl SalsaStruct { let ident = self.id_ident(); let visibility = &self.struct_item.vis; - // Extract the attributes the user gave, but screen out derive, since we are adding our own. + // Extract the attributes the user gave, but screen out derive, since we are adding our own, + // and the customize attribute that we use for our own purposes. let attrs: Vec<_> = self .struct_item .attrs .iter() .filter(|attr| !attr.path.is_ident("derive")) + .filter(|attr| !attr.path.is_ident("customize")) .collect(); parse_quote! { @@ -231,13 +248,17 @@ impl SalsaStruct { } /// Generate `impl salsa::DebugWithDb for Foo` - pub(crate) fn as_debug_with_db_impl(&self) -> syn::ItemImpl { + pub(crate) fn as_debug_with_db_impl(&self) -> Option { + if self.customizations.contains(&Customization::DebugWithDb) { + return None; + } + let ident = self.id_ident(); let db_type = self.db_dyn_ty(); let ident_string = ident.to_string(); - // `::salsa::debug::helper::SalsaDebug` will use `DebugWithDb` or fallbak to `Debug` + // `::salsa::debug::helper::SalsaDebug` will use `DebugWithDb` or fallback to `Debug` let fields = self .all_fields() .map(|field| -> TokenStream { @@ -245,36 +266,23 @@ impl SalsaStruct { let field_getter = field.get_name(); let field_ty = field.ty(); - let field_debug = quote_spanned! { field.field.span() => + 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 ) ); - }; - - if self.is_identity_field(field) { - quote_spanned! { field.field.span() => - #field_debug - } - } else { - quote_spanned! { field.field.span() => - if _include_all_fields { - #field_debug - } - } } }) .collect::(); // `use ::salsa::debug::helper::Fallback` is needed for the fallback to `Debug` impl - parse_quote_spanned! {ident.span()=> + Some(parse_quote_spanned! {ident.span()=> impl ::salsa::DebugWithDb<#db_type> for #ident { - fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>, _db: &#db_type, _include_all_fields: bool) -> ::std::fmt::Result { + fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>, _db: &#db_type) -> ::std::fmt::Result { #[allow(unused_imports)] use ::salsa::debug::helper::Fallback; let mut debug_struct = &mut f.debug_struct(#ident_string); @@ -283,7 +291,7 @@ impl SalsaStruct { debug_struct.finish() } } - } + }) } /// Disallow `#[id]` attributes on the fields of this struct. diff --git a/components/salsa-2022-macros/src/tracked_struct.rs b/components/salsa-2022-macros/src/tracked_struct.rs index feb36031..9dee55b6 100644 --- a/components/salsa-2022-macros/src/tracked_struct.rs +++ b/components/salsa-2022-macros/src/tracked_struct.rs @@ -1,6 +1,6 @@ use proc_macro2::{Literal, Span, TokenStream}; -use crate::salsa_struct::{SalsaField, SalsaStruct, SalsaStructKind}; +use crate::salsa_struct::{SalsaField, SalsaStruct}; /// For an tracked struct `Foo` with fields `f1: T1, ..., fN: TN`, we generate... /// @@ -11,8 +11,7 @@ pub(crate) fn tracked( args: proc_macro::TokenStream, struct_item: syn::ItemStruct, ) -> syn::Result { - SalsaStruct::with_struct(SalsaStructKind::Tracked, args, struct_item) - .and_then(|el| TrackedStruct(el).generate_tracked()) + SalsaStruct::with_struct(args, struct_item).and_then(|el| TrackedStruct(el).generate_tracked()) } struct TrackedStruct(SalsaStruct); diff --git a/components/salsa-2022/src/debug.rs b/components/salsa-2022/src/debug.rs index 5050a227..1c2a3a2c 100644 --- a/components/salsa-2022/src/debug.rs +++ b/components/salsa-2022/src/debug.rs @@ -5,7 +5,37 @@ use std::{ sync::Arc, }; -pub trait DebugWithDb { +use crate::database::AsSalsaDatabase; + +/// `DebugWithDb` is a version of the traditional [`Debug`](`std::fmt::Debug`) +/// trait that gives access to the salsa database, allowing tracked +/// structs to print the values of their fields. It is typically not used +/// directly, instead you should write (e.g.) `format!("{:?}", foo.debug(db))`. +/// Implementations are automatically provided for `#[salsa::tracked]` +/// items, though you can opt-out from that if you wish to provide a manual +/// implementation. +/// +/// # WARNING: Intended for debug use only! +/// +/// Debug print-outs of tracked structs include the value of all their fields, +/// but the reads of those fields are ignored by salsa. This avoids creating +/// spurious dependencies from debugging code, but if you use the resulting +/// string to influence the outputs (return value, accumulators, etc) from your +/// query, salsa's dependency tracking will be undermined. +/// +/// If for some reason you *want* to incorporate dependency output into +/// your query, do not use the `debug` or `into_debug` helpers and instead +/// invoke `fmt` manually. +pub trait DebugWithDb { + /// Creates a wrapper type that implements `Debug` but which + /// uses the `DebugWithDb::fmt`. + /// + /// # WARNING: Intended for debug use only! + /// + /// The wrapper type Debug impl will access the value of all + /// fields but those accesses are ignored by salsa. This is only + /// suitable for debug output. See [`DebugWithDb`][] trait comment + /// for more details. fn debug<'me, 'db>(&'me self, db: &'me Db) -> DebugWith<'me, Db> where Self: Sized + 'me, @@ -13,35 +43,18 @@ pub trait DebugWithDb { DebugWith { value: BoxRef::Ref(self), db, - 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, - } - } - - /// Be careful when using this method inside a tracked function, - /// because the default macro generated implementation will read all fields, - /// maybe introducing undesired dependencies. - fn debug_all<'me, 'db>(&'me self, db: &'me Db) -> DebugWith<'me, Db> - where - Self: Sized + 'me, - { - DebugWith { - value: BoxRef::Ref(self), - db, - include_all_fields: true, } } + /// Creates a wrapper type that implements `Debug` but which + /// uses the `DebugWithDb::fmt`. + /// + /// # WARNING: Intended for debug use only! + /// + /// The wrapper type Debug impl will access the value of all + /// fields but those accesses are ignored by salsa. This is only + /// suitable for debug output. See [`DebugWithDb`][] trait comment + /// for more details. fn into_debug<'me, 'db>(self, db: &'me Db) -> DebugWith<'me, Db> where Self: Sized + 'me, @@ -49,35 +62,33 @@ pub trait DebugWithDb { DebugWith { value: BoxRef::Box(Box::new(self)), db, - include_all_fields: false, } } - /// Be careful when using this method inside a tracked function, - /// because the default macro generated implementation will read all fields, - /// maybe introducing undesired dependencies. - fn into_debug_all<'me, 'db>(self, db: &'me Db) -> DebugWith<'me, Db> - where - Self: Sized + 'me, - { - DebugWith { - value: BoxRef::Box(Box::new(self)), - db, - include_all_fields: true, - } - } - - /// if `include_all_fields` is `false` only identity fields should be read, 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 fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result; + /// Format `self` given the database `db`. + /// + /// # Dependency tracking + /// + /// When invoked manually, field accesses that occur + /// within this method are tracked by salsa. But when invoked + /// the [`DebugWith`][] value returned by the [`debug`](`Self::debug`) + /// and [`into_debug`][`Self::into_debug`] methods, + /// those accesses are ignored. + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result; } -pub struct DebugWith<'me, Db: ?Sized> { +/// Helper type for the [`DebugWithDb`][] trait that +/// wraps a value and implements [`std::fmt::Debug`][], +/// redirecting calls to the `fmt` method from [`DebugWithDb`][]. +/// +/// # WARNING: Intended for debug use only! +/// +/// This type intentionally ignores salsa dependencies used +/// to generate the debug output. See the [`DebugWithDb`][] trait +/// for more notes on this. +pub struct DebugWith<'me, Db: ?Sized + AsSalsaDatabase> { value: BoxRef<'me, dyn DebugWithDb + 'me>, db: &'me Db, - include_all_fields: bool, } enum BoxRef<'me, T: ?Sized> { @@ -96,54 +107,64 @@ impl std::ops::Deref for BoxRef<'_, T> { } } -impl fmt::Debug for DebugWith<'_, D> { +impl fmt::Debug for DebugWith<'_, Db> +where + Db: AsSalsaDatabase, +{ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - DebugWithDb::fmt(&*self.value, f, self.db, self.include_all_fields) + let db = self.db.as_salsa_database(); + db.runtime() + .debug_probe(|| DebugWithDb::fmt(&*self.value, f, self.db)) } } impl DebugWithDb for &T where T: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { - T::fmt(self, f, db, include_all_fields) + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { + T::fmt(self, f, db) } } impl DebugWithDb for Box where T: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { - T::fmt(self, f, db, include_all_fields) + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { + T::fmt(self, f, db) } } impl DebugWithDb for Rc where T: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { - T::fmt(self, f, db, include_all_fields) + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { + T::fmt(self, f, db) } } impl DebugWithDb for Arc where T: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { - T::fmt(self, f, db, include_all_fields) + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { + T::fmt(self, f, db) } } impl DebugWithDb for Vec where T: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { - let elements = self.iter().map(|e| e.debug_with(db, include_all_fields)); + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { + let elements = self.iter().map(|e| e.debug(db)); f.debug_list().entries(elements).finish() } } @@ -151,9 +172,10 @@ where impl DebugWithDb for Option where T: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { - let me = self.as_ref().map(|v| v.debug_with(db, include_all_fields)); + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { + let me = self.as_ref().map(|v| v.debug(db)); fmt::Debug::fmt(&me, f) } } @@ -162,14 +184,10 @@ impl DebugWithDb for HashMap where K: DebugWithDb, V: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { - let elements = self.iter().map(|(k, v)| { - ( - k.debug_with(db, include_all_fields), - v.debug_with(db, include_all_fields), - ) - }); + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { + let elements = self.iter().map(|(k, v)| (k.debug(db), v.debug(db))); f.debug_map().entries(elements).finish() } } @@ -178,11 +196,12 @@ impl DebugWithDb for (A, B) where A: DebugWithDb, B: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { f.debug_tuple("") - .field(&self.0.debug_with(db, include_all_fields)) - .field(&self.1.debug_with(db, include_all_fields)) + .field(&self.0.debug(db)) + .field(&self.1.debug(db)) .finish() } } @@ -192,12 +211,13 @@ where A: DebugWithDb, B: DebugWithDb, C: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { f.debug_tuple("") - .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)) + .field(&self.0.debug(db)) + .field(&self.1.debug(db)) + .field(&self.2.debug(db)) .finish() } } @@ -205,9 +225,10 @@ where impl DebugWithDb for HashSet where V: DebugWithDb, + Db: AsSalsaDatabase, { - fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db, include_all_fields: bool) -> fmt::Result { - let elements = self.iter().map(|e| e.debug_with(db, include_all_fields)); + fn fmt(&self, f: &mut fmt::Formatter<'_>, db: &Db) -> fmt::Result { + let elements = self.iter().map(|e| e.debug(db)); f.debug_list().entries(elements).finish() } } @@ -217,27 +238,27 @@ where /// That's the "has impl" trick (https://github.com/nvzqz/impls#how-it-works) #[doc(hidden)] pub mod helper { - use super::{DebugWith, DebugWithDb}; + use super::{AsSalsaDatabase, DebugWith, DebugWithDb}; use std::{fmt, marker::PhantomData}; pub trait Fallback { - fn salsa_debug<'a>(a: &'a T, _db: &Db, _include_all_fields: bool) -> &'a dyn fmt::Debug { + fn salsa_debug<'a>(a: &'a T, _db: &Db) -> &'a dyn fmt::Debug { a } } + impl Fallback for Everything {} + pub struct SalsaDebug(PhantomData, PhantomData); - impl, Db: ?Sized> SalsaDebug { + impl SalsaDebug + where + T: DebugWithDb, + Db: AsSalsaDatabase, + { #[allow(dead_code)] - 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) + pub fn salsa_debug<'a, 'b: 'a>(a: &'a T, db: &'b Db) -> DebugWith<'a, Db> { + a.debug(db) } } - - impl Fallback for Everything {} } diff --git a/components/salsa-2022/src/event.rs b/components/salsa-2022/src/event.rs index ca2b9b68..fdbef715 100644 --- a/components/salsa-2022/src/event.rs +++ b/components/salsa-2022/src/event.rs @@ -28,15 +28,10 @@ impl DebugWithDb for Event where Db: ?Sized + Database, { - fn fmt( - &self, - f: &mut std::fmt::Formatter<'_>, - db: &Db, - include_all_fields: bool, - ) -> std::fmt::Result { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { f.debug_struct("Event") .field("runtime_id", &self.runtime_id) - .field("kind", &self.kind.debug_with(db, include_all_fields)) + .field("kind", &self.kind.debug(db)) .finish() } } @@ -154,19 +149,11 @@ impl DebugWithDb for EventKind where Db: ?Sized + Database, { - fn fmt( - &self, - fmt: &mut std::fmt::Formatter<'_>, - db: &Db, - include_all_fields: bool, - ) -> std::fmt::Result { + fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { match self { EventKind::DidValidateMemoizedValue { database_key } => fmt .debug_struct("DidValidateMemoizedValue") - .field( - "database_key", - &database_key.debug_with(db, include_all_fields), - ) + .field("database_key", &database_key.debug(db)) .finish(), EventKind::WillBlockOn { other_runtime_id, @@ -174,17 +161,11 @@ where } => fmt .debug_struct("WillBlockOn") .field("other_runtime_id", other_runtime_id) - .field( - "database_key", - &database_key.debug_with(db, include_all_fields), - ) + .field("database_key", &database_key.debug(db)) .finish(), EventKind::WillExecute { database_key } => fmt .debug_struct("WillExecute") - .field( - "database_key", - &database_key.debug_with(db, include_all_fields), - ) + .field("database_key", &database_key.debug(db)) .finish(), EventKind::WillCheckCancellation => fmt.debug_struct("WillCheckCancellation").finish(), EventKind::WillDiscardStaleOutput { @@ -192,29 +173,20 @@ where output_key, } => fmt .debug_struct("WillDiscardStaleOutput") - .field( - "execute_key", - &execute_key.debug_with(db, include_all_fields), - ) - .field("output_key", &output_key.debug_with(db, include_all_fields)) + .field("execute_key", &execute_key.debug(db)) + .field("output_key", &output_key.debug(db)) .finish(), EventKind::DidDiscard { key } => fmt .debug_struct("DidDiscard") - .field("key", &key.debug_with(db, include_all_fields)) + .field("key", &key.debug(db)) .finish(), EventKind::DidDiscardAccumulated { executor_key, accumulator, } => fmt .debug_struct("DidDiscardAccumulated") - .field( - "executor_key", - &executor_key.debug_with(db, include_all_fields), - ) - .field( - "accumulator", - &accumulator.debug_with(db, include_all_fields), - ) + .field("executor_key", &executor_key.debug(db)) + .field("accumulator", &accumulator.debug(db)) .finish(), } } diff --git a/components/salsa-2022/src/key.rs b/components/salsa-2022/src/key.rs index 8b71811d..c443b7bb 100644 --- a/components/salsa-2022/src/key.rs +++ b/components/salsa-2022/src/key.rs @@ -38,12 +38,7 @@ impl crate::debug::DebugWithDb for DependencyIndex where Db: ?Sized + Database, { - fn fmt( - &self, - f: &mut std::fmt::Formatter<'_>, - db: &Db, - _include_all_fields: bool, - ) -> std::fmt::Result { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { db.fmt_index(*self, f) } } @@ -73,14 +68,9 @@ impl crate::debug::DebugWithDb for DatabaseKeyIndex where Db: ?Sized + Database, { - fn fmt( - &self, - f: &mut std::fmt::Formatter<'_>, - db: &Db, - include_all_fields: bool, - ) -> std::fmt::Result { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &Db) -> std::fmt::Result { let i: DependencyIndex = (*self).into(); - DebugWithDb::fmt(&i, f, db, include_all_fields) + DebugWithDb::fmt(&i, f, db) } } diff --git a/components/salsa-2022/src/runtime.rs b/components/salsa-2022/src/runtime.rs index 87beb740..727f8342 100644 --- a/components/salsa-2022/src/runtime.rs +++ b/components/salsa-2022/src/runtime.rs @@ -104,6 +104,22 @@ impl Runtime { self.shared_state.empty_dependencies.clone() } + /// Executes `op` but ignores its effect on + /// the query dependencies; intended for use + /// by `DebugWithDb` only. + /// + /// # Danger: intended for debugging only + /// + /// This operation is intended for **debugging only**. + /// Misuse will cause Salsa to give incorrect results. + /// The expectation is that the type `R` produced will be + /// logged or printed out. **The type `R` that is produced + /// should not affect the result or other outputs + /// (such as accumulators) from the current Salsa query.** + pub fn debug_probe(&self, op: impl FnOnce() -> R) -> R { + self.local_state.debug_probe(op) + } + pub fn snapshot(&self) -> Self { if self.local_state.query_in_progress() { panic!("it is not legal to `snapshot` during a query (see salsa-rs/salsa#80)"); diff --git a/components/salsa-2022/src/runtime/active_query.rs b/components/salsa-2022/src/runtime/active_query.rs index 67ea202e..1af86f82 100644 --- a/components/salsa-2022/src/runtime/active_query.rs +++ b/components/salsa-2022/src/runtime/active_query.rs @@ -40,6 +40,26 @@ pub(super) struct ActiveQuery { pub(super) disambiguator_map: FxIndexMap, } +pub(super) struct SavedQueryState { + database_key_index: DatabaseKeyIndex, + durability: Durability, + changed_at: Revision, + input_outputs_len: usize, + untracked_read: bool, +} + +impl SavedQueryState { + fn new(query: &ActiveQuery) -> Self { + Self { + database_key_index: query.database_key_index, + durability: query.durability, + changed_at: query.changed_at, + input_outputs_len: query.input_outputs.len(), + untracked_read: query.untracked_read, + } + } +} + impl ActiveQuery { pub(super) fn new(database_key_index: DatabaseKeyIndex) -> Self { ActiveQuery { @@ -53,6 +73,26 @@ impl ActiveQuery { } } + pub(super) fn save_query_state(&self) -> SavedQueryState { + SavedQueryState::new(self) + } + + pub(super) fn restore_query_state(&mut self, state: SavedQueryState) { + assert_eq!(self.database_key_index, state.database_key_index); + + assert!(self.durability <= state.durability); + self.durability = state.durability; + + assert!(self.changed_at >= state.changed_at); + self.changed_at = state.changed_at; + + assert!(self.input_outputs.len() >= state.input_outputs_len); + self.input_outputs.truncate(state.input_outputs_len); + + assert!(self.untracked_read >= state.untracked_read); + self.untracked_read = state.untracked_read; + } + pub(super) fn add_read( &mut self, input: DependencyIndex, diff --git a/components/salsa-2022/src/runtime/local_state.rs b/components/salsa-2022/src/runtime/local_state.rs index 6405f850..2fc21cb3 100644 --- a/components/salsa-2022/src/runtime/local_state.rs +++ b/components/salsa-2022/src/runtime/local_state.rs @@ -174,6 +174,25 @@ impl LocalState { self.with_query_stack(|stack| !stack.is_empty()) } + /// Dangerous operation: executes `op` but ignores its effect on + /// the query dependencies. Useful for debugging statements, but + /// otherwise not to be toyed with! + pub(super) fn debug_probe(&self, op: impl FnOnce() -> R) -> R { + let saved_state: Option<_> = + self.with_query_stack(|stack| Some(stack.last()?.save_query_state())); + + let result = op(); + + if let Some(saved_state) = saved_state { + self.with_query_stack(|stack| { + let active_query = stack.last_mut().expect("query stack not empty"); + active_query.restore_query_state(saved_state); + }); + } + + result + } + /// Returns the index of the active query along with its *current* durability/changed-at /// information. As the query continues to execute, naturally, that information may change. pub(super) fn active_query(&self) -> Option<(DatabaseKeyIndex, StampedValue<()>)> { diff --git a/examples-2022/calc/src/parser.rs b/examples-2022/calc/src/parser.rs index bf99329a..38439c04 100644 --- a/examples-2022/calc/src/parser.rs +++ b/examples-2022/calc/src/parser.rs @@ -367,7 +367,7 @@ fn parse_string(source_text: &str) -> String { let accumulated = parse_statements::accumulated::(&db, source_program); // Format the result as a string and return it - format!("{:#?}", (statements.debug_all(&db), accumulated)) + format!("{:#?}", (statements.debug(&db), accumulated)) } // ANCHOR_END: parse_string diff --git a/salsa-2022-tests/tests/debug.rs b/salsa-2022-tests/tests/debug.rs index 204caf5f..c173d79e 100644 --- a/salsa-2022-tests/tests/debug.rs +++ b/salsa-2022-tests/tests/debug.rs @@ -4,7 +4,13 @@ use expect_test::expect; use salsa::DebugWithDb; #[salsa::jar(db = Db)] -struct Jar(MyInput, ComplexStruct); +struct Jar( + MyInput, + ComplexStruct, + leak_debug_string, + DerivedCustom, + leak_derived_custom, +); trait Db: salsa::DbWithJar {} @@ -44,15 +50,75 @@ fn input() { }; let complex_struct = ComplexStruct::new(&db, input, not_salsa); - // default debug only includes identity fields + // debug includes all fields let actual = format!("{:?}", complex_struct.debug(&db)); - let expected = expect!["ComplexStruct { [salsa id]: 0 }"]; - expected.assert_eq(&actual); - - // all fields - let actual = format!("{:?}", complex_struct.debug_all(&db)); 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); } + +#[salsa::tracked] +fn leak_debug_string(db: &dyn Db, input: MyInput) -> String { + format!("{:?}", input.debug(db)) +} + +/// Test that field reads that occur as part of `Debug` are not tracked. +/// Intentionally leaks the debug string. +/// Don't try this at home, kids. +#[test] +fn untracked_dependencies() { + let mut db = Database::default(); + + let input = MyInput::new(&db, 22); + + let s = leak_debug_string(&db, input); + expect![[r#" + "MyInput { [salsa id]: 0, field: 22 }" + "#]] + .assert_debug_eq(&s); + + input.set_field(&mut db).to(23); + + // check that we reuse the cached result for debug string + // even though the dependency changed. + let s = leak_debug_string(&db, input); + assert!(s.contains(", field: 22 }")); +} + +#[salsa::tracked] +#[customize(DebugWithDb)] +struct DerivedCustom { + my_input: MyInput, + value: u32, +} + +impl DebugWithDb for DerivedCustom { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn Db) -> std::fmt::Result { + write!( + f, + "{:?} / {:?}", + self.my_input(db).debug(db), + self.value(db) + ) + } +} + +#[salsa::tracked] +fn leak_derived_custom(db: &dyn Db, input: MyInput, value: u32) -> String { + let c = DerivedCustom::new(db, input, value); + format!("{:?}", c.debug(db)) +} + +#[test] +fn custom_debug_impl() { + let db = Database::default(); + + let input = MyInput::new(&db, 22); + + let s = leak_derived_custom(&db, input, 23); + expect![[r#" + "MyInput { [salsa id]: 0, field: 22 } / 23" + "#]] + .assert_debug_eq(&s); +} diff --git a/salsa-2022-tests/tests/input_with_ids.rs b/salsa-2022-tests/tests/input_with_ids.rs index f6aacd7b..3656c8e3 100644 --- a/salsa-2022-tests/tests/input_with_ids.rs +++ b/salsa-2022-tests/tests/input_with_ids.rs @@ -38,7 +38,7 @@ fn test_debug() { let input = MyInput::new(&mut db, 50, 10, Field {}); let actual = format!("{:?}", input.debug(&db)); - let expected = expect![[r#"MyInput { [salsa id]: 0, id_one: 50, id_two: 10 }"#]]; + let expected = expect!["MyInput { [salsa id]: 0, id_one: 50, id_two: 10, field: Field }"]; expected.assert_eq(&actual); } diff --git a/salsa-2022-tests/tests/singleton.rs b/salsa-2022-tests/tests/singleton.rs index 275a2d3c..d72b19ed 100644 --- a/salsa-2022-tests/tests/singleton.rs +++ b/salsa-2022-tests/tests/singleton.rs @@ -67,6 +67,6 @@ fn debug() { let db = Database::default(); let input = MyInput::new(&db, 3, 4); let actual = format!("{:?}", input.debug(&db)); - let expected = expect![[r#"MyInput { [salsa id]: 0, id_field: 4 }"#]]; + let expected = expect!["MyInput { [salsa id]: 0, field: 3, id_field: 4 }"]; expected.assert_eq(&actual); } diff --git a/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs b/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs index b31c6a54..12257f64 100644 --- a/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs +++ b/salsa-2022-tests/tests/tracked_fn_read_own_specify.rs @@ -55,7 +55,7 @@ fn execute() { assert_eq!(tracked_fn(&db, input), 2222); db.assert_logs(expect![[r#" [ - "tracked_fn(MyInput { [salsa id]: 0 })", + "tracked_fn(MyInput { [salsa id]: 0, field: 22 })", ]"#]]); // A "synthetic write" causes the system to act *as though* some diff --git a/salsa-2022-tests/tests/warnings/needless_borrow.rs b/salsa-2022-tests/tests/warnings/needless_borrow.rs index 92cd0677..502d23e2 100644 --- a/salsa-2022-tests/tests/warnings/needless_borrow.rs +++ b/salsa-2022-tests/tests/warnings/needless_borrow.rs @@ -7,12 +7,7 @@ struct Jar(TokenTree); enum Token {} impl salsa::DebugWithDb for Token { - fn fmt( - &self, - _f: &mut std::fmt::Formatter<'_>, - _db: &dyn Db, - _include_all_fields: bool, - ) -> std::fmt::Result { + fn fmt(&self, _f: &mut std::fmt::Formatter<'_>, _db: &dyn Db) -> std::fmt::Result { unreachable!() } }