assert failed components/salsa-2022/src/function/specify.rs:130:48 about salsa HOT 5 OPEN

@Chronostasys Would it be possible for you to share a minimum, reproducible code with us? This would help us to better understand the issue

from salsa.

@Chronostasys Would it be possible for you to share a minimum, reproducible code with us? This would help us to better understand the issue

Not yet. I've tried it yesterday, as the real-world case is really complecated, it may take more time for me to find out how to reproduce it on a sample project.

from salsa.

After moving the cycle handler to another participant function in the cycle, the panic position changed:

thread 'main' panicked at 'expected a query assigned by `compile_dry_file(11)`, not `Program_emit(11)`'

and during recover, the participants be like:

["compile_dry_file(11)", "Program_emit(11)", "compile_dry_file(12)", "Program_emit(12)", "compile_dry_file(15)", "Program_emit(15)"]

Note that in my case, panic recovery logic is triggered three times in a single revision.

from salsa.

@Chronostasys Would it be possible for you to share a minimum, reproducible code with us? This would help us to better understand the issue

@xffxff
https://github.com/Chronostasys/salsa-panic-example

from salsa.

A simpler example to reproduce this issue can be found at https://gist.github.com/xffxff/47dbd41a078c8e705aa35d7a9d6f1303. Here's a brief version:

#[salsa::tracked(jar = Jar)]
fn memoized(db: &dyn Db, input: MyInput) -> u32 {
    memoized_a(db, MyTracked::new(db, input.field(db)))
}

#[salsa::tracked(jar = Jar)]
fn memoized_a(db: &dyn Db, tracked: MyTracked) -> u32 {
    MyTracked::new(db, 0);
    return memoized_b(db, tracked);
}

fn recovery_fn(db: &dyn Db, cycle: &salsa::Cycle, input: MyTracked) -> u32 {
    0
}

#[salsa::tracked(jar = Jar, recovery_fn=recovery_fn)]
fn memoized_b(db: &dyn Db, tracked: MyTracked) -> u32 {
    tracked.field(db) + memoized_a(db, tracked)
}

This code will fail with the following error:

thread 'cycle_memoized' panicked at 'assertion failed: `(left == right)`
  left: `DatabaseKeyIndex { ingredient_index: IngredientIndex(5), key_index: Id { value: 1 } }`,
 right: `DatabaseKeyIndex { ingredient_index: IngredientIndex(6), key_index: Id { value: 1 } }`

The issue is Salsa thinks the dependencies (inputs_outputs) of memoized_b(MyTracked { [salsa id]: 0 }) to be:

QueryEdges {
    input_outputs: [
        (
            Input,
            MyTracked(),
        ),
        (
            Output,
            MyTracked(1),
        ),
        (
            Output,
            field(MyTracked { [salsa id]: 1 }),
        ),
        (
            Input,
            field(MyTracked { [salsa id]: 0 }),
        ),
    ],
},

The panic occurs when the query memoized_b(MyTracked { [salsa id]: 0 }) tries to mark the output MyTracked(1) as valid. However, the output MyTracked(1) isn't created by memoized_b(MyTracked { [salsa id]: 0 }), but by memoized_a(MyTracked { [salsa id]: 0 }). The relevant code is:

The dependencies of memoized_b(MyTracked { [salsa id]: 0 }) orginates from the following code:

When a cycle is detected, the query with recovery_fn, memoized_b(MyTracked { [salsa id]: 0 }) in this case, takes all the inputs and outputs from the cycle participants. However, as the function name take_inputs_from suggests, it might be more appropriate to only take the inputs from the cycle participants. (This might be a bug introduced by #413)

pub(crate) fn take_inputs_from(&mut self, cycle_query: &ActiveQuery) {
    self.changed_at = cycle_query.changed_at;
    self.durability = cycle_query.durability;
    self.input_outputs = cycle_query.input_outputs.iter().filter(|(edge_kind, database_key_index)| {
        *edge_kind == EdgeKind::Input
    })
    .copied().collect::<FxIndexSet<(EdgeKind, DependencyIndex)>>();
}

I'm uncertain if this is the correct solution. Furthermore, I'm unclear why the query withrecovery_fn in a cycle needs to take all the inputs from the cycle participants.

from salsa.