Skip to content

Use explicit discriminants for QueryOriginKind for better comparisons #913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 12, 2025
Merged
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
2 changes: 1 addition & 1 deletion components/salsa-macro-rules/src/setup_input_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ macro_rules! setup_input_struct {

$(
#[must_use]
$field_setter_vis fn $field_setter_id<'db, $Db>(self, db: &'db mut $Db) -> impl salsa::Setter<FieldTy = $field_ty> + 'db
$field_setter_vis fn $field_setter_id<'db, $Db>(self, db: &'db mut $Db) -> impl salsa::Setter<FieldTy = $field_ty> + use<'db, $Db>
where
// FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
$Db: ?Sized + $zalsa::Database,
Expand Down
13 changes: 7 additions & 6 deletions src/function/diff_outputs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::function::memo::Memo;
use crate::function::{Configuration, IngredientImpl};
use crate::hash::FxIndexSet;
use crate::zalsa::Zalsa;
use crate::zalsa_local::QueryRevisions;
use crate::zalsa_local::{output_edges, QueryOriginRef, QueryRevisions};
use crate::{DatabaseKeyIndex, Event, EventKind, Id};

impl<C> IngredientImpl<C>
Expand All @@ -21,15 +21,16 @@ where
old_memo: &Memo<C::Output<'_>>,
revisions: &mut QueryRevisions,
) {
let (QueryOriginRef::Derived(edges) | QueryOriginRef::DerivedUntracked(edges)) =
old_memo.revisions.origin.as_ref()
else {
return;
};
// Iterate over the outputs of the `old_memo` and put them into a hashset
//
// Ignore key_generation here, because we use the same tracked struct allocation for
// all generations with the same key_index and can't report it as stale
let mut old_outputs: FxIndexSet<_> = old_memo
.revisions
.origin
.as_ref()
.outputs()
let mut old_outputs: FxIndexSet<_> = output_edges(edges)
.map(|a| (a.ingredient_index(), a.key_index().index()))
.collect();

Expand Down
2 changes: 1 addition & 1 deletion src/function/memo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ impl<V> Memo<V> {
}
}

pub(super) fn tracing_debug(&self) -> impl std::fmt::Debug + '_ {
pub(super) fn tracing_debug(&self) -> impl std::fmt::Debug + use<'_, V> {
struct TracingDebug<'a, T> {
memo: &'a Memo<T>,
}
Expand Down
49 changes: 28 additions & 21 deletions src/zalsa_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,75 +520,82 @@ impl QueryRevisions {
/// Tracks the way that a memoized value for a query was created.
///
/// This is a read-only reference to a `PackedQueryOrigin`.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy)]
#[repr(u8)]
pub enum QueryOriginRef<'a> {
/// The value was assigned as the output of another query (e.g., using `specify`).
/// The `DatabaseKeyIndex` is the identity of the assigning query.
Assigned(DatabaseKeyIndex),
Assigned(DatabaseKeyIndex) = QueryOriginKind::Assigned as u8,

/// The value was derived by executing a function
/// and we were able to track ALL of that function's inputs.
/// Those inputs are described in [`QueryEdges`].
Derived(&'a [QueryEdge]),
Derived(&'a [QueryEdge]) = QueryOriginKind::Derived as u8,

/// The value was derived by executing a function
/// but that function also reported that it read untracked inputs.
/// The [`QueryEdges`] argument contains a listing of all the inputs we saw
/// (but we know there were more).
DerivedUntracked(&'a [QueryEdge]),
DerivedUntracked(&'a [QueryEdge]) = QueryOriginKind::DerivedUntracked as u8,

/// The value is an initial provisional value for a query that supports fixpoint iteration.
FixpointInitial,
FixpointInitial = QueryOriginKind::FixpointInitial as u8,
}

impl<'a> QueryOriginRef<'a> {
/// Indices for queries *read* by this query
pub(crate) fn inputs(&self) -> impl DoubleEndedIterator<Item = DatabaseKeyIndex> + '_ {
#[inline]
pub(crate) fn inputs(self) -> impl DoubleEndedIterator<Item = DatabaseKeyIndex> + use<'a> {
let opt_edges = match self {
QueryOriginRef::Derived(edges) | QueryOriginRef::DerivedUntracked(edges) => Some(edges),
QueryOriginRef::Assigned(_) | QueryOriginRef::FixpointInitial => None,
};
opt_edges.into_iter().flat_map(|edges| input_edges(edges))
opt_edges.into_iter().flat_map(input_edges)
}

/// Indices for queries *written* by this query (if any)
pub(crate) fn outputs(&self) -> impl DoubleEndedIterator<Item = DatabaseKeyIndex> + '_ {
pub(crate) fn outputs(self) -> impl DoubleEndedIterator<Item = DatabaseKeyIndex> + use<'a> {
let opt_edges = match self {
QueryOriginRef::Derived(edges) | QueryOriginRef::DerivedUntracked(edges) => Some(edges),
QueryOriginRef::Assigned(_) | QueryOriginRef::FixpointInitial => None,
};
opt_edges.into_iter().flat_map(|edges| output_edges(edges))
opt_edges.into_iter().flat_map(output_edges)
}

pub(crate) fn edges(&self) -> &'a [QueryEdge] {
#[inline]
pub(crate) fn edges(self) -> &'a [QueryEdge] {
let opt_edges = match self {
QueryOriginRef::Derived(edges) | QueryOriginRef::DerivedUntracked(edges) => Some(edges),
QueryOriginRef::Assigned(_) | QueryOriginRef::FixpointInitial => None,
};

opt_edges.copied().unwrap_or_default()
opt_edges.unwrap_or_default()
}
}

// Note: The discriminant assignment is intentional,
// we want to group `Derived` and `DerivedUntracked` together on a same bit (the second LSB)
// as we tend to match against both of them in the same branch.
#[derive(Clone, Copy)]
#[repr(u8)]
enum QueryOriginKind {
/// An initial provisional value.
///
/// This will occur occur in queries that support fixpoint iteration.
FixpointInitial = 0b00,

/// The value was assigned as the output of another query.
///
/// This can, for example, can occur when `specify` is used.
Assigned,
Assigned = 0b01,

/// The value was derived by executing a function
/// _and_ Salsa was able to track all of said function's inputs.
Derived,
Derived = 0b11,

/// The value was derived by executing a function
/// but that function also reported that it read untracked inputs.
DerivedUntracked,

/// An initial provisional value.
///
/// This will occur occur in queries that support fixpoint iteration.
FixpointInitial,
DerivedUntracked = 0b10,
}

/// Tracks how a memoized value for a given query was created.
Expand Down Expand Up @@ -828,7 +835,7 @@ pub enum QueryEdgeKind {
/// These will always be in execution order.
pub(crate) fn input_edges(
input_outputs: &[QueryEdge],
) -> impl DoubleEndedIterator<Item = DatabaseKeyIndex> + '_ {
) -> impl DoubleEndedIterator<Item = DatabaseKeyIndex> + use<'_> {
input_outputs.iter().filter_map(|&edge| match edge.kind() {
QueryEdgeKind::Input(dependency_index) => Some(dependency_index),
QueryEdgeKind::Output(_) => None,
Expand All @@ -840,7 +847,7 @@ pub(crate) fn input_edges(
/// These will always be in execution order.
pub(crate) fn output_edges(
input_outputs: &[QueryEdge],
) -> impl DoubleEndedIterator<Item = DatabaseKeyIndex> + '_ {
) -> impl DoubleEndedIterator<Item = DatabaseKeyIndex> + use<'_> {
input_outputs.iter().filter_map(|&edge| match edge.kind() {
QueryEdgeKind::Output(dependency_index) => Some(dependency_index),
QueryEdgeKind::Input(_) => None,
Expand Down
2 changes: 1 addition & 1 deletion tests/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct Inputs {
}

impl Inputs {
fn values(self, db: &dyn Db) -> impl Iterator<Item = Value> + '_ {
fn values(self, db: &dyn Db) -> impl Iterator<Item = Value> + use<'_> {
self.inputs(db).iter().map(|input| input.eval(db))
}
}
Expand Down