From dbe8561721537f840e3c740ce608533c552eefbb Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 27 Aug 2022 06:22:04 +0800 Subject: [PATCH 1/6] add some docs for `specify` --- components/salsa-2022/src/function/specify.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/components/salsa-2022/src/function/specify.rs b/components/salsa-2022/src/function/specify.rs index d76aa0fc..1200fb24 100644 --- a/components/salsa-2022/src/function/specify.rs +++ b/components/salsa-2022/src/function/specify.rs @@ -33,6 +33,18 @@ where None => panic!("can only use `set` with an active query"), }; + // `specify` only works if the key is a tracked struct created in the current query. + // + // The reason is this. We want to ensure that the same result is reached regardless of + // the "path" that the user takes through the execution graph. + // If you permit values to be specified from other queries, you can have a situation like this: + // * Q0 creates the tracked struct T0 + // * Q1 specifies the value for F(T0) + // * Q2 invokes F(T0) + // * Q3 invokes Q1 and then Q2 + // * Q4 invokes Q2 and then Q1 + // + // Now, if We invoke Q3 first, We get one result for Q2, but if We invoke Q4 first, We get a different value. That's no good. let database_key_index = key.database_key_index(db); if !runtime.is_output_of_active_query(database_key_index) { panic!("can only use `set` on entities created during current query"); From 8e53905b876e9fff8bb84e45f207fd532f37f7ed Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 27 Aug 2022 06:42:06 +0800 Subject: [PATCH 2/6] add a test for specify `specify` only works if the key is created in the current query. --- ...the-key-is-created-in-the-current-query.rs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 salsa-2022-tests/tests/specify-only-works-if-the-key-is-created-in-the-current-query.rs diff --git a/salsa-2022-tests/tests/specify-only-works-if-the-key-is-created-in-the-current-query.rs b/salsa-2022-tests/tests/specify-only-works-if-the-key-is-created-in-the-current-query.rs new file mode 100644 index 00000000..3c51e1e7 --- /dev/null +++ b/salsa-2022-tests/tests/specify-only-works-if-the-key-is-created-in-the-current-query.rs @@ -0,0 +1,57 @@ +//! Test that `specify` only works if the key is a tracked struct created in the current query. +//! compilation succeeds but execution panics +#![allow(warnings)] + +#[salsa::jar(db = Db)] +struct Jar(MyInput, MyTracked, tracked_fn, tracked_fn_extra, tracked_struct_created_in_another_query); + +trait Db: salsa::DbWithJar {} + +#[salsa::input(jar = Jar)] +struct MyInput { + field: u32, +} + +#[salsa::tracked(jar = Jar)] +struct MyTracked { + field: u32, +} + +#[salsa::tracked(jar = Jar)] +fn tracked_struct_created_in_another_query(db: &dyn Db, input: MyInput) -> MyTracked { + MyTracked::new(db, input.field(db) * 2) +} + +#[salsa::tracked(jar = Jar)] +fn tracked_fn(db: &dyn Db, input: MyInput) -> MyTracked { + let t = tracked_struct_created_in_another_query(db, input); + if input.field(db) != 0 { + tracked_fn_extra::specify(db, t, 2222); + } + t +} + +#[salsa::tracked(jar = Jar, specify)] +fn tracked_fn_extra(_db: &dyn Db, _input: MyTracked) -> u32 { + 0 +} + +#[salsa::db(Jar)] +#[derive(Default)] +struct Database { + storage: salsa::Storage, +} + +impl salsa::Database for Database {} + +impl Db for Database {} + +#[test] +#[should_panic] +fn execute_when_specified() { + let mut db = Database::default(); + let input = MyInput::new(&mut db, 22); + let tracked = tracked_fn(&db, input); +} + + From 82a6251ebc48eb9ce43ec69affeb8ac1a31b289b Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 27 Aug 2022 07:05:35 +0800 Subject: [PATCH 3/6] add a test for specify `specify` does not work if the key is a `salsa::input` --- ...es-not-work-if-the-key-is-a-salsa-input.rs | 35 +++++++++++++++++++ ...ot-work-if-the-key-is-a-salsa-input.stderr | 14 ++++++++ 2 files changed, 49 insertions(+) create mode 100644 salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs create mode 100644 salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr diff --git a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs new file mode 100644 index 00000000..10d9f727 --- /dev/null +++ b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs @@ -0,0 +1,35 @@ +//! Test that `specify` does not work if the key is a `salsa::input` +//! compilation fails +#![allow(warnings)] + +#[salsa::jar(db = Db)] +struct Jar(MyInput, MyTracked, tracked_fn); + +trait Db: salsa::DbWithJar {} + +#[salsa::input(jar = Jar)] +struct MyInput { + field: u32, +} + +#[salsa::tracked(jar = Jar)] +struct MyTracked { + field: u32, +} + +#[salsa::tracked(jar = Jar, specify)] +fn tracked_fn(db: &dyn Db, input: MyInput) -> MyTracked { + MyTracked::new(db, input.field(db) * 2) +} + +#[salsa::db(Jar)] +#[derive(Default)] +struct Database { + storage: salsa::Storage, +} + +impl salsa::Database for Database {} + +impl Db for Database {} + +fn main() {} \ No newline at end of file diff --git a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr new file mode 100644 index 00000000..5c52c233 --- /dev/null +++ b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.stderr @@ -0,0 +1,14 @@ +error[E0277]: the trait bound `MyInput: TrackedStructInDb` is not satisfied + --> tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs:21:28 + | +20 | #[salsa::tracked(jar = Jar, specify)] + | ------------------------------------- required by a bound introduced by this call +21 | fn tracked_fn(db: &dyn Db, input: MyInput) -> MyTracked { + | ^^^^^ the trait `TrackedStructInDb` is not implemented for `MyInput` + | + = help: the trait `TrackedStructInDb` is implemented for `MyTracked` +note: required by a bound in `function::specify::>::specify_and_record` + --> $WORKSPACE/components/salsa-2022/src/function/specify.rs + | + | C::Key: TrackedStructInDb>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `function::specify::>::specify_and_record` From f7519acb137009b1734effbbdf2618c0120e0e06 Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 27 Aug 2022 07:51:35 +0800 Subject: [PATCH 4/6] add a test for specify `specify` does not work if the key is a `salsa::interned` --- components/salsa-2022/src/function/specify.rs | 6 ++-- ...es-not-work-if-the-key-is-a-salsa-input.rs | 2 +- ...not-work-if-the-key-is-a-salsa-interned.rs | 35 +++++++++++++++++++ ...work-if-the-key-is-a-salsa-interned.stderr | 14 ++++++++ ...the-key-is-created-in-the-current-query.rs | 12 ++++--- 5 files changed, 61 insertions(+), 8 deletions(-) create mode 100644 salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.rs create mode 100644 salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr diff --git a/components/salsa-2022/src/function/specify.rs b/components/salsa-2022/src/function/specify.rs index 1200fb24..dd6b3382 100644 --- a/components/salsa-2022/src/function/specify.rs +++ b/components/salsa-2022/src/function/specify.rs @@ -34,9 +34,9 @@ where }; // `specify` only works if the key is a tracked struct created in the current query. - // - // The reason is this. We want to ensure that the same result is reached regardless of - // the "path" that the user takes through the execution graph. + // + // The reason is this. We want to ensure that the same result is reached regardless of + // the "path" that the user takes through the execution graph. // If you permit values to be specified from other queries, you can have a situation like this: // * Q0 creates the tracked struct T0 // * Q1 specifies the value for F(T0) diff --git a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs index 10d9f727..ac5811a3 100644 --- a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs +++ b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-input.rs @@ -32,4 +32,4 @@ impl salsa::Database for Database {} impl Db for Database {} -fn main() {} \ No newline at end of file +fn main() {} diff --git a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.rs b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.rs new file mode 100644 index 00000000..8c4cca31 --- /dev/null +++ b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.rs @@ -0,0 +1,35 @@ +//! Test that `specify` does not work if the key is a `salsa::interned` +//! compilation fails +#![allow(warnings)] + +#[salsa::jar(db = Db)] +struct Jar(MyInterned, MyTracked, tracked_fn); + +trait Db: salsa::DbWithJar {} + +#[salsa::interned(jar = Jar)] +struct MyInterned { + field: u32, +} + +#[salsa::tracked(jar = Jar)] +struct MyTracked { + field: u32, +} + +#[salsa::tracked(jar = Jar, specify)] +fn tracked_fn(db: &dyn Db, input: MyInterned) -> MyTracked { + MyTracked::new(db, input.field(db) * 2) +} + +#[salsa::db(Jar)] +#[derive(Default)] +struct Database { + storage: salsa::Storage, +} + +impl salsa::Database for Database {} + +impl Db for Database {} + +fn main() {} diff --git a/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr new file mode 100644 index 00000000..e00f9b92 --- /dev/null +++ b/salsa-2022-tests/tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.stderr @@ -0,0 +1,14 @@ +error[E0277]: the trait bound `MyInterned: TrackedStructInDb` is not satisfied + --> tests/compile-fail/specify-does-not-work-if-the-key-is-a-salsa-interned.rs:21:28 + | +20 | #[salsa::tracked(jar = Jar, specify)] + | ------------------------------------- required by a bound introduced by this call +21 | fn tracked_fn(db: &dyn Db, input: MyInterned) -> MyTracked { + | ^^^^^ the trait `TrackedStructInDb` is not implemented for `MyInterned` + | + = help: the trait `TrackedStructInDb` is implemented for `MyTracked` +note: required by a bound in `function::specify::>::specify_and_record` + --> $WORKSPACE/components/salsa-2022/src/function/specify.rs + | + | C::Key: TrackedStructInDb>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `function::specify::>::specify_and_record` diff --git a/salsa-2022-tests/tests/specify-only-works-if-the-key-is-created-in-the-current-query.rs b/salsa-2022-tests/tests/specify-only-works-if-the-key-is-created-in-the-current-query.rs index 3c51e1e7..fcebf7e6 100644 --- a/salsa-2022-tests/tests/specify-only-works-if-the-key-is-created-in-the-current-query.rs +++ b/salsa-2022-tests/tests/specify-only-works-if-the-key-is-created-in-the-current-query.rs @@ -3,7 +3,13 @@ #![allow(warnings)] #[salsa::jar(db = Db)] -struct Jar(MyInput, MyTracked, tracked_fn, tracked_fn_extra, tracked_struct_created_in_another_query); +struct Jar( + MyInput, + MyTracked, + tracked_fn, + tracked_fn_extra, + tracked_struct_created_in_another_query, +); trait Db: salsa::DbWithJar {} @@ -19,7 +25,7 @@ struct MyTracked { #[salsa::tracked(jar = Jar)] fn tracked_struct_created_in_another_query(db: &dyn Db, input: MyInput) -> MyTracked { - MyTracked::new(db, input.field(db) * 2) + MyTracked::new(db, input.field(db) * 2) } #[salsa::tracked(jar = Jar)] @@ -53,5 +59,3 @@ fn execute_when_specified() { let input = MyInput::new(&mut db, 22); let tracked = tracked_fn(&db, input); } - - From f46f2ac202b05d5e25b8c20981267223f092c8bc Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 27 Aug 2022 07:52:18 +0800 Subject: [PATCH 5/6] fix typos --- components/salsa-2022/src/function/specify.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/salsa-2022/src/function/specify.rs b/components/salsa-2022/src/function/specify.rs index dd6b3382..8c0fe19c 100644 --- a/components/salsa-2022/src/function/specify.rs +++ b/components/salsa-2022/src/function/specify.rs @@ -30,7 +30,7 @@ where let (active_query_key, current_deps) = match runtime.active_query() { Some(v) => v, - None => panic!("can only use `set` with an active query"), + None => panic!("can only use `specify` with an active query"), }; // `specify` only works if the key is a tracked struct created in the current query. @@ -47,7 +47,7 @@ where // Now, if We invoke Q3 first, We get one result for Q2, but if We invoke Q4 first, We get a different value. That's no good. let database_key_index = key.database_key_index(db); if !runtime.is_output_of_active_query(database_key_index) { - panic!("can only use `set` on entities created during current query"); + panic!("can only use `specfiy` on entities created during current query"); } // Subtle: we treat the "input" to a set query as if it were From cece3d840a887cd5ce891646007263577c936abc Mon Sep 17 00:00:00 2001 From: XFFXFF <1247714429@qq.com> Date: Sat, 27 Aug 2022 09:05:51 +0800 Subject: [PATCH 6/6] run compile_fail test only on stable --- salsa-2022-tests/Cargo.toml | 1 + salsa-2022-tests/tests/compile_fail.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/salsa-2022-tests/Cargo.toml b/salsa-2022-tests/Cargo.toml index 6fb25b97..6ac555cd 100644 --- a/salsa-2022-tests/Cargo.toml +++ b/salsa-2022-tests/Cargo.toml @@ -12,3 +12,4 @@ parking_lot = "0.12.1" test-log = "0.2.11" env_logger = "*" trybuild = "1.0" +rustversion = "1.0" diff --git a/salsa-2022-tests/tests/compile_fail.rs b/salsa-2022-tests/tests/compile_fail.rs index db50dfda..116dd9f8 100644 --- a/salsa-2022-tests/tests/compile_fail.rs +++ b/salsa-2022-tests/tests/compile_fail.rs @@ -1,3 +1,4 @@ +#[rustversion::stable] #[test] fn compile_fail() { let t = trybuild::TestCases::new();