Merge #147

147: Cancelation r=matklad a=matklad

This series of commits switch cancellation strategy from `JobToken` (which are cancellation tokens, explicitly controlled by the called) to salsa built-in auto cancellation. "Auto" means that, as soon as we advance the revision, all pending queries are cancelled automatically, and this looks like a semantic we actually want. 

"client-side" cancellation is a rare event, and it's ok to just punt on it. Automatic cancellation after the user types something in happens all the time. 

Co-authored-by: Aleksey Kladov <[email protected]>

5 files changed, 53 insertions, 76 deletions

diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index 639fe4553..f5dff4c80 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -1,12 +1,13 @@
-use rustc_hash::FxHashMap;
+use std::collections::HashMap;
 
+use rustc_hash::FxHashMap;
 use languageserver_types::{
     CodeActionResponse, Command, CompletionItem, CompletionItemKind, Diagnostic,
     DiagnosticSeverity, DocumentSymbol, FoldingRange, FoldingRangeKind, FoldingRangeParams,
     InsertTextFormat, Location, Position, SymbolInformation, TextDocumentIdentifier, TextEdit,
     RenameParams, WorkspaceEdit, PrepareRenameResponse
 };
-use ra_analysis::{FileId, FoldKind, JobToken, Query, RunnableKind};
+use ra_analysis::{FileId, FoldKind, Query, RunnableKind};
 use ra_syntax::text_utils::contains_offset_nonstrict;
 use serde_json::to_value;
 
@@ -18,12 +19,9 @@ use crate::{
     Result,
 };
 
-use std::collections::HashMap;
-
 pub fn handle_syntax_tree(
     world: ServerWorld,
     params: req::SyntaxTreeParams,
-    _token: JobToken,
 ) -> Result<String> {
     let id = params.text_document.try_conv_with(&world)?;
     let res = world.analysis().syntax_tree(id);
@@ -33,7 +31,6 @@ pub fn handle_syntax_tree(
 pub fn handle_extend_selection(
     world: ServerWorld,
     params: req::ExtendSelectionParams,
-    _token: JobToken,
 ) -> Result<req::ExtendSelectionResult> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let file = world.analysis().file_syntax(file_id);
@@ -51,7 +48,6 @@ pub fn handle_extend_selection(
 pub fn handle_find_matching_brace(
     world: ServerWorld,
     params: req::FindMatchingBraceParams,
-    _token: JobToken,
 ) -> Result<Vec<Position>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let file = world.analysis().file_syntax(file_id);
@@ -74,7 +70,6 @@ pub fn handle_find_matching_brace(
 pub fn handle_join_lines(
     world: ServerWorld,
     params: req::JoinLinesParams,
-    _token: JobToken,
 ) -> Result<req::SourceChange> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
@@ -88,7 +83,6 @@ pub fn handle_join_lines(
 pub fn handle_on_enter(
     world: ServerWorld,
     params: req::TextDocumentPositionParams,
-    _token: JobToken,
 ) -> Result<Option<req::SourceChange>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
@@ -102,7 +96,6 @@ pub fn handle_on_enter(
 pub fn handle_on_type_formatting(
     world: ServerWorld,
     params: req::DocumentOnTypeFormattingParams,
-    _token: JobToken,
 ) -> Result<Option<Vec<TextEdit>>> {
     if params.ch != "=" {
         return Ok(None);
@@ -122,7 +115,6 @@ pub fn handle_on_type_formatting(
 pub fn handle_document_symbol(
     world: ServerWorld,
     params: req::DocumentSymbolParams,
-    _token: JobToken,
 ) -> Result<Option<req::DocumentSymbolResponse>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
@@ -161,7 +153,6 @@ pub fn handle_document_symbol(
 pub fn handle_workspace_symbol(
     world: ServerWorld,
     params: req::WorkspaceSymbolParams,
-    token: JobToken,
 ) -> Result<Option<Vec<SymbolInformation>>> {
     let all_symbols = params.query.contains("#");
     let libs = params.query.contains("*");
@@ -181,11 +172,11 @@ pub fn handle_workspace_symbol(
         q.limit(128);
         q
     };
-    let mut res = exec_query(&world, query, &token)?;
+    let mut res = exec_query(&world, query)?;
     if res.is_empty() && !all_symbols {
         let mut query = Query::new(params.query);
         query.limit(128);
-        res = exec_query(&world, query, &token)?;
+        res = exec_query(&world, query)?;
     }
 
     return Ok(Some(res));
@@ -193,10 +184,9 @@ pub fn handle_workspace_symbol(
     fn exec_query(
         world: &ServerWorld,
         query: Query,
-        token: &JobToken,
     ) -> Result<Vec<SymbolInformation>> {
         let mut res = Vec::new();
-        for (file_id, symbol) in world.analysis().symbol_search(query, token) {
+        for (file_id, symbol) in world.analysis().symbol_search(query)? {
             let line_index = world.analysis().file_line_index(file_id);
             let info = SymbolInformation {
                 name: symbol.name.to_string(),
@@ -214,7 +204,6 @@ pub fn handle_workspace_symbol(
 pub fn handle_goto_definition(
     world: ServerWorld,
     params: req::TextDocumentPositionParams,
-    token: JobToken,
 ) -> Result<Option<req::GotoDefinitionResponse>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
@@ -222,7 +211,7 @@ pub fn handle_goto_definition(
     let mut res = Vec::new();
     for (file_id, symbol) in world
         .analysis()
-        .approximately_resolve_symbol(file_id, offset, &token)
+        .approximately_resolve_symbol(file_id, offset)?
     {
         let line_index = world.analysis().file_line_index(file_id);
         let location = to_location(file_id, symbol.node_range, &world, &line_index)?;
@@ -234,11 +223,10 @@ pub fn handle_goto_definition(
 pub fn handle_parent_module(
     world: ServerWorld,
     params: TextDocumentIdentifier,
-    _token: JobToken,
 ) -> Result<Vec<Location>> {
     let file_id = params.try_conv_with(&world)?;
     let mut res = Vec::new();
-    for (file_id, symbol) in world.analysis().parent_module(file_id) {
+    for (file_id, symbol) in world.analysis().parent_module(file_id)? {
         let line_index = world.analysis().file_line_index(file_id);
         let location = to_location(file_id, symbol.node_range, &world, &line_index)?;
         res.push(location);
@@ -249,20 +237,19 @@ pub fn handle_parent_module(
 pub fn handle_runnables(
     world: ServerWorld,
     params: req::RunnablesParams,
-    _token: JobToken,
 ) -> Result<Vec<req::Runnable>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
     let offset = params.position.map(|it| it.conv_with(&line_index));
     let mut res = Vec::new();
-    for runnable in world.analysis().runnables(file_id) {
+    for runnable in world.analysis().runnables(file_id)? {
         if let Some(offset) = offset {
             if !contains_offset_nonstrict(runnable.range, offset) {
                 continue;
             }
         }
 
-        let args = runnable_args(&world, file_id, &runnable.kind);
+        let args = runnable_args(&world, file_id, &runnable.kind)?;
 
         let r = req::Runnable {
             range: runnable.range.conv_with(&line_index),
@@ -282,9 +269,9 @@ pub fn handle_runnables(
     }
     return Ok(res);
 
-    fn runnable_args(world: &ServerWorld, file_id: FileId, kind: &RunnableKind) -> Vec<String> {
-        let spec = if let Some(&crate_id) = world.analysis().crate_for(file_id).first() {
-            let file_id = world.analysis().crate_root(crate_id);
+    fn runnable_args(world: &ServerWorld, file_id: FileId, kind: &RunnableKind) -> Result<Vec<String>> {
+        let spec = if let Some(&crate_id) = world.analysis().crate_for(file_id)?.first() {
+            let file_id = world.analysis().crate_root(crate_id)?;
             let path = world.path_map.get_path(file_id);
             world
                 .workspaces
@@ -319,7 +306,7 @@ pub fn handle_runnables(
                 }
             }
         }
-        res
+        Ok(res)
     }
 
     fn spec_args(pkg_name: &str, tgt_name: &str, tgt_kind: TargetKind, buf: &mut Vec<String>) {
@@ -353,21 +340,19 @@ pub fn handle_runnables(
 pub fn handle_decorations(
     world: ServerWorld,
     params: TextDocumentIdentifier,
-    _token: JobToken,
 ) -> Result<Vec<Decoration>> {
     let file_id = params.try_conv_with(&world)?;
-    Ok(highlight(&world, file_id))
+    highlight(&world, file_id)
 }
 
 pub fn handle_completion(
     world: ServerWorld,
     params: req::CompletionParams,
-    _token: JobToken,
 ) -> Result<Option<req::CompletionResponse>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
     let offset = params.position.conv_with(&line_index);
-    let items = match world.analysis().completions(file_id, offset) {
+    let items = match world.analysis().completions(file_id, offset)? {
         None => return Ok(None),
         Some(items) => items,
     };
@@ -394,7 +379,6 @@ pub fn handle_completion(
 pub fn handle_folding_range(
     world: ServerWorld,
     params: FoldingRangeParams,
-    _token: JobToken,
 ) -> Result<Option<Vec<FoldingRange>>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
@@ -427,7 +411,6 @@ pub fn handle_folding_range(
 pub fn handle_signature_help(
     world: ServerWorld,
     params: req::TextDocumentPositionParams,
-    token: JobToken,
 ) -> Result<Option<req::SignatureHelp>> {
     use languageserver_types::{ParameterInformation, SignatureInformation};
 
@@ -436,7 +419,7 @@ pub fn handle_signature_help(
     let offset = params.position.conv_with(&line_index);
 
     if let Some((descriptor, active_param)) =
-        world.analysis().resolve_callable(file_id, offset, &token)
+        world.analysis().resolve_callable(file_id, offset)?
     {
         let parameters: Vec<ParameterInformation> = descriptor
             .params
@@ -466,7 +449,6 @@ pub fn handle_signature_help(
 pub fn handle_prepare_rename(
     world: ServerWorld,
     params: req::TextDocumentPositionParams,
-    token: JobToken,
 ) -> Result<Option<PrepareRenameResponse>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
@@ -474,7 +456,7 @@ pub fn handle_prepare_rename(
 
     // We support renaming references like handle_rename does.
     // In the future we may want to reject the renaming of things like keywords here too.
-    let refs = world.analysis().find_all_refs(file_id, offset, &token);
+    let refs = world.analysis().find_all_refs(file_id, offset)?;
     if refs.is_empty() {
         return Ok(None);
     }
@@ -488,7 +470,6 @@ pub fn handle_prepare_rename(
 pub fn handle_rename(
     world: ServerWorld,
     params: RenameParams,
-    token: JobToken,
 ) -> Result<Option<WorkspaceEdit>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
@@ -498,7 +479,7 @@ pub fn handle_rename(
         return Ok(None);
     }
 
-    let refs = world.analysis().find_all_refs(file_id, offset, &token);
+    let refs = world.analysis().find_all_refs(file_id, offset)?;
     if refs.is_empty() {
         return Ok(None);
     }
@@ -525,13 +506,12 @@ pub fn handle_rename(
 pub fn handle_references(
     world: ServerWorld,
     params: req::ReferenceParams,
-    token: JobToken,
 ) -> Result<Option<Vec<Location>>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
     let offset = params.position.conv_with(&line_index);
 
-    let refs = world.analysis().find_all_refs(file_id, offset, &token);
+    let refs = world.analysis().find_all_refs(file_id, offset)?;
 
     Ok(Some(refs.into_iter()
         .filter_map(|r| to_location(r.0, r.1, &world, &line_index).ok())
@@ -541,16 +521,15 @@ pub fn handle_references(
 pub fn handle_code_action(
     world: ServerWorld,
     params: req::CodeActionParams,
-    _token: JobToken,
 ) -> Result<Option<CodeActionResponse>> {
     let file_id = params.text_document.try_conv_with(&world)?;
     let line_index = world.analysis().file_line_index(file_id);
     let range = params.range.conv_with(&line_index);
 
-    let assists = world.analysis().assists(file_id, range).into_iter();
+    let assists = world.analysis().assists(file_id, range)?.into_iter();
     let fixes = world
         .analysis()
-        .diagnostics(file_id)
+        .diagnostics(file_id)?
         .into_iter()
         .filter_map(|d| Some((d.range, d.fix?)))
         .filter(|(range, _fix)| contains_offset_nonstrict(*range, range.start()))
@@ -579,7 +558,7 @@ pub fn publish_diagnostics(
     let line_index = world.analysis().file_line_index(file_id);
     let diagnostics = world
         .analysis()
-        .diagnostics(file_id)
+        .diagnostics(file_id)?
         .into_iter()
         .map(|d| Diagnostic {
             range: d.range.conv_with(&line_index),
@@ -600,19 +579,20 @@ pub fn publish_decorations(
     let uri = world.file_id_to_uri(file_id)?;
     Ok(req::PublishDecorationsParams {
         uri,
-        decorations: highlight(&world, file_id),
+        decorations: highlight(&world, file_id)?,
     })
 }
 
-fn highlight(world: &ServerWorld, file_id: FileId) -> Vec<Decoration> {
+fn highlight(world: &ServerWorld, file_id: FileId) -> Result<Vec<Decoration>> {
     let line_index = world.analysis().file_line_index(file_id);
-    world
+    let res = world
         .analysis()
-        .highlight(file_id)
+        .highlight(file_id)?
         .into_iter()
         .map(|h| Decoration {
             range: h.range.conv_with(&line_index),
             tag: h.tag,
         })
-        .collect()
+        .collect();
+    Ok(res)
 }
diff --git a/crates/ra_lsp_server/src/main_loop/mod.rs b/crates/ra_lsp_server/src/main_loop/mod.rs
index 165f2e78f..b35ebd38b 100644
--- a/crates/ra_lsp_server/src/main_loop/mod.rs
+++ b/crates/ra_lsp_server/src/main_loop/mod.rs
@@ -8,9 +8,9 @@ use gen_lsp_server::{
     handle_shutdown, ErrorCode, RawMessage, RawNotification, RawRequest, RawResponse,
 };
 use languageserver_types::NumberOrString;
-use ra_analysis::{FileId, JobHandle, JobToken, LibraryData};
+use ra_analysis::{FileId, LibraryData};
 use rayon::{self, ThreadPool};
-use rustc_hash::FxHashMap;
+use rustc_hash::FxHashSet;
 use serde::{de::DeserializeOwned, Serialize};
 
 use crate::{
@@ -47,7 +47,7 @@ pub fn main_loop(
     info!("server initialized, serving requests");
     let mut state = ServerWorldState::new();
 
-    let mut pending_requests = FxHashMap::default();
+    let mut pending_requests = FxHashSet::default();
     let mut subs = Subscriptions::new();
     let main_res = main_loop_inner(
         internal_mode,
@@ -92,7 +92,7 @@ fn main_loop_inner(
     fs_worker: Worker<PathBuf, (PathBuf, Vec<FileEvent>)>,
     ws_worker: Worker<PathBuf, Result<CargoWorkspace>>,
     state: &mut ServerWorldState,
-    pending_requests: &mut FxHashMap<u64, JobHandle>,
+    pending_requests: &mut FxHashSet<u64>,
     subs: &mut Subscriptions,
 ) -> Result<()> {
     let (libdata_sender, libdata_receiver) = unbounded();
@@ -204,14 +204,13 @@ fn main_loop_inner(
 fn on_task(
     task: Task,
     msg_sender: &Sender<RawMessage>,
-    pending_requests: &mut FxHashMap<u64, JobHandle>,
+    pending_requests: &mut FxHashSet<u64>,
 ) {
     match task {
         Task::Respond(response) => {
-            if let Some(handle) = pending_requests.remove(&response.id) {
-                assert!(handle.has_completed());
+            if pending_requests.remove(&response.id) {
+                msg_sender.send(RawMessage::Response(response))
             }
-            msg_sender.send(RawMessage::Response(response))
         }
         Task::Notify(n) => msg_sender.send(RawMessage::Notification(n)),
     }
@@ -219,7 +218,7 @@ fn on_task(
 
 fn on_request(
     world: &mut ServerWorldState,
-    pending_requests: &mut FxHashMap<u64, JobHandle>,
+    pending_requests: &mut FxHashSet<u64>,
     pool: &ThreadPool,
     sender: &Sender<Task>,
     req: RawRequest,
@@ -253,8 +252,8 @@ fn on_request(
         .on::<req::References>(handlers::handle_references)?
         .finish();
     match req {
-        Ok((id, handle)) => {
-            let inserted = pending_requests.insert(id, handle).is_none();
+        Ok(id) => {
+            let inserted = pending_requests.insert(id);
             assert!(inserted, "duplicate request: {}", id);
             Ok(None)
         }
@@ -265,7 +264,7 @@ fn on_request(
 fn on_notification(
     msg_sender: &Sender<RawMessage>,
     state: &mut ServerWorldState,
-    pending_requests: &mut FxHashMap<u64, JobHandle>,
+    pending_requests: &mut FxHashSet<u64>,
     subs: &mut Subscriptions,
     not: RawNotification,
 ) -> Result<()> {
@@ -277,9 +276,7 @@ fn on_notification(
                     panic!("string id's not supported: {:?}", id);
                 }
             };
-            if let Some(handle) = pending_requests.remove(&id) {
-                handle.cancel();
-            }
+            pending_requests.remove(&id);
             return Ok(());
         }
         Err(not) => not,
@@ -336,7 +333,7 @@ fn on_notification(
 
 struct PoolDispatcher<'a> {
     req: Option<RawRequest>,
-    res: Option<(u64, JobHandle)>,
+    res: Option<u64>,
     pool: &'a ThreadPool,
     world: &'a ServerWorldState,
     sender: &'a Sender<Task>,
@@ -345,7 +342,7 @@ struct PoolDispatcher<'a> {
 impl<'a> PoolDispatcher<'a> {
     fn on<'b, R>(
         &'b mut self,
-        f: fn(ServerWorld, R::Params, JobToken) -> Result<R::Result>,
+        f: fn(ServerWorld, R::Params) -> Result<R::Result>,
     ) -> Result<&'b mut Self>
     where
         R: req::Request,
@@ -358,11 +355,10 @@ impl<'a> PoolDispatcher<'a> {
         };
         match req.cast::<R>() {
             Ok((id, params)) => {
-                let (handle, token) = JobHandle::new();
                 let world = self.world.snapshot();
                 let sender = self.sender.clone();
                 self.pool.spawn(move || {
-                    let resp = match f(world, params, token) {
+                    let resp = match f(world, params) {
                         Ok(resp) => RawResponse::ok::<R>(id, &resp),
                         Err(e) => {
                             RawResponse::err(id, ErrorCode::InternalError as i32, e.to_string())
@@ -371,14 +367,14 @@ impl<'a> PoolDispatcher<'a> {
                     let task = Task::Respond(resp);
                     sender.send(task);
                 });
-                self.res = Some((id, handle));
+                self.res = Some(id);
             }
             Err(req) => self.req = Some(req),
         }
         Ok(self)
     }
 
-    fn finish(&mut self) -> ::std::result::Result<(u64, JobHandle), RawRequest> {
+    fn finish(&mut self) -> ::std::result::Result<u64, RawRequest> {
         match (self.res.take(), self.req.take()) {
             (Some(res), None) => Ok(res),
             (None, Some(req)) => Err(req),
diff --git a/crates/ra_lsp_server/src/path_map.rs b/crates/ra_lsp_server/src/path_map.rs
index 585013acd..d32829382 100644
--- a/crates/ra_lsp_server/src/path_map.rs
+++ b/crates/ra_lsp_server/src/path_map.rs
@@ -1,9 +1,9 @@
+use std::path::{Component, Path, PathBuf};
+
 use im;
 use ra_analysis::{FileId, FileResolver};
 use relative_path::RelativePath;
 
-use std::path::{Component, Path, PathBuf};
-
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Root {
     Workspace,
diff --git a/crates/ra_lsp_server/src/project_model.rs b/crates/ra_lsp_server/src/project_model.rs
index d170ceb73..04e2ef9c8 100644
--- a/crates/ra_lsp_server/src/project_model.rs
+++ b/crates/ra_lsp_server/src/project_model.rs
@@ -1,9 +1,9 @@
+use std::path::{Path, PathBuf};
+
 use cargo_metadata::{metadata_run, CargoOpt};
 use ra_syntax::SmolStr;
 use rustc_hash::{FxHashMap, FxHashSet};
 
-use std::path::{Path, PathBuf};
-
 use crate::{
     thread_watcher::{ThreadWatcher, Worker},
     Result,
diff --git a/crates/ra_lsp_server/src/thread_watcher.rs b/crates/ra_lsp_server/src/thread_watcher.rs
index 67952eb74..51b35fa66 100644
--- a/crates/ra_lsp_server/src/thread_watcher.rs
+++ b/crates/ra_lsp_server/src/thread_watcher.rs
@@ -1,8 +1,9 @@
-use crate::Result;
+use std::thread;
+
 use crossbeam_channel::{bounded, unbounded, Receiver, Sender};
 use drop_bomb::DropBomb;
 
-use std::thread;
+use crate::Result;
 
 pub struct Worker<I, O> {
     pub inp: Sender<I>,