Use Result over catch_unwind for cancellation and cycle handling

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

Hide file tree

Showing 101 changed files with 1,998 additions and 1,674 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/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:?}");
 }