Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/function/maybe_changed_after.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,23 @@ where
return Some(if old_memo.revisions.changed_at > revision {
VerifyResult::changed()
} else {
// Returns unchanged but propagates the accumulated values
deep_verify
// Propagate accumulated values from inputs *and* from this memo itself.
// Without the own-accumulated check, a caller querying accumulated values
// would see `Empty` and skip traversing into this subtree even though
// this memo directly pushed accumulated values.
VerifyResult::unchanged_with_accumulated(
#[cfg(feature = "accumulator")]
{
let VerifyResult::Unchanged { accumulated } = deep_verify else {
unreachable!()
};
if old_memo.revisions.accumulated().is_some() {
InputAccumulatedValues::Any
} else {
accumulated
}
},
)
});
}

Expand Down
74 changes: 74 additions & 0 deletions tests/accumulate-behind-tracked-fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#![cfg(all(feature = "inventory", feature = "accumulator"))]

//! Regression test for https://github.com/salsa-rs/salsa/issues/923.
//!
//! Accumulated values were silently dropped when a tracked fn that calls
//! another tracked fn (which does the actual accumulating) is re-used
//! without re-executing (its inputs haven't changed).

use expect_test::expect;
use salsa::{Accumulator, Setter};
use test_log::test;

#[salsa::input(debug)]
struct List {
value: u32,
next: Option<List>,
}

#[allow(unused)]
#[salsa::accumulator]
#[derive(Copy, Clone, Debug)]
struct Integers(u32);

/// Outer tracked fn: iterates the list and delegates accumulation to `compute_single`.
#[salsa::tracked]
fn compute(db: &dyn salsa::Database, input: List) {
compute_single(db, input);
if let Some(next) = input.next(db) {
compute(db, next);
}
}

/// Inner tracked fn: performs the actual accumulation.
#[salsa::tracked]
fn compute_single(db: &dyn salsa::Database, input: List) {
Integers(input.value(db)).accumulate(db);
}

#[test]
fn accumulated_values_not_lost_after_partial_reuse() {
let mut db = salsa::DatabaseImpl::new();

let l0 = List::new(&db, 1, None);
let l1 = List::new(&db, 10, Some(l0));

compute(&db, l1);
expect![[r#"
[
Integers(
10,
),
Integers(
1,
),
]
"#]]
.assert_debug_eq(&compute::accumulated::<Integers>(&db, l1));

// Mutate l1; l0 is unchanged so compute(l0) / compute_single(l0) should be reused.
// Reused memos must still report their accumulated values so callers don't skip them.
l1.set_value(&mut db).to(11);
compute(&db, l1);
expect![[r#"
[
Integers(
11,
),
Integers(
1,
),
]
"#]]
.assert_debug_eq(&compute::accumulated::<Integers>(&db, l1));
}
Loading