Merge #548

548: check_canceled does not return Result r=matklad a=matklad



Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_db/src/lib.rs

@@ -21,11 +21,23 @@ pub use crate::{
 };
 
 pub trait BaseDatabase: salsa::Database + panic::RefUnwindSafe {
-    fn check_canceled(&self) -> Cancelable<()> {
+    /// Aborts current query if there are pending changes.
+    ///
+    /// rust-analyzer needs to be able to answer semantic questions about the
+    /// code while the code is being modified. A common problem is that a
+    /// long-running query is being calculated when a new change arrives.
+    ///
+    /// We can't just apply the change immediately: this will cause the pending
+    /// query to see inconsistent state (it will observe an absence of
+    /// repeatable read). So what we do is we **cancel** all pending queries
+    /// before applying the change.
+    ///
+    /// We implement cancellation by panicking with a special value and catching
+    /// it on the API boundary. Salsa explicitly supports this use-case.
+    fn check_canceled(&self) {
         if self.salsa_runtime().is_current_revision_canceled() {
             Canceled::throw()
         }
-        Ok(())
     }
 
     fn catch_canceled<F: FnOnce(&Self) -> T + panic::UnwindSafe, T>(

crates/ra_hir/src/module_tree.rs

@@ -42,7 +42,7 @@ impl Submodule {
         db: &impl HirDatabase,
         source: SourceItemId,
     ) -> Cancelable<Arc<Vec<Submodule>>> {
-        db.check_canceled()?;
+        db.check_canceled();
         let file_id = source.file_id;
         let file_items = db.file_items(file_id);
         let module_source = ModuleSource::from_source_item_id(db, source);
@@ -117,7 +117,7 @@ impl ModuleTree {
         db: &impl HirDatabase,
         source_root: SourceRootId,
     ) -> Cancelable<Arc<ModuleTree>> {
-        db.check_canceled()?;
+        db.check_canceled();
         let res = create_module_tree(db, source_root);
         Ok(Arc::new(res?))
     }

crates/ra_hir/src/nameres.rs

@@ -327,7 +327,7 @@ where
         loop {
             let processed_imports_count = self.processed_imports.len();
             for &module_id in self.input.keys() {
-                self.db.check_canceled()?;
+                self.db.check_canceled();
                 self.resolve_imports(module_id)?;
             }
             if processed_imports_count == self.processed_imports.len() {

crates/ra_hir/src/ty.rs

@@ -1203,7 +1203,7 @@ impl<'a, D: HirDatabase> InferenceContext<'a, D> {
 }
 
 pub fn infer(db: &impl HirDatabase, def_id: DefId) -> Cancelable<Arc<InferenceResult>> {
-    db.check_canceled()?;
+    db.check_canceled();
     let function = Function::new(def_id); // TODO: consts also need inference
     let body = function.body(db)?;
     let scopes = db.fn_scopes(def_id)?;

crates/ra_ide_api/src/symbol_index.rs

@@ -54,7 +54,7 @@ salsa::query_group! {
 }
 
 fn file_symbols(db: &impl SymbolsDatabase, file_id: FileId) -> Cancelable<Arc<SymbolIndex>> {
-    db.check_canceled()?;
+    db.check_canceled();
     let source_file = db.source_file(file_id);
     let mut symbols = source_file
         .syntax()