428: salsa-2022: fix hanging cancellations due to cvar not being notified r=nikomatsakis a=manapointer
This PR fixes an issue with the `cvar` condvar field of `Shared<DB>` not being notified when `Arc<Shared<DB>>`s were getting dropped.
Previously, the condvar was being notified here:
```rust
impl<DB> Drop for Shared<DB>
where
DB: HasJars,
{
fn drop(&mut self) {
self.cvar.notify_all();
}
}
```
However, because this is implemented on `Shared<DB>`, the `drop` code only ran after all `Arc<Shared<DB>>`s (including that of the actual database and not just its snapshots) had dropped first, even though the intention was for the `drop` code to run when each individual `Arc<Shared<DB>>` was dropped so that the condvar would be notified each time.
To fix this, I've modified the `Shared<DB>` struct to instead hold `Arc`s to both the jars and the condvar, and `Storage` now holds `Shared<DB>` directly rather than `Arc<Shared<DB>>`. When `Shared<DB>` is dropped, it first drops its `Arc` for the jars, and then notifies the condvar.
## Note
On my local branch I have a test case for this functionality, although it relied on timing using `std:🧵:sleep`. I figured this was less than ideal, so I decided not to include it in this PR - I'd love to get feedback on a better way to test this!
Co-authored-by: manapointer <manapointer@gmail.com>
Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
423: salsa 2022: fix input macro set_* being off by one if id field present r=XFFXFF a=jhgg
This PR fixes an issue with code generation for `#[salsa::input]` struct's `set_` methods.
Consider the following code:
```rust
#[salsa::input(jar = Jar)]
struct BuggedInput {
#[id]
id: u32,
other: String,
}
```
This will expand to:
```rust
#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Debug)]
struct BuggedInput(salsa::Id);
impl BuggedInput {
// snip...
fn set_other<'db>(
self,
__db: &'db mut <Jar as salsa:🫙:Jar<'_>>::DynDb,
) -> salsa::setter::Setter<'db, BuggedInput, u32> {
// ^^^ wrong type (should be `String`)
let (__jar, __runtime) = <_ as salsa::storage::HasJar<Jar>>::jar_mut(__db);
let __ingredients =
<Jar as salsa::storage::HasIngredientsFor<BuggedInput>>::ingredient_mut(__jar);
salsa::setter::Setter::new(__runtime, self, &mut __ingredients.0)
// ^ wrong index (should be `1`)
}
}
```
Here we can see that the generated `set_other` impl is improperly setting the `id` field. This bug is caused because the filtering of the id fields in `InputStruct::all_set_field_names` causes a mismatch in `InputStruct::input_inherent_impl` when zipped with the field indices and types.
This PR changes `all_set_field_names` to return an `Option<&syn::Ident>` where the None is provided when a setter should not be generated, thus not causing index mismatches because no filtering occurs. Instead, we filter inside of `input_inherent_impl` after all the zips have been applied to the iterator.
After this PR, the code generated is now correct:
```rust
#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Debug)]
struct BuggedInput(salsa::Id);
impl BuggedInput {
// snip...
fn set_other<'db>(
self,
__db: &'db mut <Jar as salsa:🫙:Jar<'_>>::DynDb,
) -> salsa::setter::Setter<'db, BuggedInput, String> {
let (__jar, __runtime) = <_ as salsa::storage::HasJar<Jar>>::jar_mut(__db);
let __ingredients =
<Jar as salsa::storage::HasIngredientsFor<BuggedInput>>::ingredient_mut(__jar);
salsa::setter::Setter::new(__runtime, self, &mut __ingredients.1)
}
}
```
Co-authored-by: Jake Heinz <jh@discordapp.com>
This may have been true in the past, but both the Salsa 2022 and original tests demonstrate recovery from a cycle where only a subset of the cycle queries have recovery functions.
412: Fix race condition in interned data r=nikomatsakis a=Skepfyr
In some scenarios the `InternedIngredient` could return two different ids for the same data if called in parallel.
Co-authored-by: Jack Rickard <jack.rickard@outlook.com>
416: Correct docs to refer to `#[salsa::cycle]` r=nikomatsakis a=ssbr
Judging by the examples, `recover` is an outdated name.
E.g. https://github.com/salsa-rs/salsa/blob/master/tests/cycles.rs
(I can't really test this on version 0.16, so I'm just going by the examples.)
Co-authored-by: Devin Jeanpierre <jeanpierreda@google.com>
404: On-demand input r=nikomatsakis a=Skepfyr
Fixes#394
This is not yet done, but I'd like to get some feedback on the approach.
I think these things are remaining:
- [x] Clean up some now incorrect comments in `input_field.rs`
- [x] ~Update calc-example to use on-demand inputs, and remove the lazy-input example (this is quite fun as you end up with an interactive calculator)~
- [x] Update the book
Co-authored-by: Jack Rickard <jack.rickard@outlook.com>
This adds initial support for on-demand inputs by allowing new inputs to
be created with only a shared reference to the database. This allows
creating new inputs during a revision and therefore from inside tracked
functions.
