Merge #735

735: make HirDatabase object-safe r=matklad a=matklad



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

crates/ra_db/src/lib.rs

@@ -19,7 +19,7 @@ pub use crate::{
     loc2id::LocationIntener,
 };
 
-pub trait CheckCanceled: salsa::Database + panic::RefUnwindSafe {
+pub trait CheckCanceled: panic::RefUnwindSafe {
     /// Aborts current query if there are pending changes.
     ///
     /// rust-analyzer needs to be able to answer semantic questions about the
@@ -33,16 +33,13 @@ pub trait CheckCanceled: salsa::Database + panic::RefUnwindSafe {
     ///
     /// 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()
-        }
-    }
+    fn check_canceled(&self);
 
-    fn catch_canceled<F: FnOnce(&Self) -> T + panic::UnwindSafe, T>(
-        &self,
-        f: F,
-    ) -> Result<T, Canceled> {
+    fn catch_canceled<F, T>(&self, f: F) -> Result<T, Canceled>
+    where
+        Self: Sized,
+        F: FnOnce(&Self) -> T + panic::UnwindSafe,
+    {
         panic::catch_unwind(|| f(self)).map_err(|err| match err.downcast::<Canceled>() {
             Ok(canceled) => *canceled,
             Err(payload) => panic::resume_unwind(payload),
@@ -50,6 +47,14 @@ pub trait CheckCanceled: salsa::Database + panic::RefUnwindSafe {
     }
 }
 
+impl<T: salsa::Database + panic::RefUnwindSafe> CheckCanceled for T {
+    fn check_canceled(&self) {
+        if self.salsa_runtime().is_current_revision_canceled() {
+            Canceled::throw()
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug)]
 pub struct FilePosition {
     pub file_id: FileId,
@@ -65,7 +70,7 @@ pub struct FileRange {
 /// Database which stores all significant input facts: source code and project
 /// model. Everything else in rust-analyzer is derived from these queries.
 #[salsa::query_group(SourceDatabaseStorage)]
-pub trait SourceDatabase: salsa::Database + CheckCanceled {
+pub trait SourceDatabase: CheckCanceled {
     /// Text of the file.
     #[salsa::input]
     fn file_text(&self, file_id: FileId) -> Arc<String>;

crates/ra_hir/src/db.rs

@@ -103,3 +103,8 @@ pub trait HirDatabase: PersistentHirDatabase {
     #[salsa::invoke(crate::ty::method_resolution::CrateImplBlocks::impls_in_crate_query)]
     fn impls_in_crate(&self, krate: Crate) -> Arc<CrateImplBlocks>;
 }
+
+#[test]
+fn hir_database_is_object_safe() {
+    fn _assert_object_safe(_: &dyn HirDatabase) {}
+}

crates/ra_hir/src/mock.rs

@@ -2,7 +2,7 @@ use std::{sync::Arc, panic};
 
 use parking_lot::Mutex;
 use ra_db::{
-    CheckCanceled, FilePosition, FileId, CrateGraph, SourceRoot, SourceRootId, SourceDatabase, salsa,
+    FilePosition, FileId, CrateGraph, SourceRoot, SourceRootId, SourceDatabase, salsa,
 };
 use relative_path::RelativePathBuf;
 use test_utils::{parse_fixture, CURSOR_MARKER, extract_offset};
@@ -159,8 +159,6 @@ impl salsa::ParallelDatabase for MockDatabase {
     }
 }
 
-impl CheckCanceled for MockDatabase {}
-
 impl AsRef<HirInterner> for MockDatabase {
     fn as_ref(&self) -> &HirInterner {
         &self.interner

crates/ra_ide_api/src/db.rs

@@ -60,8 +60,6 @@ impl salsa::ParallelDatabase for RootDatabase {
     }
 }
 
-impl CheckCanceled for RootDatabase {}
-
 impl AsRef<hir::HirInterner> for RootDatabase {
     fn as_ref(&self) -> &hir::HirInterner {
         &self.interner

crates/ra_ide_api/src/lib.rs

@@ -9,6 +9,10 @@
 //!
 //! The sibling `ra_ide_api_light` handles thouse bits of IDE functionality
 //! which are restricted to a single file and need only syntax.
+
+// For proving that RootDatabase is RefUnwindSafe.
+#![recursion_limit = "128"]
+
 mod db;
 mod imp;
 pub mod mock_analysis;