diff --git a/components/salsa-2022/src/function/specify.rs b/components/salsa-2022/src/function/specify.rs index d76aa0fc..8c0fe19c 100644 --- a/components/salsa-2022/src/function/specify.rs +++ b/components/salsa-2022/src/function/specify.rs @@ -30,12 +30,24 @@ 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. + // + // 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"); + panic!("can only use `specfiy` on entities created during current query"); } // Subtle: we treat the "input" to a set query as if it were 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/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..ac5811a3 --- /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() {} 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` 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/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(); 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..fcebf7e6 --- /dev/null +++ b/salsa-2022-tests/tests/specify-only-works-if-the-key-is-created-in-the-current-query.rs @@ -0,0 +1,61 @@ +//! 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); +}