Repository: salsa-rs/salsa (rust)

Issue:
Automatically derive `DebugInDb` for salsa structs
When you create a salsa struct (tracked/input/interned), we should automatically implement both `Debug` and `DebugInDb`.

For tracked/input structs in particular, the identity of the struct is important, so it is important to include not only the value of the fields but also the value of `self.0` (i.e., its numeric identity).

Pull request:
Derive DebugWithDb
Closes #317 

Patch:
diff --git a/book/src/tutorial/debug.md b/book/src/tutorial/debug.md
index d7302c2b..1fd30767 100644
--- a/book/src/tutorial/debug.md
+++ b/book/src/tutorial/debug.md
@@ -18,23 +18,7 @@ eprintln!("Expression = {:?}", expr.debug(db));
 
 and get back the output you expect.
 
-## Implementing the `DebugWithDb` trait
-
-For now, unfortunately, you have to implement the `DebugWithDb` trait manually, as we do not provide a derive.
-This is tedious but not difficult. Here is an example of implementing the trait for `Expression`:
-
-```rust
-{{#include ../../../calc-example/calc/src/ir.rs:expression_debug_impl}}
-```
-
-Some things to note:
-
-- The `data` method gives access to the full enum from the database.
-- The [`Formatter`] methods (e.g., [`debug_tuple`]) can be used to provide consistent output.
-- When printing the value of a field, use `.field(&a.debug(db))` for fields that are themselves interned or entities, and use `.field(&a)` for fields that just implement the ordinary `Debug` trait.
-
-[`debug_tuple`]: https://doc.rust-lang.org/std/fmt/struct.Formatter.html#method.debug_tuple
-[`formatter`]: https://doc.rust-lang.org/std/fmt/struct.Formatter.html#
+The `DebugWithDb` trait is automatically derived for all `#[input]`, `#[interned]` and `#[tracked]` structs.
 
 ## Forwarding to the ordinary `Debug` trait
 
diff --git a/calc-example/calc/src/ir.rs b/calc-example/calc/src/ir.rs
index c122ba2d..9321c4e0 100644
--- a/calc-example/calc/src/ir.rs
+++ b/calc-example/calc/src/ir.rs
@@ -72,85 +72,6 @@ pub enum Op {
 }
 // ANCHOR_END: statements_and_expressions
 
-impl DebugWithDb<dyn crate::Db + '_> for Function {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn crate::Db) -> std::fmt::Result {
-        f.debug_struct("Function")
-            .field("name", &self.name(db).debug(db))
-            .field("args", &self.args(db).debug(db))
-            .field("body", &self.body(db).debug(db))
-            .finish()
-    }
-}
-
-impl DebugWithDb<dyn crate::Db + '_> for Statement {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn crate::Db) -> std::fmt::Result {
-        match &self.data {
-            StatementData::Function(a) => DebugWithDb::fmt(&a, f, db),
-            StatementData::Print(a) => DebugWithDb::fmt(&a, f, db),
-        }
-    }
-}
-
-// ANCHOR: expression_debug_impl
-impl DebugWithDb<dyn crate::Db + '_> for Expression {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn crate::Db) -> std::fmt::Result {
-        match &self.data {
-            ExpressionData::Op(a, b, c) => f
-                .debug_tuple("ExpressionData::Op")
-                .field(&a.debug(db)) // use `a.debug(db)` for interned things
-                .field(&b.debug(db))
-                .field(&c.debug(db))
-                .finish(),
-            ExpressionData::Number(a) => {
-                f.debug_tuple("Number")
-                    .field(a) // use just `a` otherwise
-                    .finish()
-            }
-            ExpressionData::Variable(a) => f.debug_tuple("Variable").field(&a.debug(db)).finish(),
-            ExpressionData::Call(a, b) => f
-                .debug_tuple("Call")
-                .field(&a.debug(db))
-                .field(&b.debug(db))
-                .finish(),
-        }
-    }
-}
-// ANCHOR_END: expression_debug_impl
-
-impl DebugWithDb<dyn crate::Db + '_> for Program {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn crate::Db) -> std::fmt::Result {
-        f.debug_struct("Program")
-            .field("statements", &self.statements(db))
-            .finish()
-    }
-}
-
-impl DebugWithDb<dyn crate::Db + '_> for FunctionId {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn crate::Db) -> std::fmt::Result {
-        write!(f, "{:?}", self.text(db))
-    }
-}
-
-impl DebugWithDb<dyn crate::Db + '_> for VariableId {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn crate::Db) -> std::fmt::Result {
-        write!(f, "{:?}", self.text(db))
-    }
-}
-
-// ANCHOR: op_debug_impl
-impl DebugWithDb<dyn crate::Db + '_> for Op {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>, _db: &dyn crate::Db) -> std::fmt::Result {
-        write!(f, "{:?}", self)
-    }
-}
-// ANCHOR: op_debug_impl
-
-impl DebugWithDb<dyn crate::Db + '_> for Diagnostic {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>, _db: &dyn crate::Db) -> std::fmt::Result {
-        write!(f, "{:?}", self)
-    }
-}
-
 // ANCHOR: functions
 #[salsa::tracked]
 pub struct Function {
diff --git a/calc-example/calc/src/parser.rs b/calc-example/calc/src/parser.rs
index c01acb39..43b99950 100644
--- a/calc-example/calc/src/parser.rs
+++ b/calc-example/calc/src/parser.rs
@@ -371,7 +371,7 @@ fn parse_string(source_text: &str) -> String {
     let accumulated = parse_statements::accumulated::<Diagnostics>(&db, source_program);
 
     // Format the result as a string and return it
-    format!("{:#?}", (statements, accumulated).debug(&db))
+    format!("{:#?}", (statements.debug(&db), accumulated))
 }
 // ANCHOR_END: parse_string
 
@@ -382,6 +382,7 @@ fn parse_print() {
     let expected = expect_test::expect![[r#"
         (
             Program {
+                [salsa id]: 0,
                 statements: [
                     Statement {
                         span: Span(
@@ -448,6 +449,7 @@ fn parse_example() {
     let expected = expect_test::expect![[r#"
         (
             Program {
+                [salsa id]: 0,
                 statements: [
                     Statement {
                         span: Span(
@@ -621,10 +623,15 @@ fn parse_error() {
     let expected = expect_test::expect![[r#"
         (
             Program {
+                [salsa id]: 0,
                 statements: [],
             },
             [
-                Diagnostic { start: 10, end: 11, message: "unexpected character" },
+                Diagnostic {
+                    start: 10,
+                    end: 11,
+                    message: "unexpected character",
+                },
             ],
         )"#]];
     expected.assert_eq(&actual);
@@ -638,6 +645,7 @@ fn parse_precedence() {
     let expected = expect_test::expect![[r#"
         (
             Program {
+                [salsa id]: 0,
                 statements: [
                     Statement {
                         span: Span(
diff --git a/components/salsa-2022-macros/src/input.rs b/components/salsa-2022-macros/src/input.rs
index f10c8a78..99e7a246 100644
--- a/components/salsa-2022-macros/src/input.rs
+++ b/components/salsa-2022-macros/src/input.rs
@@ -38,6 +38,7 @@ impl InputStruct {
         let ingredients_for_impl = self.input_ingredients();
         let as_id_impl = self.as_id_impl();
         let salsa_struct_in_db_impl = self.salsa_struct_in_db_impl();
+        let as_debug_with_db_impl = self.as_debug_with_db_impl();
 
         Ok(quote! {
             #(#config_structs)*
@@ -45,6 +46,7 @@ impl InputStruct {
             #inherent_impl
             #ingredients_for_impl
             #as_id_impl
+            #as_debug_with_db_impl
             #(#config_impls)*
             #salsa_struct_in_db_impl
         })
diff --git a/components/salsa-2022-macros/src/interned.rs b/components/salsa-2022-macros/src/interned.rs
index 54df62ab..47d75140 100644
--- a/components/salsa-2022-macros/src/interned.rs
+++ b/components/salsa-2022-macros/src/interned.rs
@@ -39,6 +39,7 @@ impl InternedStruct {
         let as_id_impl = self.as_id_impl();
         let named_fields_impl = self.inherent_impl_for_named_fields();
         let salsa_struct_in_db_impl = self.salsa_struct_in_db_impl();
+        let as_debug_with_db_impl = self.as_debug_with_db_impl();
 
         Ok(quote! {
             #id_struct
@@ -47,6 +48,7 @@ impl InternedStruct {
             #as_id_impl
             #named_fields_impl
             #salsa_struct_in_db_impl
+            #as_debug_with_db_impl
         })
     }
 
diff --git a/components/salsa-2022-macros/src/salsa_struct.rs b/components/salsa-2022-macros/src/salsa_struct.rs
index 7473d1c1..00ed928b 100644
--- a/components/salsa-2022-macros/src/salsa_struct.rs
+++ b/components/salsa-2022-macros/src/salsa_struct.rs
@@ -26,7 +26,8 @@
 //!     * this could be optimized, particularly for interned fields
 
 use heck::ToUpperCamelCase;
-use proc_macro2::{Ident, Literal, Span};
+use proc_macro2::{Ident, Literal, Span, TokenStream};
+use syn::spanned::Spanned;
 
 use crate::{configuration, options::Options};
 
@@ -293,6 +294,56 @@ impl SalsaStruct {
         }
     }
 
+    /// Generate `impl salsa::DebugWithDb for Foo`
+    pub(crate) fn as_debug_with_db_impl(&self) -> syn::ItemImpl {
+        let ident = self.id_ident();
+
+        let db_type = self.db_dyn_ty();
+        let ident_string = ident.to_string();
+
+        let fields = self.all_fields().map(|field| {
+            let field_name_string = field.name().to_string();
+            let field_getter = field.get_name();
+            let field_ty = field.ty();
+            // If the field type implements DebugWithDb, use that.
+            // Otherwise, use Debug.
+            // That's the "has impl" trick (https://github.com/nvzqz/impls#how-it-works)
+            parse_quote_spanned! {field.field.span()=>
+                .field(#field_name_string, {
+                    struct Test<T, Db: ?Sized>(::core::marker::PhantomData<T>, ::core::marker::PhantomData<Db>);
+
+                    #[allow(dead_code)]
+                    impl<T: ::salsa::debug::DebugWithDb<Db>, Db: ?Sized> Test<T, Db> {
+                        fn salsa_debug<'a, 'b: 'a>(a: &'a T, db: &'b Db) -> ::salsa::debug::DebugWith<'a, Db> {
+                            a.debug(db)
+                        }
+                    }
+
+                    trait Fallback<T: ::core::fmt::Debug, Db: ?Sized> {
+                        fn salsa_debug<'a, 'b>(a: &'a T, _db: &'b Db) -> &'a dyn ::core::fmt::Debug {
+                            a
+                        }
+                    }
+
+                    impl<Everything, Db: ?Sized, T: ::core::fmt::Debug> Fallback<T, Db> for Everything {}
+
+                    &Test::<#field_ty, #db_type>::salsa_debug(&self.#field_getter(db), db)
+                })
+            }
+        }).collect::<Vec<TokenStream>>();
+
+        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 {
+                    f.debug_struct(#ident_string)
+                        .field("[salsa id]", &self.0.as_u32())
+                        #(#fields)*
+                        .finish()
+                }
+            }
+        }
+    }
+
     /// Disallow `#[id]` attributes on the fields of this struct.
     ///
     /// If an `#[id]` field is found, return an error.
diff --git a/components/salsa-2022-macros/src/tracked_struct.rs b/components/salsa-2022-macros/src/tracked_struct.rs
index 99f12b53..7bec3f54 100644
--- a/components/salsa-2022-macros/src/tracked_struct.rs
+++ b/components/salsa-2022-macros/src/tracked_struct.rs
@@ -42,6 +42,7 @@ impl TrackedStruct {
         let salsa_struct_in_db_impl = self.salsa_struct_in_db_impl();
         let tracked_struct_in_db_impl = self.tracked_struct_in_db_impl();
         let as_id_impl = self.as_id_impl();
+        let as_debug_with_db_impl = self.as_debug_with_db_impl();
         Ok(quote! {
             #(#config_structs)*
             #id_struct
@@ -50,6 +51,7 @@ impl TrackedStruct {
             #salsa_struct_in_db_impl
             #tracked_struct_in_db_impl
             #as_id_impl
+            #as_debug_with_db_impl
             #(#config_impls)*
         })
     }


Test patch:
diff --git a/salsa-2022-tests/tests/cycles.rs b/salsa-2022-tests/tests/cycles.rs
index 6aeb3ed9..90a8bb5a 100644
--- a/salsa-2022-tests/tests/cycles.rs
+++ b/salsa-2022-tests/tests/cycles.rs
@@ -4,7 +4,6 @@ use std::panic::{RefUnwindSafe, UnwindSafe};
 
 use expect_test::expect;
 use salsa::storage::HasJarsDyn;
-use salsa::Durability;
 
 // Axes:
 //
@@ -41,7 +40,6 @@ use salsa::Durability;
 // | Intra  | Fallback | Both     | Tracked   | direct   | cycle_revalidate |
 // | Intra  | Fallback | New      | Tracked   | direct   | cycle_appears |
 // | Intra  | Fallback | Old      | Tracked   | direct   | cycle_disappears |
-// | Intra  | Fallback | Old      | Tracked   | direct   | cycle_disappears_durability |
 // | Intra  | Mixed    | N/A      | Tracked   | direct   | cycle_mixed_1 |
 // | Intra  | Mixed    | N/A      | Tracked   | direct   | cycle_mixed_2 |
 // | Cross  | Panic    | N/A      | Tracked   | both     | parallel/parallel_cycle_none_recover.rs |
@@ -297,41 +295,6 @@ fn cycle_disappears() {
     assert!(cycle_a(&db, abc).is_ok());
 }
 
-/// A variant on `cycle_disappears` in which the values of
-/// `a` and `b` are set with durability values.
-/// If we are not careful, this could cause us to overlook
-/// the fact that the cycle will no longer occur.
-#[test]
-fn cycle_disappears_durability() {
-    let mut db = Database::default();
-    let abc = ABC::new(
-        &mut db,
-        CycleQuery::None,
-        CycleQuery::None,
-        CycleQuery::None,
-    );
-    abc.set_a(&mut db)
-        .with_durability(Durability::LOW)
-        .to(CycleQuery::B);
-    abc.set_b(&mut db)
-        .with_durability(Durability::HIGH)
-        .to(CycleQuery::A);
-
-    assert!(cycle_a(&db, abc).is_err());
-
-    // At this point, `a` read `LOW` input, and `b` read `HIGH` input. However,
-    // because `b` participates in the same cycle as `a`, its final durability
-    // should be `LOW`.
-    //
-    // Check that setting a `LOW` input causes us to re-execute `b` query, and
-    // observe that the cycle goes away.
-    abc.set_a(&mut db)
-        .with_durability(Durability::LOW)
-        .to(CycleQuery::None);
-
-    assert!(cycle_b(&mut db, abc).is_ok());
-}
-
 #[test]
 fn cycle_mixed_1() {
     let mut db = Database::default();
diff --git a/salsa-2022-tests/tests/debug.rs b/salsa-2022-tests/tests/debug.rs
new file mode 100644
index 00000000..f0b4d1f7
--- /dev/null
+++ b/salsa-2022-tests/tests/debug.rs
@@ -0,0 +1,56 @@
+//! Test that `DeriveWithDb` is correctly derived.
+
+use expect_test::expect;
+use salsa::DebugWithDb;
+
+#[salsa::jar(db = Db)]
+struct Jar(MyInput, ComplexStruct);
+
+trait Db: salsa::DbWithJar<Jar> {}
+
+#[salsa::input]
+struct MyInput {
+    field: u32,
+}
+
+#[derive(Debug, Eq, PartialEq, Clone)]
+struct NotSalsa {
+    field: String,
+}
+
+#[salsa::input]
+struct ComplexStruct {
+    my_input: MyInput,
+    not_salsa: NotSalsa,
+}
+
+#[salsa::db(Jar)]
+#[derive(Default)]
+struct Database {
+    storage: salsa::Storage<Self>,
+}
+
+impl salsa::Database for Database {
+    fn salsa_runtime(&self) -> &salsa::Runtime {
+        self.storage.runtime()
+    }
+}
+
+impl Db for Database {}
+
+#[test]
+fn input() {
+    let mut db = Database::default();
+
+    let input = MyInput::new(&mut db, 22);
+    let not_salsa = NotSalsa {
+        field: "it's salsa time".to_string(),
+    };
+    let complex_struct = ComplexStruct::new(&mut db, input, not_salsa);
+
+    let actual = format!("{:?}", complex_struct.debug(&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);
+}