Instead of grabbing the arc, just pass back an `&mut Runtime`.
The eventual goal is to get rid of the lock on the `set` pathway
altogether, but one step at a time.
288: Make with_incremented_revision take FnOnce r=nikomatsakis a=mheiber
Removes a bug vector, since this function would
panic if the closure is used more than once.
iuc, the code opted out of the compiler's checks
before via a clever `.take()`.
Another change is to take the closure by value
rather than by reference. The monomorphization
seems harmless, since `with_incremented_revision`
is only called from two places.
289: Remove ': salsa::Database' bound from two examples r=nikomatsakis a=mheiber
Two examples had a superfluous bound
': salsa::Database' that wasn't present
in the `compiler` example and doesn't seem to be needed.
The `query_group` macro adds this bound
automatically.
This change can lead to a trailing `+` in
the bounds list. I verified this is OK by
running the examples and verifying that the production
is allowed
[per the Rust Reference](https://doc.rust-lang.org/reference/trait-bounds.html)
Co-authored-by: Maxwell Elliot Heiber <mheiber@fb.com>
The function is morally an FnOnce, since if
called multiple times the function will panic.
iuc, the code opted out of the compiler's checks
before via a clever `.take()`.
Another change is to take the closure by value
rather than by reference. The monomorphization
seems harmless, since `with_incremented_revision`
is only called from two places.
We still record the same dependencies (or else the tests fail,
so +1 for test coverage).
This has the immediate advantage that we don't invoke the fallback
function twice for the repeated node in the cycle.
Also, fix a bug where revalidating cycles could lead to a
CycleParticipant error that is not caught (added a test for it).
It turns out this is necessary, as this test reveals!
If we don't panic, you might encounter further cycles
that aren't supposed to have executed. (Prior to these changes,
this test was panicking from the second cycle.)
The `Cancelled` struct now reflects multiple potential reasons
for a query execution to be cancelled, including unexpected cycles.
It also gives information about the participants that lacked
recovery information.
This had two unexpected consequences, one unfortunate, one "medium":
* All `salsa::Database` must be `'static`. This falls out from
`Q::DynDb` not having access to any lifetimes, but also the defaulting
rules for `dyn QueryGroup` that make it `dyn QueryGroup + 'static`. We
don't really support generic databases anyway yet so this isn't a big
deal, and we can add workarounds later (ideally via GATs).
* It is now statically impossible to invoke `snapshot` from a query,
and so we don't need to test that it panics. This is because the
signature of `snapshot` returns a `Snapshot<Self>` and that is not
accessible to a `dyn QueryGroup` type. Similarly, invoking
`Runtime::snapshot` directly is not possible becaues it is
crate-private. So I removed the test. This seems ok, but eventually I
would like to expose ways for queries to do parallel
execution (matklad and I had talked about a "speculation" primitive
for enabling that).
* This commit is 99% boilerplate I did with search-and-replace. I also
rolled in a few other changes I might have preferred to factor out,
most notably removing the `GetQueryTable` plumbing trait in favor of
free-methods, but it was awkward to factor them out and get all the
generics right (so much simpler in this version).
Quickest POC I could create to get some potentially cyclic queries to
not panic and instead return a result I could act on. (gluon's module
importing need to error on cycles).
```
// Causes `db.query()` to actually return `Result<V, CycleError>`
fn query(&self, key: K, key2: K2) -> V;
```
A proper implementation of this would likely return
`Result<V, CycleError<(K, K2)>>` or maybe larger changes are needed.
cc #6
We now track the last revision in which constants were modified. When
we see a constant query result, we record the current revision as
well. Then later we can check if the result is "still" constant. This
lets us cut out a lot of intermediate work.
