Merge pull request #644 from Veykril/veykril/push-ltpvwpwqwkwn

Remove unreachable unwraps via typing

src/accumulator.rs

@@ -122,15 +122,15 @@ impl<A: Accumulator> Ingredient for IngredientImpl<A> {
         &self,
         _db: &dyn Database,
         _executor: DatabaseKeyIndex,
-        _output_key: Option<crate::Id>,
+        _output_key: crate::Id,
     ) {
     }
 
     fn remove_stale_output(
         &self,
         _db: &dyn Database,
         _executor: DatabaseKeyIndex,
-        _stale_output_key: Option<crate::Id>,
+        _stale_output_key: crate::Id,
     ) {
     }
 

src/active_query.rs

@@ -1,12 +1,14 @@
 use rustc_hash::FxHashMap;
 
-use super::zalsa_local::{EdgeKind, QueryEdges, QueryOrigin, QueryRevisions};
+use super::zalsa_local::{QueryEdges, QueryOrigin, QueryRevisions};
+use crate::key::OutputDependencyIndex;
 use crate::tracked_struct::IdentityHash;
+use crate::zalsa_local::QueryEdge;
 use crate::{
     accumulator::accumulated_map::{AccumulatedMap, InputAccumulatedValues},
     durability::Durability,
     hash::FxIndexSet,
-    key::{DatabaseKeyIndex, DependencyIndex},
+    key::{DatabaseKeyIndex, InputDependencyIndex},
     tracked_struct::{Disambiguator, Identity},
     Cycle, Id, Revision,
 };
@@ -29,7 +31,7 @@ pub(crate) struct ActiveQuery {
     /// * tracked structs created
     /// * invocations of `specify`
     /// * accumulators pushed to
-    input_outputs: FxIndexSet<(EdgeKind, DependencyIndex)>,
+    input_outputs: FxIndexSet<QueryEdge>,
 
     /// True if there was an untracked read.
     untracked_read: bool,
@@ -72,12 +74,12 @@ impl ActiveQuery {
 
     pub(super) fn add_read(
         &mut self,
-        input: DependencyIndex,
+        input: InputDependencyIndex,
         durability: Durability,
         revision: Revision,
         accumulated: InputAccumulatedValues,
     ) {
-        self.input_outputs.insert((EdgeKind::Input, input));
+        self.input_outputs.insert(QueryEdge::Input(input));
         self.durability = self.durability.min(durability);
         self.changed_at = self.changed_at.max(revision);
         self.accumulated.add_input(accumulated);
@@ -96,24 +98,17 @@ impl ActiveQuery {
     }
 
     /// Adds a key to our list of outputs.
-    pub(super) fn add_output(&mut self, key: DependencyIndex) {
-        self.input_outputs.insert((EdgeKind::Output, key));
+    pub(super) fn add_output(&mut self, key: OutputDependencyIndex) {
+        self.input_outputs.insert(QueryEdge::Output(key));
     }
 
     /// True if the given key was output by this query.
-    pub(super) fn is_output(&self, key: DependencyIndex) -> bool {
-        self.input_outputs.contains(&(EdgeKind::Output, key))
+    pub(super) fn is_output(&self, key: OutputDependencyIndex) -> bool {
+        self.input_outputs.contains(&QueryEdge::Output(key))
     }
 
     pub(crate) fn into_revisions(self) -> QueryRevisions {
-        let input_outputs = if self.input_outputs.is_empty() {
-            Box::default()
-        } else {
-            self.input_outputs.into_iter().collect()
-        };
-
-        let edges = QueryEdges::new(input_outputs);
-
+        let edges = QueryEdges::new(self.input_outputs);
         let origin = if self.untracked_read {
             QueryOrigin::DerivedUntracked(edges)
         } else {
@@ -143,8 +138,8 @@ impl ActiveQuery {
     /// Used during cycle recovery, see [`Runtime::unblock_cycle_and_maybe_throw`].
     pub(super) fn remove_cycle_participants(&mut self, cycle: &Cycle) {
         for p in cycle.participant_keys() {
-            let p: DependencyIndex = p.into();
-            self.input_outputs.shift_remove(&(EdgeKind::Input, p));
+            let p: InputDependencyIndex = p.into();
+            self.input_outputs.shift_remove(&QueryEdge::Input(p));
         }
     }
 

src/cycle.rs

@@ -16,7 +16,7 @@ use std::{panic::AssertUnwindSafe, sync::Arc};
 ///
 /// You can read more about cycle handling in
 /// the [salsa book](https://https://salsa-rs.github.io/salsa/cycles.html).
-#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, PartialEq, Eq, Hash)]
 pub struct Cycle {
     participants: CycleParticipants,
 }

src/event.rs

@@ -1,6 +1,9 @@
 use std::thread::ThreadId;
 
-use crate::{key::DatabaseKeyIndex, key::DependencyIndex};
+use crate::{
+    key::DatabaseKeyIndex,
+    key::{InputDependencyIndex, OutputDependencyIndex},
+};
 
 /// The `Event` struct identifies various notable things that can
 /// occur during salsa execution. Instances of this struct are given
@@ -64,7 +67,7 @@ pub enum EventKind {
         execute_key: DatabaseKeyIndex,
 
         /// Key for the query that is no longer output
-        output_key: DependencyIndex,
+        output_key: OutputDependencyIndex,
     },
 
     /// Tracked structs or memoized data were discarded (freed).
@@ -79,6 +82,6 @@ pub enum EventKind {
         executor_key: DatabaseKeyIndex,
 
         /// Accumulator that was accumulated into
-        accumulator: DependencyIndex,
+        accumulator: InputDependencyIndex,
     },
 }

src/function.rs

@@ -212,17 +212,16 @@ where
         &self,
         db: &dyn Database,
         executor: DatabaseKeyIndex,
-        output_key: Option<crate::Id>,
+        output_key: crate::Id,
     ) {
-        let output_key = output_key.unwrap();
         self.validate_specified_value(db, executor, output_key);
     }
 
     fn remove_stale_output(
         &self,
         _db: &dyn Database,
         _executor: DatabaseKeyIndex,
-        _stale_output_key: Option<crate::Id>,
+        _stale_output_key: crate::Id,
     ) {
         // This function is invoked when a query Q specifies the value for `stale_output_key` in rev 1,
         // but not in rev 2. We don't do anything in this case, we just leave the (now stale) memo.

src/function/diff_outputs.rs

@@ -1,6 +1,6 @@
 use super::{memo::Memo, Configuration, IngredientImpl};
 use crate::{
-    hash::FxHashSet, key::DependencyIndex, zalsa_local::QueryRevisions, AsDynDatabase as _,
+    hash::FxHashSet, key::OutputDependencyIndex, zalsa_local::QueryRevisions, AsDynDatabase as _,
     DatabaseKeyIndex, Event, EventKind,
 };
 
@@ -32,11 +32,8 @@ where
         if !old_outputs.is_empty() {
             // Remove the outputs that are no longer present in the current revision
             // to prevent that the next revision is seeded with a id mapping that no longer exists.
-            revisions.tracked_struct_ids.retain(|k, value| {
-                !old_outputs.contains(&DependencyIndex {
-                    ingredient_index: k.ingredient_index(),
-                    key_index: Some(*value),
-                })
+            revisions.tracked_struct_ids.retain(|&k, &mut value| {
+                !old_outputs.contains(&OutputDependencyIndex::new(k.ingredient_index(), value))
             });
         }
 
@@ -45,7 +42,7 @@ where
         }
     }
 
-    fn report_stale_output(db: &C::DbView, key: DatabaseKeyIndex, output: DependencyIndex) {
+    fn report_stale_output(db: &C::DbView, key: DatabaseKeyIndex, output: OutputDependencyIndex) {
         let db = db.as_dyn_database();
 
         db.salsa_event(&|| Event {

src/function/maybe_changed_after.rs

@@ -1,7 +1,7 @@
 use crate::{
     key::DatabaseKeyIndex,
     zalsa::{Zalsa, ZalsaDatabase},
-    zalsa_local::{ActiveQueryGuard, EdgeKind, QueryOrigin},
+    zalsa_local::{ActiveQueryGuard, QueryEdge, QueryOrigin},
     AsDynDatabase as _, Id, Revision,
 };
 
@@ -182,16 +182,16 @@ where
                 // valid, then some later input I1 might never have executed at all, so verifying
                 // it is still up to date is meaningless.
                 let last_verified_at = old_memo.verified_at.load();
-                for &(edge_kind, dependency_index) in edges.input_outputs.iter() {
-                    match edge_kind {
-                        EdgeKind::Input => {
+                for &edge in edges.input_outputs.iter() {
+                    match edge {
+                        QueryEdge::Input(dependency_index) => {
                             if dependency_index
                                 .maybe_changed_after(db.as_dyn_database(), last_verified_at)
                             {
                                 return false;
                             }
                         }
-                        EdgeKind::Output => {
+                        QueryEdge::Output(dependency_index) => {
                             // Subtle: Mark outputs as validated now, even though we may
                             // later find an input that requires us to re-execute the function.
                             // Even if it re-execute, the function will wind up writing the same value,

src/ingredient.rs

@@ -83,7 +83,7 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
         &'db self,
         db: &'db dyn Database,
         executor: DatabaseKeyIndex,
-        output_key: Option<Id>,
+        output_key: crate::Id,
     );
 
     /// Invoked when the value `stale_output` was output by `executor` in a previous
@@ -94,7 +94,7 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
         &self,
         db: &dyn Database,
         executor: DatabaseKeyIndex,
-        stale_output_key: Option<Id>,
+        stale_output_key: Id,
     );
 
     /// Returns the [`IngredientIndex`] of this ingredient.

src/input.rs

@@ -16,7 +16,7 @@ use crate::{
     cycle::CycleRecoveryStrategy,
     id::{AsId, FromId},
     ingredient::{fmt_index, Ingredient},
-    key::{DatabaseKeyIndex, DependencyIndex},
+    key::{DatabaseKeyIndex, InputDependencyIndex},
     plumbing::{Jar, JarAux, Stamp},
     table::{memo::MemoTable, sync::SyncTable, Slot, Table},
     zalsa::{IngredientIndex, Zalsa},
@@ -191,10 +191,7 @@ impl<C: Configuration> IngredientImpl<C> {
         let value = Self::data(zalsa, id);
         let stamp = &value.stamps[field_index];
         zalsa_local.report_tracked_read(
-            DependencyIndex {
-                ingredient_index: field_ingredient_index,
-                key_index: Some(id),
-            },
+            InputDependencyIndex::new(field_ingredient_index, id),
             stamp.durability,
             stamp.changed_at,
             InputAccumulatedValues::Empty,
@@ -240,7 +237,7 @@ impl<C: Configuration> Ingredient for IngredientImpl<C> {
         &self,
         _db: &dyn Database,
         executor: DatabaseKeyIndex,
-        output_key: Option<Id>,
+        output_key: Id,
     ) {
         unreachable!(
             "mark_validated_output({:?}, {:?}): input cannot be the output of a tracked function",
@@ -252,7 +249,7 @@ impl<C: Configuration> Ingredient for IngredientImpl<C> {
         &self,
         _db: &dyn Database,
         executor: DatabaseKeyIndex,
-        stale_output_key: Option<Id>,
+        stale_output_key: Id,
     ) {
         unreachable!(
             "remove_stale_output({:?}, {:?}): input cannot be the output of a tracked function",

src/input/input_field.rs

@@ -69,15 +69,15 @@ where
         &self,
         _db: &dyn Database,
         _executor: DatabaseKeyIndex,
-        _output_key: Option<Id>,
+        _output_key: Id,
     ) {
     }
 
     fn remove_stale_output(
         &self,
         _db: &dyn Database,
         _executor: DatabaseKeyIndex,
-        _stale_output_key: Option<Id>,
+        _stale_output_key: Id,
     ) {
     }
 

src/interned.rs

@@ -2,7 +2,7 @@ use crate::accumulator::accumulated_map::InputAccumulatedValues;
 use crate::durability::Durability;
 use crate::id::AsId;
 use crate::ingredient::fmt_index;
-use crate::key::DependencyIndex;
+use crate::key::InputDependencyIndex;
 use crate::plumbing::{Jar, JarAux};
 use crate::table::memo::MemoTable;
 use crate::table::sync::SyncTable;
@@ -136,7 +136,7 @@ where
     ) -> C::Struct<'db> {
         let zalsa_local = db.zalsa_local();
         zalsa_local.report_tracked_read(
-            DependencyIndex::for_table(self.ingredient_index),
+            InputDependencyIndex::for_table(self.ingredient_index),
             Durability::MAX,
             self.reset_at,
             InputAccumulatedValues::Empty,
@@ -241,7 +241,7 @@ where
         &self,
         _db: &dyn Database,
         executor: DatabaseKeyIndex,
-        output_key: Option<crate::Id>,
+        output_key: crate::Id,
     ) {
         unreachable!(
             "mark_validated_output({:?}, {:?}): input cannot be the output of a tracked function",
@@ -253,7 +253,7 @@ where
         &self,
         _db: &dyn Database,
         executor: DatabaseKeyIndex,
-        stale_output_key: Option<crate::Id>,
+        stale_output_key: crate::Id,
     ) {
         unreachable!(
             "remove_stale_output({:?}, {:?}): interned ids are not outputs",