Use Result over catch_unwind for cancellation and cycle handling

salsa-rs · Sep 1, 2024 · 12cf727 · 12cf727
1 parent 2af849b
commit 12cf727
Show file tree

Hide file tree

Showing 103 changed files with 2,021 additions and 1,694 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -11,6 +11,7 @@ description = "A generic framework for on-demand, incrementalized computation (e
 arc-swap = "1"
 crossbeam = "0.8"
 dashmap = "6"
+drop_bomb = "0.1.5"
 hashlink = "0.9"
 indexmap = "2"
 append-only-vec = "0.1.5"

diff --git a/benches/compare.rs b/benches/compare.rs
@@ -7,8 +7,8 @@ pub struct Input {
 }
 
 #[salsa::tracked]
-pub fn length(db: &dyn salsa::Database, input: Input) -> usize {
- input.text(db).len()
+pub fn length(db: &dyn salsa::Database, input: Input) -> salsa::Result<usize> {
+ Ok(input.text(db)?.len())
 }
 
 #[salsa::interned]
@@ -17,8 +17,11 @@ pub struct InternedInput<'db> {
 }
 
 #[salsa::tracked]
-pub fn interned_length<'db>(db: &'db dyn salsa::Database, input: InternedInput<'db>) -> usize {
- input.text(db).len()
+pub fn interned_length<'db>(
+ db: &'db dyn salsa::Database,
+ input: InternedInput<'db>,
+) -> salsa::Result<usize> {
+ Ok(input.text(db).len())
 }
 
 fn mutating_inputs(c: &mut Criterion) {
@@ -38,11 +41,11 @@ fn mutating_inputs(c: &mut Criterion) {
  group.bench_function(BenchmarkId::new("mutating", n), |b| {
  b.iter(|| {
  let input = Input::new(&db, base_string.clone());
- let actual_len = length(&db, input);
+ let actual_len = length(&db, input).unwrap();
  assert_eq!(base_len, actual_len);
 
  input.set_text(&mut db).to(string.clone());
- let actual_len = length(&db, input);
+ let actual_len = length(&db, input).unwrap();
  assert_eq!(new_len, actual_len);
  })
  });
@@ -60,30 +63,30 @@ fn inputs(c: &mut Criterion) {
 
  group.bench_function(BenchmarkId::new("new", "InternedInput"), |b| {
  b.iter(|| {
- let input: InternedInput = InternedInput::new(&db, "hello, world!".to_owned());
- interned_length(&db, input);
+ let input: InternedInput = InternedInput::new(&db, "hello, world!".to_owned()).unwrap();
+ interned_length(&db, input).unwrap();
  })
  });
 
  group.bench_function(BenchmarkId::new("amortized", "InternedInput"), |b| {
- let input = InternedInput::new(&db, "hello, world!".to_owned());
- let _ = interned_length(&db, input);
+ let input = InternedInput::new(&db, "hello, world!".to_owned()).unwrap();
+ let _ = interned_length(&db, input).unwrap();
 
- b.iter(|| interned_length(&db, input));
+ b.iter(|| interned_length(&db, input).unwrap());
  });
 
  group.bench_function(BenchmarkId::new("new", "Input"), |b| {
  b.iter(|| {
  let input = Input::new(&db, "hello, world!".to_owned());
- length(&db, input);
+ length(&db, input).unwrap();
  })
  });
 
  group.bench_function(BenchmarkId::new("amortized", "Input"), |b| {
  let input = Input::new(&db, "hello, world!".to_owned());
- let _ = length(&db, input);
+ let _ = length(&db, input).unwrap();
 
- b.iter(|| length(&db, input));
+ b.iter(|| length(&db, input).unwrap());
  });
 
  group.finish();

diff --git a/benches/incremental.rs b/benches/incremental.rs
@@ -12,14 +12,14 @@ struct Tracked<'db> {
 }
 
 #[salsa::tracked(return_ref)]
-fn index<'db>(db: &'db dyn salsa::Database, input: Input) -> Vec<Tracked<'db>> {
- (0..input.field(db)).map(|i| Tracked::new(db, i)).collect()
+fn index<'db>(db: &'db dyn salsa::Database, input: Input) -> salsa::Result<Vec<Tracked<'db>>> {
+ (0..input.field(db)?).map(|i| Tracked::new(db, i)).collect()
 }
 
 #[salsa::tracked]
-fn root(db: &dyn salsa::Database, input: Input) -> usize {
- let index = index(db, input);
- index.len()
+fn root(db: &dyn salsa::Database, input: Input) -> salsa::Result<usize> {
+ let index = index(db, input)?;
+ Ok(index.len())
 }
 
 fn many_tracked_structs(criterion: &mut Criterion) {
@@ -41,7 +41,7 @@ fn many_tracked_structs(criterion: &mut Criterion) {
  // Make a change, but fetch the result for the other input
  input2.set_field(db).to(2);
 
- let result = root(db, *input);
+ let result = root(db, *input).unwrap();
 
  assert_eq!(result, 1_000);
  },

diff --git a/components/salsa-macro-rules/src/setup_input_struct.rs b/components/salsa-macro-rules/src/setup_input_struct.rs
@@ -142,7 +142,7 @@ macro_rules! setup_input_struct {
  }
 
  $(
- $field_getter_vis fn $field_getter_id<'db, $Db>(self, db: &'db $Db) -> $zalsa::maybe_cloned_ty!($field_option, 'db, $field_ty)
+ $field_getter_vis fn $field_getter_id<'db, $Db>(self, db: &'db $Db) -> salsa::Result<$zalsa::maybe_cloned_ty!($field_option, 'db, $field_ty)>
  where
  // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
  $Db: ?Sized + $zalsa::Database,
@@ -151,12 +151,12 @@ macro_rules! setup_input_struct {
  db.as_dyn_database(),
  self,
  $field_index,
- );
- $zalsa::maybe_clone!(
+ )?;
+ Ok($zalsa::maybe_clone!(
  $field_option,
  $field_ty,
  &fields.$field_index,
- )
+ ))
  }
  )*
 

diff --git a/components/salsa-macro-rules/src/setup_interned_struct.rs b/components/salsa-macro-rules/src/setup_interned_struct.rs
@@ -126,7 +126,7 @@ macro_rules! setup_interned_struct {
  }
 
  impl<$db_lt> $Struct<$db_lt> {
- pub fn $new_fn<$Db>(db: &$db_lt $Db, $($field_id: $field_ty),*) -> Self
+ pub fn $new_fn<$Db>(db: &$db_lt $Db, $($field_id: $field_ty),*) -> salsa::Result<Self>
  where
  // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
  $Db: ?Sized + salsa::Database,

diff --git a/components/salsa-macro-rules/src/setup_tracked_fn.rs b/components/salsa-macro-rules/src/setup_tracked_fn.rs
@@ -80,7 +80,7 @@ macro_rules! setup_tracked_fn {
  $($input_id: $input_ty,)*
  ) -> salsa::plumbing::macro_if! {
  if $return_ref {
- &$db_lt $output_ty
+ salsa::Result<&$db_lt <$output_ty as salsa::plumbing::HasOutput>::Output>
  } else {
  $output_ty
  }
@@ -156,7 +156,7 @@ macro_rules! setup_tracked_fn {
 
  type Input<$db_lt> = ($($input_ty),*);
 
- type Output<$db_lt> = $output_ty;
+ type Output<$db_lt> = <$output_ty as $zalsa::HasOutput>::Output;
 
  const CYCLE_STRATEGY: $zalsa::CycleRecoveryStrategy = $zalsa::CycleRecoveryStrategy::$cycle_recovery_strategy;
 
@@ -173,7 +173,7 @@ macro_rules! setup_tracked_fn {
  }
  }
 
- fn execute<$db_lt>($db: &$db_lt Self::DbView, ($($input_id),*): ($($input_ty),*)) -> Self::Output<$db_lt> {
+ fn execute<$db_lt>($db: &$db_lt Self::DbView, ($($input_id),*): ($($input_ty),*)) -> salsa::Result<Self::Output<$db_lt>> {
  $inner_fn
 
  $inner($db, $($input_id),*)
@@ -231,11 +231,11 @@ macro_rules! setup_tracked_fn {
  pub fn accumulated<$db_lt, A: salsa::Accumulator>(
  $db: &$db_lt dyn $Db,
  $($input_id: $input_ty,)*
- ) -> Vec<A> {
+ ) -> salsa::Result<Vec<A>> {
  use salsa::plumbing as $zalsa;
  let key = $zalsa::macro_if! {
  if $needs_interner {
- $Configuration::intern_ingredient($db).intern_id($db.as_dyn_database(), ($($input_id),*))
+ $Configuration::intern_ingredient($db).intern_id($db.as_dyn_database(), ($($input_id),*))?
  } else {
  $zalsa::AsId::as_id(&($($input_id),*))
  }
@@ -248,7 +248,7 @@ macro_rules! setup_tracked_fn {
  pub fn specify<$db_lt>(
  $db: &$db_lt dyn $Db,
  $($input_id: $input_ty,)*
- value: $output_ty,
+ value: <$Configuration as $zalsa::function::Configuration>::Output<$db_lt>,
  ) {
  let key = $zalsa::AsId::as_id(&($($input_id),*));
  $Configuration::fn_ingredient($db).specify_and_record(
@@ -271,21 +271,21 @@ macro_rules! setup_tracked_fn {
  let result = $zalsa::macro_if! {
  if $needs_interner {
  {
- let key = $Configuration::intern_ingredient($db).intern_id($db.as_dyn_database(), ($($input_id),*));
- $Configuration::fn_ingredient($db).fetch($db, key)
+ let key = $Configuration::intern_ingredient($db).intern_id($db.as_dyn_database(), ($($input_id),*))?;
+ $Configuration::fn_ingredient($db).fetch($db, key)?
  }
  } else {
- $Configuration::fn_ingredient($db).fetch($db, $zalsa::AsId::as_id(&($($input_id),*)))
+ $Configuration::fn_ingredient($db).fetch($db, $zalsa::AsId::as_id(&($($input_id),*)))?
  }
  };
 
- $zalsa::macro_if! {
+ Ok($zalsa::macro_if! {
  if $return_ref {
  result
  } else {
- <$output_ty as std::clone::Clone>::clone(result)
+ <<$Configuration as $zalsa::function::Configuration>::Output<$db_lt> as std::clone::Clone>::clone(result)
  }
- }
+ })
  })
  }
  };

diff --git a/components/salsa-macro-rules/src/setup_tracked_struct.rs b/components/salsa-macro-rules/src/setup_tracked_struct.rs
@@ -184,7 +184,7 @@ macro_rules! setup_tracked_struct {
  }
 
  impl<$db_lt> $Struct<$db_lt> {
- pub fn $new_fn<$Db>(db: &$db_lt $Db, $($field_id: $field_ty),*) -> Self
+ pub fn $new_fn<$Db>(db: &$db_lt $Db, $($field_id: $field_ty),*) -> salsa::Result<Self>
  where
  // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
  $Db: ?Sized + $zalsa::Database,
@@ -196,18 +196,18 @@ macro_rules! setup_tracked_struct {
  }
 
  $(
- $field_getter_vis fn $field_getter_id<$Db>(self, db: &$db_lt $Db) -> $crate::maybe_cloned_ty!($field_option, $db_lt, $field_ty)
+ $field_getter_vis fn $field_getter_id<$Db>(self, db: &$db_lt $Db) -> salsa::Result<$crate::maybe_cloned_ty!($field_option, $db_lt, $field_ty)>
  where
  // FIXME(rust-lang/rust#65991): The `db` argument *should* have the type `dyn Database`
  $Db: ?Sized + $zalsa::Database,
  {
  let db = db.as_dyn_database();
- let fields = $Configuration::ingredient(db).field(db, self, $field_index);
- $crate::maybe_clone!(
+ let fields = $Configuration::ingredient(db).field(db, self, $field_index)?;
+ Ok($crate::maybe_clone!(
  $field_option,
  $field_ty,
  &fields.$field_index,
- )
+ ))
  }
  )*
 

diff --git a/components/salsa-macros/src/tracked_impl.rs b/components/salsa-macros/src/tracked_impl.rs
@@ -274,7 +274,8 @@ impl Macro {
  ) -> syn::Result<()> {
  if let Some(return_ref) = &args.return_ref {
  if let syn::ReturnType::Type(_, t) = &mut sig.output {
- **t = parse_quote!(& #db_lt #t)
+ **t =
+ parse_quote!(salsa::Result<&#db_lt <#t as salsa::plumbing::HasOutput>::Output>)
  } else {
  return Err(syn::Error::new_spanned(
  return_ref,

diff --git a/examples/calc/compile.rs b/examples/calc/compile.rs
@@ -1,7 +1,9 @@
 use crate::{ir::SourceProgram, parser::parse_statements, type_check::type_check_program};
 
 #[salsa::tracked]
-pub fn compile(db: &dyn crate::Db, source_program: SourceProgram) {
- let program = parse_statements(db, source_program);
- type_check_program(db, program);
+pub fn compile(db: &dyn crate::Db, source_program: SourceProgram) -> salsa::Result<()> {
+ let program = parse_statements(db, source_program)?;
+ type_check_program(db, program)?;
+
+ Ok(())
 }
diff --git a/examples/calc/main.rs b/examples/calc/main.rs
@@ -11,7 +11,7 @@ mod type_check;
 pub fn main() {
  let db: CalcDatabaseImpl = Default::default();
  let source_program = SourceProgram::new(&db, String::new());
- compile::compile(&db, source_program);
- let diagnostics = compile::compile::accumulated::<Diagnostic>(&db, source_program);
+ compile::compile(&db, source_program).unwrap();
+ let diagnostics = compile::compile::accumulated::<Diagnostic>(&db, source_program).unwrap();
  eprintln!("{diagnostics:?}");
 }