feat: Drop Debug requirements and flip implementation defaults (#756)

Author: Veykril

benches/compare.rs

@@ -26,7 +26,7 @@ pub struct InternedInput<'db> {
     pub text: String,
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, salsa::Supertype)]
+#[derive(Clone, Copy, PartialEq, Eq, Hash, salsa::Supertype)]
 enum SupertypeInput<'db> {
     InternedInput(InternedInput<'db>),
     Input(Input),

components/salsa-macro-rules/src/setup_input_struct.rs

@@ -223,7 +223,12 @@ macro_rules! setup_input_struct {
                 }
 
                 /// Default debug formatting for this struct (may be useful if you define your own `Debug` impl)
-                pub fn default_debug_fmt(this: Self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+                pub fn default_debug_fmt(this: Self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result
+                where
+                    // rustc rejects trivial bounds, but it cannot see through higher-ranked bounds
+                    // with its check :^)
+                    $(for<'__trivial_bounds> $field_ty: std::fmt::Debug),*
+                {
                     $zalsa::with_attached_database(|db| {
                         let fields = $Configuration::ingredient(db).leak_fields(db, this);
                         let mut f = f.debug_struct(stringify!($Struct));

components/salsa-macro-rules/src/setup_tracked_struct.rs

@@ -286,9 +286,18 @@ macro_rules! setup_tracked_struct {
                         )
                     }
                 )*
+            }
 
+            impl<'_db> $Struct<'_db> {
                 /// Default debug formatting for this struct (may be useful if you define your own `Debug` impl)
-                pub fn default_debug_fmt(this: Self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+                pub fn default_debug_fmt(this: Self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result
+                where
+                    // `zalsa::with_attached_database` has a local lifetime for the database
+                    // so we need this function to be higher-ranked over the db lifetime
+                    // Thus the actual lifetime of `Self` does not matter here so we discard
+                    // it with the `'_db` lifetime name as we cannot shadow lifetimes.
+                    $(for<$db_lt> $field_ty: std::fmt::Debug),*
+                {
                     $zalsa::with_attached_database(|db| {
                         let fields = $Configuration::ingredient(db).leak_fields(db, this);
                         let mut f = f.debug_struct(stringify!($Struct));

components/salsa-macros/src/accumulator.rs

@@ -19,7 +19,7 @@ pub(crate) fn accumulator(
     let ident = struct_item.ident.clone();
     let m = StructMacro {
         hygiene,
-        args,
+        _args: args,
         struct_item,
     };
     match m.try_expand() {
@@ -34,8 +34,8 @@ impl AllowedOptions for Accumulator {
     const RETURN_REF: bool = false;
     const SPECIFY: bool = false;
     const NO_EQ: bool = false;
-    const NO_DEBUG: bool = true;
-    const NO_CLONE: bool = true;
+    const DEBUG: bool = false;
+    const NO_CLONE: bool = false;
     const NO_LIFETIME: bool = false;
     const SINGLETON: bool = false;
     const DATA: bool = false;
@@ -49,7 +49,7 @@ impl AllowedOptions for Accumulator {
 
 struct StructMacro {
     hygiene: Hygiene,
-    args: Options<Accumulator>,
+    _args: Options<Accumulator>,
     struct_item: syn::ItemStruct,
 }
 
@@ -65,16 +65,7 @@ impl StructMacro {
 
         let struct_item = self.struct_item;
 
-        let mut derives = vec![];
-        if self.args.no_debug.is_none() {
-            derives.push(quote!(Debug));
-        }
-        if self.args.no_clone.is_none() {
-            derives.push(quote!(Clone));
-        }
-
         Ok(quote! {
-            #[derive(#(#derives),*)]
             #struct_item
 
             salsa::plumbing::setup_accumulator_impl! {

components/salsa-macros/src/input.rs

@@ -40,7 +40,7 @@ impl crate::options::AllowedOptions for InputStruct {
 
     const NO_EQ: bool = false;
 
-    const NO_DEBUG: bool = true;
+    const DEBUG: bool = true;
 
     const NO_LIFETIME: bool = false;
 

components/salsa-macros/src/interned.rs

@@ -41,7 +41,7 @@ impl crate::options::AllowedOptions for InternedStruct {
 
     const NO_EQ: bool = false;
 
-    const NO_DEBUG: bool = true;
+    const DEBUG: bool = true;
 
     const NO_LIFETIME: bool = true;
 

components/salsa-macros/src/options.rs

@@ -20,10 +20,10 @@ pub(crate) struct Options<A: AllowedOptions> {
     /// If this is `Some`, the value is the `no_eq` identifier.
     pub no_eq: Option<syn::Ident>,
 
-    /// Signal we should not generate a `Debug` impl.
+    /// Signal we should generate a `Debug` impl.
     ///
-    /// If this is `Some`, the value is the `no_debug` identifier.
-    pub no_debug: Option<syn::Ident>,
+    /// If this is `Some`, the value is the `debug` identifier.
+    pub debug: Option<syn::Ident>,
 
     /// Signal we should not include the `'db` lifetime.
     ///
@@ -93,7 +93,7 @@ impl<A: AllowedOptions> Default for Options<A> {
             return_ref: Default::default(),
             specify: Default::default(),
             no_eq: Default::default(),
-            no_debug: Default::default(),
+            debug: Default::default(),
             no_lifetime: Default::default(),
             no_clone: Default::default(),
             db_path: Default::default(),
@@ -114,7 +114,7 @@ pub(crate) trait AllowedOptions {
     const RETURN_REF: bool;
     const SPECIFY: bool;
     const NO_EQ: bool;
-    const NO_DEBUG: bool;
+    const DEBUG: bool;
     const NO_LIFETIME: bool;
     const NO_CLONE: bool;
     const SINGLETON: bool;
@@ -161,18 +161,15 @@ impl<A: AllowedOptions> syn::parse::Parse for Options<A> {
                         "`no_eq` option not allowed here",
                     ));
                 }
-            } else if ident == "no_debug" {
-                if A::NO_DEBUG {
-                    if let Some(old) = options.no_debug.replace(ident) {
-                        return Err(syn::Error::new(
-                            old.span(),
-                            "option `no_debug` provided twice",
-                        ));
+            } else if ident == "debug" {
+                if A::DEBUG {
+                    if let Some(old) = options.debug.replace(ident) {
+                        return Err(syn::Error::new(old.span(), "option `debug` provided twice"));
                     }
                 } else {
                     return Err(syn::Error::new(
                         ident.span(),
-                        "`no_debug` option not allowed here",
+                        "`debug` option not allowed here",
                     ));
                 }
             } else if ident == "no_lifetime" {

components/salsa-macros/src/salsa_struct.rs

@@ -330,7 +330,7 @@ where
     }
 
     pub fn generate_debug_impl(&self) -> bool {
-        self.args.no_debug.is_none()
+        self.args.debug.is_some()
     }
 
     pub fn generate_lifetime(&self) -> bool {

components/salsa-macros/src/tracked_fn.rs

@@ -29,7 +29,7 @@ impl crate::options::AllowedOptions for TrackedFn {
 
     const NO_EQ: bool = true;
 
-    const NO_DEBUG: bool = false;
+    const DEBUG: bool = false;
 
     const NO_LIFETIME: bool = false;
 

components/salsa-macros/src/tracked_struct.rs

@@ -35,7 +35,7 @@ impl crate::options::AllowedOptions for TrackedStruct {
 
     const NO_EQ: bool = false;
 
-    const NO_DEBUG: bool = true;
+    const DEBUG: bool = true;
 
     const NO_LIFETIME: bool = false;
 

examples/calc/ir.rs

@@ -3,29 +3,29 @@
 use ordered_float::OrderedFloat;
 
 // ANCHOR: input
-#[salsa::input]
+#[salsa::input(debug)]
 pub struct SourceProgram {
     #[return_ref]
     pub text: String,
 }
 // ANCHOR_END: input
 
 // ANCHOR: interned_ids
-#[salsa::interned]
+#[salsa::interned(debug)]
 pub struct VariableId<'db> {
     #[return_ref]
     pub text: String,
 }
 
-#[salsa::interned]
+#[salsa::interned(debug)]
 pub struct FunctionId<'db> {
     #[return_ref]
     pub text: String,
 }
 // ANCHOR_END: interned_ids
 
 // ANCHOR: program
-#[salsa::tracked]
+#[salsa::tracked(debug)]
 pub struct Program<'db> {
     #[tracked]
     #[return_ref]
@@ -86,7 +86,7 @@ pub enum Op {
 // ANCHOR_END: statements_and_expressions
 
 // ANCHOR: functions
-#[salsa::tracked]
+#[salsa::tracked(debug)]
 pub struct Function<'db> {
     pub name: FunctionId<'db>,
 
@@ -102,7 +102,7 @@ pub struct Function<'db> {
 }
 // ANCHOR_END: functions
 
-#[salsa::tracked]
+#[salsa::tracked(debug)]
 pub struct Span<'db> {
     #[tracked]
     pub start: usize,
@@ -112,6 +112,7 @@ pub struct Span<'db> {
 
 // ANCHOR: diagnostic
 #[salsa::accumulator]
+#[derive(Debug)]
 #[allow(dead_code)] // Debug impl uses them
 pub struct Diagnostic {
     pub start: usize,

src/accumulator.rs

@@ -2,7 +2,7 @@
 
 use std::{
     any::{Any, TypeId},
-    fmt::{self, Debug},
+    fmt,
     marker::PhantomData,
     panic::UnwindSafe,
 };
@@ -25,7 +25,7 @@ pub(crate) mod accumulated_map;
 
 /// Trait implemented on the struct that user annotated with `#[salsa::accumulator]`.
 /// The `Self` type is therefore the types to be accumulated.
-pub trait Accumulator: Debug + Send + Sync + Any + Sized + UnwindSafe {
+pub trait Accumulator: Send + Sync + Any + Sized + UnwindSafe {
     const DEBUG_NAME: &'static str;
 
     /// Accumulate an instance of this in the database for later retrieval.

src/accumulator/accumulated.rs

@@ -8,7 +8,7 @@ pub(crate) struct Accumulated<A: Accumulator> {
     values: Vec<A>,
 }
 
-pub(crate) trait AnyAccumulated: Any + Debug + Send + Sync {
+pub(crate) trait AnyAccumulated: Any + Send + Sync {
     fn as_dyn_any(&self) -> &dyn Any;
     fn as_dyn_any_mut(&mut self) -> &mut dyn Any;
 }

src/accumulator/accumulated_map.rs

@@ -9,11 +9,19 @@ use crate::IngredientIndex;
 
 use super::{accumulated::Accumulated, Accumulator, AnyAccumulated};
 
-#[derive(Default, Debug)]
+#[derive(Default)]
 pub struct AccumulatedMap {
     map: FxHashMap<IngredientIndex, Box<dyn AnyAccumulated>>,
 }
 
+impl std::fmt::Debug for AccumulatedMap {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("AccumulatedMap")
+            .field("map", &self.map.keys())
+            .finish()
+    }
+}
+
 impl AccumulatedMap {
     pub fn accumulate<A: Accumulator>(&mut self, index: IngredientIndex, value: A) {
         self.map

src/function.rs

@@ -48,7 +48,7 @@ pub trait Configuration: Any {
     type Input<'db>: Send + Sync;
 
     /// The value computed by the function.
-    type Output<'db>: fmt::Debug + Send + Sync;
+    type Output<'db>: Send + Sync;
 
     /// Determines whether this function can recover from being a participant in a cycle
     /// (and, if so, how).

src/function/memo.rs

@@ -276,7 +276,7 @@ impl<V> Memo<V> {
     }
 }
 
-impl<V: Debug + Send + Sync + Any> crate::table::memo::Memo for Memo<V> {
+impl<V: Send + Sync + Any> crate::table::memo::Memo for Memo<V> {
     fn origin(&self) -> &QueryOrigin {
         &self.revisions.origin
     }

src/id.rs

@@ -59,7 +59,7 @@ pub trait AsId: Sized {
 }
 
 /// Internal Salsa trait for types that are just a newtype'd [`Id`][].
-pub trait FromId: AsId + Copy + Eq + Hash + Debug {
+pub trait FromId: AsId + Copy + Eq + Hash {
     fn from_id(id: Id) -> Self;
 }
 
@@ -77,7 +77,7 @@ impl FromId for Id {
 
 /// Enums cannot use [`FromId`] because they need access to the DB to tell the `TypeId` of the variant,
 /// so they use this trait instead, that has a blanket implementation for `FromId`.
-pub trait FromIdWithDb: AsId + Copy + Eq + Hash + Debug {
+pub trait FromIdWithDb: AsId + Copy + Eq + Hash {
     fn from_id(id: Id, db: &(impl ?Sized + Database)) -> Self;
 }
 

src/table/memo.rs

@@ -1,6 +1,5 @@
 use std::{
     any::{Any, TypeId},
-    fmt::Debug,
     ptr::NonNull,
     sync::atomic::{AtomicPtr, Ordering},
 };
@@ -17,7 +16,7 @@ pub(crate) struct MemoTable {
     memos: RwLock<Vec<MemoEntry>>,
 }
 
-pub(crate) trait Memo: Any + Send + Sync + Debug {
+pub(crate) trait Memo: Any + Send + Sync {
     /// Returns the `origin` of this memo
     fn origin(&self) -> &QueryOrigin;
 }

tests/accumulate-chain.rs

@@ -8,6 +8,7 @@ use salsa::{Accumulator, Database, DatabaseImpl};
 use test_log::test;
 
 #[salsa::accumulator]
+#[derive(Debug)]
 struct Log(#[allow(dead_code)] String);
 
 #[salsa::tracked]

tests/accumulate-custom-debug.rs

@@ -4,12 +4,12 @@ use expect_test::expect;
 use salsa::{Accumulator, Database};
 use test_log::test;
 
-#[salsa::input]
+#[salsa::input(debug)]
 struct MyInput {
     count: u32,
 }
 
-#[salsa::accumulator(no_debug)]
+#[salsa::accumulator]
 struct Log(String);
 
 impl std::fmt::Debug for Log {

tests/accumulate-dag.rs

@@ -4,13 +4,14 @@ use expect_test::expect;
 use salsa::{Accumulator, Database};
 use test_log::test;
 
-#[salsa::input]
+#[salsa::input(debug)]
 struct MyInput {
     field_a: u32,
     field_b: u32,
 }
 
 #[salsa::accumulator]
+#[derive(Debug)]
 struct Log(#[allow(dead_code)] String);
 
 #[salsa::tracked]

tests/accumulate-execution-order.rs

@@ -8,6 +8,7 @@ use salsa::{Accumulator, Database};
 use test_log::test;
 
 #[salsa::accumulator]
+#[derive(Debug)]
 struct Log(#[allow(dead_code)] String);
 
 #[salsa::tracked]