From 43cd95fe47a6d83cf46fbb7da4502afe32f09cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Sun, 21 Aug 2022 21:44:51 +0200 Subject: [PATCH 1/8] Derive DebugWithDb --- calc-example/calc/src/ir.rs | 79 ------------------- calc-example/calc/src/main.rs | 11 ++- components/salsa-2022-macros/src/input.rs | 2 + components/salsa-2022-macros/src/interned.rs | 2 + .../salsa-2022-macros/src/salsa_struct.rs | 53 ++++++++++++- .../salsa-2022-macros/src/tracked_struct.rs | 2 + 6 files changed, 68 insertions(+), 81 deletions(-) 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 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 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 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 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 for FunctionId { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>, db: &dyn crate::Db) -> std::fmt::Result { - write!(f, "{:?}", self.text(db)) - } -} - -impl DebugWithDb 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 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 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/main.rs b/calc-example/calc/src/main.rs index 27495343..ee5b3083 100644 --- a/calc-example/calc/src/main.rs +++ b/calc-example/calc/src/main.rs @@ -1,4 +1,5 @@ use ir::{Diagnostics, SourceProgram}; +use salsa::DebugWithDb; // ANCHOR: jar_struct #[salsa::jar(db = Db)] @@ -32,9 +33,17 @@ mod ir; mod parser; mod type_check; +const PROGRAM: &str = r" +fn area_rectangle(w, h) = w * h +fn area_circle(r) = 3.14 * r * r +print area_rectangle(3, 4) +print area_circle(1) +print 11 * 2 +"; + pub fn main() { let mut db = db::Database::default(); - let source_program = SourceProgram::new(&mut db, String::new()); + let source_program = SourceProgram::new(&mut db, PROGRAM.to_string()); compile::compile(&db, source_program); let diagnostics = compile::compile::accumulated::(&db, source_program); eprintln!("{diagnostics:?}"); 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..70df0973 100644 --- a/components/salsa-2022-macros/src/salsa_struct.rs +++ b/components/salsa-2022-macros/src/salsa_struct.rs @@ -25,8 +25,10 @@ //! * data method `impl Foo { fn data(&self, db: &dyn crate::Db) -> FooData { FooData { f: self.f(db), ... } } }` //! * this could be optimized, particularly for interned fields +use syn::spanned::Spanned; use heck::ToUpperCamelCase; -use proc_macro2::{Ident, Literal, Span}; +use proc_macro2::{Ident, Literal, Span, TokenStream}; +use quote::ToTokens; use crate::{configuration, options::Options}; @@ -293,6 +295,55 @@ 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(::core::marker::PhantomData, ::core::marker::PhantomData); + + impl, Db: ?Sized> Test { + fn salsa_debug<'a, 'b: 'a>(a: &'a T, db: &'b Db) -> ::salsa::debug::DebugWith<'a, Db> { + a.debug(db) + } + } + + trait Fallback { + fn salsa_debug<'a, 'b>(a: &'a T, _db: &'b Db) -> &'a dyn ::core::fmt::Debug { + a + } + } + + impl Fallback for Everything {} + + &Test::<#field_ty, #db_type>::salsa_debug(&self.#field_getter(db), db) + }) + } + }).collect::>(); + + 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)* }) } From e3f09fa122d52a878476f634d441f3afff6ffe47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Tue, 23 Aug 2022 23:33:14 +0200 Subject: [PATCH 2/8] Update book --- book/src/tutorial/debug.md | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) 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 From fdc8018234683900348f26e1a9fee499d72bbdbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Tue, 23 Aug 2022 23:38:19 +0200 Subject: [PATCH 3/8] Cleanup --- components/salsa-2022-macros/src/salsa_struct.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/salsa-2022-macros/src/salsa_struct.rs b/components/salsa-2022-macros/src/salsa_struct.rs index 70df0973..fbabd2c9 100644 --- a/components/salsa-2022-macros/src/salsa_struct.rs +++ b/components/salsa-2022-macros/src/salsa_struct.rs @@ -28,7 +28,6 @@ use syn::spanned::Spanned; use heck::ToUpperCamelCase; use proc_macro2::{Ident, Literal, Span, TokenStream}; -use quote::ToTokens; use crate::{configuration, options::Options}; @@ -313,6 +312,7 @@ impl SalsaStruct { .field(#field_name_string, { struct Test(::core::marker::PhantomData, ::core::marker::PhantomData); + #[allow(dead_code)] impl, Db: ?Sized> Test { fn salsa_debug<'a, 'b: 'a>(a: &'a T, db: &'b Db) -> ::salsa::debug::DebugWith<'a, Db> { a.debug(db) From 8fc6dc82ac428a18d7b078306f0f82fdd985f883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Tue, 23 Aug 2022 23:39:51 +0200 Subject: [PATCH 4/8] More cleanup --- calc-example/calc/src/main.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/calc-example/calc/src/main.rs b/calc-example/calc/src/main.rs index ee5b3083..27495343 100644 --- a/calc-example/calc/src/main.rs +++ b/calc-example/calc/src/main.rs @@ -1,5 +1,4 @@ use ir::{Diagnostics, SourceProgram}; -use salsa::DebugWithDb; // ANCHOR: jar_struct #[salsa::jar(db = Db)] @@ -33,17 +32,9 @@ mod ir; mod parser; mod type_check; -const PROGRAM: &str = r" -fn area_rectangle(w, h) = w * h -fn area_circle(r) = 3.14 * r * r -print area_rectangle(3, 4) -print area_circle(1) -print 11 * 2 -"; - pub fn main() { let mut db = db::Database::default(); - let source_program = SourceProgram::new(&mut db, PROGRAM.to_string()); + let source_program = SourceProgram::new(&mut db, String::new()); compile::compile(&db, source_program); let diagnostics = compile::compile::accumulated::(&db, source_program); eprintln!("{diagnostics:?}"); From fbb219a52757e6ba40014b073c160446fcd162e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Tue, 23 Aug 2022 23:54:18 +0200 Subject: [PATCH 5/8] Add a test --- salsa-2022-tests/tests/debug.rs | 57 +++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 salsa-2022-tests/tests/debug.rs diff --git a/salsa-2022-tests/tests/debug.rs b/salsa-2022-tests/tests/debug.rs new file mode 100644 index 00000000..f41046c0 --- /dev/null +++ b/salsa-2022-tests/tests/debug.rs @@ -0,0 +1,57 @@ +//! Test that a `tracked` fn on a `salsa::input` +//! compiles and executes successfully. + +use expect_test::expect; +use test_log::test; +use salsa::DebugWithDb; + +#[salsa::jar(db = Db)] +struct Jar(MyInput, ComplexStruct); + +trait Db: salsa::DbWithJar {} + +#[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, +} + +impl salsa::Database for Database { + fn salsa_runtime(&self) -> &salsa::Runtime { + self.storage.runtime() + } +} + +impl Db for Database {} + + +#[test] +fn execute() { + 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); +} From 0c3084d0091b8af221dd2d7614a1bed45c646041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Wed, 24 Aug 2022 00:01:44 +0200 Subject: [PATCH 6/8] Test cleanup --- salsa-2022-tests/tests/debug.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/salsa-2022-tests/tests/debug.rs b/salsa-2022-tests/tests/debug.rs index f41046c0..f0b4d1f7 100644 --- a/salsa-2022-tests/tests/debug.rs +++ b/salsa-2022-tests/tests/debug.rs @@ -1,8 +1,6 @@ -//! Test that a `tracked` fn on a `salsa::input` -//! compiles and executes successfully. +//! Test that `DeriveWithDb` is correctly derived. use expect_test::expect; -use test_log::test; use salsa::DebugWithDb; #[salsa::jar(db = Db)] @@ -40,9 +38,8 @@ impl salsa::Database for Database { impl Db for Database {} - #[test] -fn execute() { +fn input() { let mut db = Database::default(); let input = MyInput::new(&mut db, 22); @@ -52,6 +49,8 @@ fn execute() { 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" } }"#]]; + 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); } From 8e27ecd0986b779373e7d4a8ce19bffebbcaaa94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Wed, 24 Aug 2022 00:10:07 +0200 Subject: [PATCH 7/8] Fix formatting --- components/salsa-2022-macros/src/salsa_struct.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/salsa-2022-macros/src/salsa_struct.rs b/components/salsa-2022-macros/src/salsa_struct.rs index fbabd2c9..00ed928b 100644 --- a/components/salsa-2022-macros/src/salsa_struct.rs +++ b/components/salsa-2022-macros/src/salsa_struct.rs @@ -25,9 +25,9 @@ //! * data method `impl Foo { fn data(&self, db: &dyn crate::Db) -> FooData { FooData { f: self.f(db), ... } } }` //! * this could be optimized, particularly for interned fields -use syn::spanned::Spanned; use heck::ToUpperCamelCase; use proc_macro2::{Ident, Literal, Span, TokenStream}; +use syn::spanned::Spanned; use crate::{configuration, options::Options}; From fe1c4b42e247c324e242d93943b365abe3a859cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82uszak?= Date: Wed, 24 Aug 2022 11:24:31 +0200 Subject: [PATCH 8/8] Fix parser test --- calc-example/calc/src/parser.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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::(&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(