17 files changed, 106 insertions, 80 deletions

diff --git a/crates/ra_analysis/src/imp.rs b/crates/ra_analysis/src/imp.rs
index 03d17de0d..0de0e2645 100644
--- a/crates/ra_analysis/src/imp.rs
+++ b/crates/ra_analysis/src/imp.rs
@@ -10,7 +10,7 @@ use ra_syntax::{
     SyntaxKind::*,
     SyntaxNodeRef, TextRange, TextUnit,
 };
-use ra_db::{FilesDatabase, SourceRoot, SourceRootId, WORKSPACE, SyntaxDatabase, SourceFileQuery};
+use ra_db::{FilesDatabase, SourceRoot, SourceRootId, WORKSPACE, SyntaxDatabase};
 use rayon::prelude::*;
 use rustc_hash::FxHashSet;
 use salsa::{Database, ParallelDatabase};
@@ -43,6 +43,7 @@ impl AnalysisHostImpl {
     }
     pub fn apply_change(&mut self, change: AnalysisChange) {
         log::info!("apply_change {:?}", change);
+        // self.gc_syntax_trees();
 
         for (file_id, text) in change.files_changed {
             self.db
@@ -115,6 +116,25 @@ impl AnalysisHostImpl {
                 .set((), Arc::new(crate_graph))
         }
     }
+
+    #[allow(unused)]
+    /// Ideally, we should call this function from time to time to collect heavy
+    /// syntax trees. However, if we actually do that, everything is recomputed
+    /// for some reason. Needs investigation.
+    fn gc_syntax_trees(&mut self) {
+        self.db
+            .query(ra_db::SourceFileQuery)
+            .sweep(salsa::SweepStrategy::default().discard_values());
+        self.db
+            .query(hir::db::FnSyntaxQuery)
+            .sweep(salsa::SweepStrategy::default().discard_values());
+        self.db
+            .query(hir::db::SourceFileItemsQuery)
+            .sweep(salsa::SweepStrategy::default().discard_values());
+        self.db
+            .query(hir::db::FileItemQuery)
+            .sweep(salsa::SweepStrategy::default().discard_values());
+    }
 }
 
 pub(crate) struct AnalysisImpl {
@@ -160,9 +180,6 @@ impl AnalysisImpl {
                 .filter_map(|it| it.ok())
                 .collect()
         };
-        self.db
-            .query(SourceFileQuery)
-            .sweep(salsa::SweepStrategy::default().discard_values());
         Ok(query.search(&buf))
     }
     /// This returns `Vec` because a module may be included from several places. We
diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs
index ff41fd326..62cf9ab17 100644
--- a/crates/ra_hir/src/db.rs
+++ b/crates/ra_hir/src/db.rs
@@ -27,20 +27,16 @@ pub trait HirDatabase: SyntaxDatabase
     }
     fn fn_syntax(fn_id: FnId) -> FnDefNode {
         type FnSyntaxQuery;
-        // Don't retain syntax trees in memory
-        storage dependencies;
         use fn query_definitions::fn_syntax;
     }
 
     fn file_items(file_id: FileId) -> Arc<SourceFileItems> {
         type SourceFileItemsQuery;
-        storage dependencies;
         use fn query_definitions::file_items;
     }
 
     fn file_item(source_item_id: SourceItemId) -> SyntaxNode {
         type FileItemQuery;
-        storage dependencies;
         use fn query_definitions::file_item;
     }
 
diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs
index 760524f6b..5941a9ea3 100644
--- a/crates/ra_hir/src/lib.rs
+++ b/crates/ra_hir/src/lib.rs
@@ -109,7 +109,8 @@ pub(crate) type SourceFileItemId = Id<SyntaxNode>;
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct SourceItemId {
     file_id: FileId,
-    item_id: SourceFileItemId,
+    /// None for the whole file.
+    item_id: Option<SourceFileItemId>,
 }
 
 /// Maps item's `SyntaxNode`s to `SourceFileItemId` and back.
@@ -143,11 +144,14 @@ impl SourceFileItems {
             return id;
         }
         // This should not happen. Let's try to give a sensible diagnostics.
-        if let Some((_, i)) = self.arena.iter().find(|(_id, i)| i.range() == item.range()) {
-            panic!(
+        if let Some((id, i)) = self.arena.iter().find(|(_id, i)| i.range() == item.range()) {
+            // FIXME(#288): whyyy are we getting here?
+            log::error!(
                 "unequal syntax nodes with the same range:\n{:?}\n{:?}",
-                item, i
-            )
+                item,
+                i
+            );
+            return id;
         }
         panic!(
             "Can't find {:?} in SourceFileItems:\n{:?}",
diff --git a/crates/ra_hir/src/module.rs b/crates/ra_hir/src/module.rs
index c6bb76d56..d5866f6ef 100644
--- a/crates/ra_hir/src/module.rs
+++ b/crates/ra_hir/src/module.rs
@@ -271,15 +271,13 @@ pub struct ModuleData {
 
 impl ModuleSource {
     // precondition: item_id **must** point to module
-    fn new(file_id: FileId, item_id: SourceFileItemId) -> ModuleSource {
+    fn new(file_id: FileId, item_id: Option<SourceFileItemId>) -> ModuleSource {
         let source_item_id = SourceItemId { file_id, item_id };
         ModuleSource(source_item_id)
     }
 
-    pub(crate) fn new_file(db: &impl HirDatabase, file_id: FileId) -> ModuleSource {
-        let file_items = db.file_items(file_id);
-        let item_id = file_items.id_of_source_file();
-        ModuleSource::new(file_id, item_id)
+    pub(crate) fn new_file(file_id: FileId) -> ModuleSource {
+        ModuleSource::new(file_id, None)
     }
 
     pub(crate) fn new_inline(
@@ -290,7 +288,7 @@ impl ModuleSource {
         assert!(!m.has_semi());
         let file_items = db.file_items(file_id);
         let item_id = file_items.id_of(file_id, m.syntax());
-        ModuleSource::new(file_id, item_id)
+        ModuleSource::new(file_id, Some(item_id))
     }
 
     pub fn file_id(self) -> FileId {
diff --git a/crates/ra_hir/src/module/imp.rs b/crates/ra_hir/src/module/imp.rs
index 9f144e139..4a19842c4 100644
--- a/crates/ra_hir/src/module/imp.rs
+++ b/crates/ra_hir/src/module/imp.rs
@@ -66,7 +66,7 @@ fn create_module_tree<'a>(
 
     let source_root = db.source_root(source_root);
     for &file_id in source_root.files.iter() {
-        let source = ModuleSource::new_file(db, file_id);
+        let source = ModuleSource::new_file(file_id);
         if visited.contains(&source) {
             continue; // TODO: use explicit crate_roots here
         }
@@ -126,7 +126,7 @@ fn build_subtree(
                             visited,
                             roots,
                             Some(link),
-                            ModuleSource::new_file(db, file_id),
+                            ModuleSource::new_file(file_id),
                         ),
                     })
                     .collect::<Cancelable<Vec<_>>>()?;
diff --git a/crates/ra_hir/src/module/nameres.rs b/crates/ra_hir/src/module/nameres.rs
index 9afeade9e..5540b827f 100644
--- a/crates/ra_hir/src/module/nameres.rs
+++ b/crates/ra_hir/src/module/nameres.rs
@@ -98,7 +98,7 @@ impl NamedImport {
     pub fn range(&self, db: &impl HirDatabase, file_id: FileId) -> TextRange {
         let source_item_id = SourceItemId {
             file_id,
-            item_id: self.file_item_id,
+            item_id: Some(self.file_item_id),
         };
         let syntax = db.file_item(source_item_id);
         let offset = syntax.borrowed().range().start();
@@ -281,7 +281,7 @@ where
                 module_id,
                 source_item_id: SourceItemId {
                     file_id,
-                    item_id: item.id,
+                    item_id: Some(item.id),
                 },
             };
             let def_id = def_loc.id(self.db);
diff --git a/crates/ra_hir/src/query_definitions.rs b/crates/ra_hir/src/query_definitions.rs
index 37c4f9e4f..efaeb1525 100644
--- a/crates/ra_hir/src/query_definitions.rs
+++ b/crates/ra_hir/src/query_definitions.rs
@@ -38,7 +38,6 @@ pub(super) fn fn_scopes(db: &impl HirDatabase, fn_id: FnId) -> Arc<FnScopes> {
 pub(super) fn file_items(db: &impl HirDatabase, file_id: FileId) -> Arc<SourceFileItems> {
     let mut res = SourceFileItems::new(file_id);
     let source_file = db.source_file(file_id);
-    res.alloc(source_file.syntax().owned());
     let source_file = source_file.borrowed();
     source_file
         .syntax()
@@ -52,7 +51,10 @@ pub(super) fn file_items(db: &impl HirDatabase, file_id: FileId) -> Arc<SourceFi
 }
 
 pub(super) fn file_item(db: &impl HirDatabase, source_item_id: SourceItemId) -> SyntaxNode {
-    db.file_items(source_item_id.file_id)[source_item_id.item_id].clone()
+    match source_item_id.item_id {
+        Some(id) => db.file_items(source_item_id.file_id)[id].clone(),
+        None => db.source_file(source_item_id.file_id).syntax().owned(),
+    }
 }
 
 pub(crate) fn submodules(
diff --git a/crates/ra_hir/src/source_binder.rs b/crates/ra_hir/src/source_binder.rs
index 0c16ccc24..ce2a0f2e8 100644
--- a/crates/ra_hir/src/source_binder.rs
+++ b/crates/ra_hir/src/source_binder.rs
@@ -20,7 +20,7 @@ use crate::{
 
 /// Locates the module by `FileId`. Picks topmost module in the file.
 pub fn module_from_file_id(db: &impl HirDatabase, file_id: FileId) -> Cancelable<Option<Module>> {
-    let module_source = ModuleSource::new_file(db, file_id);
+    let module_source = ModuleSource::new_file(file_id);
     module_from_source(db, module_source)
 }
 
@@ -32,7 +32,7 @@ pub fn module_from_position(
     let file = db.source_file(position.file_id);
     let module_source = match find_node_at_offset::<ast::Module>(file.syntax(), position.offset) {
         Some(m) if !m.has_semi() => ModuleSource::new_inline(db, position.file_id, m),
-        _ => ModuleSource::new_file(db, position.file_id),
+        _ => ModuleSource::new_file(position.file_id),
     };
     module_from_source(db, module_source)
 }
@@ -50,7 +50,7 @@ pub fn module_from_child_node(
     {
         ModuleSource::new_inline(db, file_id, m)
     } else {
-        ModuleSource::new_file(db, file_id)
+        ModuleSource::new_file(file_id)
     };
     module_from_source(db, module_source)
 }
@@ -76,7 +76,10 @@ pub fn function_from_source(
     let module = ctry!(module_from_child_node(db, file_id, fn_def.syntax())?);
     let file_items = db.file_items(file_id);
     let item_id = file_items.id_of(file_id, fn_def.syntax());
-    let source_item_id = SourceItemId { file_id, item_id };
+    let source_item_id = SourceItemId {
+        file_id,
+        item_id: Some(item_id),
+    };
     let def_loc = DefLoc {
         kind: DefKind::Function,
         source_root_id: module.source_root_id,
diff --git a/crates/ra_lsp_server/Cargo.toml b/crates/ra_lsp_server/Cargo.toml
index 133decc52..30a8d35cd 100644
--- a/crates/ra_lsp_server/Cargo.toml
+++ b/crates/ra_lsp_server/Cargo.toml
@@ -26,6 +26,7 @@ text_unit = { version = "0.1.2", features = ["serde"] }
 smol_str = { version = "0.1.5", features = ["serde"] }
 rustc-hash = "1.0"
 
+thread_worker = { path = "../thread_worker" }
 ra_syntax = { path = "../ra_syntax" }
 ra_editor = { path = "../ra_editor" }
 ra_text_edit = { path = "../ra_text_edit" }
diff --git a/crates/ra_lsp_server/src/lib.rs b/crates/ra_lsp_server/src/lib.rs
index 75c6fa1b8..1d7258c35 100644
--- a/crates/ra_lsp_server/src/lib.rs
+++ b/crates/ra_lsp_server/src/lib.rs
@@ -5,7 +5,6 @@ mod path_map;
 mod project_model;
 pub mod req;
 mod server_world;
-pub mod thread_watcher;
 mod vfs;
 
 pub type Result<T> = ::std::result::Result<T, ::failure::Error>;
diff --git a/crates/ra_lsp_server/src/main.rs b/crates/ra_lsp_server/src/main.rs
index 9ba972562..4497980e5 100644
--- a/crates/ra_lsp_server/src/main.rs
+++ b/crates/ra_lsp_server/src/main.rs
@@ -6,11 +6,11 @@ use ra_lsp_server::Result;
 
 fn main() -> Result<()> {
     ::std::env::set_var("RUST_BACKTRACE", "short");
-    Logger::with_env_or_str("error")
-        .duplicate_to_stderr(Duplicate::All)
-        .log_to_file()
-        .directory("log")
-        .start()?;
+    let logger = Logger::with_env_or_str("error").duplicate_to_stderr(Duplicate::All);
+    match ::std::env::var("RA_INTERNAL_MODE") {
+        Ok(ref v) if v == "1" => logger.log_to_file().directory("log").start()?,
+        _ => logger.start()?,
+    };
     log::info!("lifecycle: server started");
     match ::std::panic::catch_unwind(main_inner) {
         Ok(res) => {
diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs
index 41f70f263..eab82ee85 100644
--- a/crates/ra_lsp_server/src/main_loop.rs
+++ b/crates/ra_lsp_server/src/main_loop.rs
@@ -10,6 +10,7 @@ use gen_lsp_server::{
 use languageserver_types::NumberOrString;
 use ra_analysis::{Canceled, FileId, LibraryData};
 use rayon;
+use thread_worker::Worker;
 use threadpool::ThreadPool;
 use rustc_hash::FxHashSet;
 use serde::{de::DeserializeOwned, Serialize};
@@ -21,7 +22,6 @@ use crate::{
     project_model::{workspace_loader, CargoWorkspace},
     req,
     server_world::{ServerWorld, ServerWorldState},
-    thread_watcher::Worker,
     vfs::{self, FileEvent},
     Result,
 };
@@ -92,8 +92,8 @@ pub fn main_loop(
     let ws_res = ws_watcher.stop();
 
     main_res?;
-    fs_res?;
-    ws_res?;
+    fs_res.map_err(|_| format_err!("fs watcher died"))?;
+    ws_res.map_err(|_| format_err!("ws watcher died"))?;
 
     Ok(())
 }
diff --git a/crates/ra_lsp_server/src/project_model.rs b/crates/ra_lsp_server/src/project_model.rs
index cb91ada90..b881f8b6f 100644
--- a/crates/ra_lsp_server/src/project_model.rs
+++ b/crates/ra_lsp_server/src/project_model.rs
@@ -4,11 +4,9 @@ use cargo_metadata::{metadata_run, CargoOpt};
 use ra_syntax::SmolStr;
 use rustc_hash::{FxHashMap, FxHashSet};
 use failure::{format_err, bail};
+use thread_worker::{WorkerHandle, Worker};
 
-use crate::{
-    Result,
-    thread_watcher::{ThreadWatcher, Worker},
-};
+use crate::Result;
 
 /// `CargoWorksapce` represents the logical structure of, well, a Cargo
 /// workspace. It pretty closely mirrors `cargo metadata` output.
@@ -199,8 +197,8 @@ impl TargetKind {
     }
 }
 
-pub fn workspace_loader() -> (Worker<PathBuf, Result<CargoWorkspace>>, ThreadWatcher) {
-    Worker::<PathBuf, Result<CargoWorkspace>>::spawn(
+pub fn workspace_loader() -> (Worker<PathBuf, Result<CargoWorkspace>>, WorkerHandle) {
+    thread_worker::spawn::<PathBuf, Result<CargoWorkspace>, _>(
         "workspace loader",
         1,
         |input_receiver, output_sender| {
diff --git a/crates/ra_lsp_server/src/vfs.rs b/crates/ra_lsp_server/src/vfs.rs
index 00ab3e6c3..fcf7693d8 100644
--- a/crates/ra_lsp_server/src/vfs.rs
+++ b/crates/ra_lsp_server/src/vfs.rs
@@ -4,8 +4,7 @@ use std::{
 };
 
 use walkdir::WalkDir;
-
-use crate::thread_watcher::{ThreadWatcher, Worker};
+use thread_worker::{WorkerHandle, Worker};
 
 #[derive(Debug)]
 pub struct FileEvent {
@@ -18,8 +17,8 @@ pub enum FileEventKind {
     Add(String),
 }
 
-pub fn roots_loader() -> (Worker<PathBuf, (PathBuf, Vec<FileEvent>)>, ThreadWatcher) {
-    Worker::<PathBuf, (PathBuf, Vec<FileEvent>)>::spawn(
+pub fn roots_loader() -> (Worker<PathBuf, (PathBuf, Vec<FileEvent>)>, WorkerHandle) {
+    thread_worker::spawn::<PathBuf, (PathBuf, Vec<FileEvent>), _>(
         "roots loader",
         128,
         |input_receiver, output_sender| {
diff --git a/crates/ra_lsp_server/tests/heavy_tests/support.rs b/crates/ra_lsp_server/tests/heavy_tests/support.rs
index 4b75be3ee..07a878a26 100644
--- a/crates/ra_lsp_server/tests/heavy_tests/support.rs
+++ b/crates/ra_lsp_server/tests/heavy_tests/support.rs
@@ -17,11 +17,11 @@ use languageserver_types::{
 use serde::Serialize;
 use serde_json::{to_string_pretty, Value};
 use tempdir::TempDir;
+use thread_worker::{WorkerHandle, Worker};
 use test_utils::{parse_fixture, find_mismatch};
 
 use ra_lsp_server::{
     main_loop, req,
-    thread_watcher::{ThreadWatcher, Worker},
 };
 
 pub fn project(fixture: &str) -> Server {
@@ -45,13 +45,13 @@ pub struct Server {
     messages: RefCell<Vec<RawMessage>>,
     dir: TempDir,
     worker: Option<Worker<RawMessage, RawMessage>>,
-    watcher: Option<ThreadWatcher>,
+    watcher: Option<WorkerHandle>,
 }
 
 impl Server {
     fn new(dir: TempDir, files: Vec<(PathBuf, String)>) -> Server {
         let path = dir.path().to_path_buf();
-        let (worker, watcher) = Worker::<RawMessage, RawMessage>::spawn(
+        let (worker, watcher) = thread_worker::spawn::<RawMessage, RawMessage, _>(
             "test server",
             128,
             move |mut msg_receiver, mut msg_sender| {
diff --git a/crates/thread_worker/Cargo.toml b/crates/thread_worker/Cargo.toml
new file mode 100644
index 000000000..62d66a1a3
--- /dev/null
+++ b/crates/thread_worker/Cargo.toml
@@ -0,0 +1,11 @@
+[package]
+edition = "2018"
+name = "thread_worker"
+version = "0.1.0"
+authors = ["Aleksey Kladov <[email protected]>"]
+
+[dependencies]
+drop_bomb = "0.1.0"
+crossbeam-channel = "0.2.4"
+log = "0.4.3"
+
diff --git a/crates/ra_lsp_server/src/thread_watcher.rs b/crates/thread_worker/src/lib.rs
index 99825d440..e558559ef 100644
--- a/crates/ra_lsp_server/src/thread_watcher.rs
+++ b/crates/thread_worker/src/lib.rs
@@ -1,28 +1,35 @@
+//! Small utility to correctly spawn crossbeam-channel based worker threads.
+
 use std::thread;
 
 use crossbeam_channel::{bounded, unbounded, Receiver, Sender};
 use drop_bomb::DropBomb;
-use failure::format_err;
-
-use crate::Result;
 
 pub struct Worker<I, O> {
     pub inp: Sender<I>,
     pub out: Receiver<O>,
 }
 
-impl<I, O> Worker<I, O> {
-    pub fn spawn<F>(name: &'static str, buf: usize, f: F) -> (Self, ThreadWatcher)
-    where
-        F: FnOnce(Receiver<I>, Sender<O>) + Send + 'static,
-        I: Send + 'static,
-        O: Send + 'static,
-    {
-        let (worker, inp_r, out_s) = worker_chan(buf);
-        let watcher = ThreadWatcher::spawn(name, move || f(inp_r, out_s));
-        (worker, watcher)
-    }
+pub struct WorkerHandle {
+    name: &'static str,
+    thread: thread::JoinHandle<()>,
+    bomb: DropBomb,
+}
 
+pub fn spawn<I, O, F>(name: &'static str, buf: usize, f: F) -> (Worker<I, O>, WorkerHandle)
+where
+    F: FnOnce(Receiver<I>, Sender<O>) + Send + 'static,
+    I: Send + 'static,
+    O: Send + 'static,
+{
+    let (worker, inp_r, out_s) = worker_chan(buf);
+    let watcher = WorkerHandle::spawn(name, move || f(inp_r, out_s));
+    (worker, watcher)
+}
+
+impl<I, O> Worker<I, O> {
+    /// Stops the worker. Returns the message receiver to fetch results which
+    /// have become ready before the worker is stopped.
     pub fn stop(self) -> Receiver<O> {
         self.out
     }
@@ -32,30 +39,21 @@ impl<I, O> Worker<I, O> {
     }
 }
 
-pub struct ThreadWatcher {
-    name: &'static str,
-    thread: thread::JoinHandle<()>,
-    bomb: DropBomb,
-}
-
-impl ThreadWatcher {
-    fn spawn(name: &'static str, f: impl FnOnce() + Send + 'static) -> ThreadWatcher {
+impl WorkerHandle {
+    fn spawn(name: &'static str, f: impl FnOnce() + Send + 'static) -> WorkerHandle {
         let thread = thread::spawn(f);
-        ThreadWatcher {
+        WorkerHandle {
             name,
             thread,
-            bomb: DropBomb::new(format!("ThreadWatcher {} was not stopped", name)),
+            bomb: DropBomb::new(format!("WorkerHandle {} was not stopped", name)),
         }
     }
 
-    pub fn stop(mut self) -> Result<()> {
+    pub fn stop(mut self) -> thread::Result<()> {
         log::info!("waiting for {} to finish ...", self.name);
         let name = self.name;
         self.bomb.defuse();
-        let res = self
-            .thread
-            .join()
-            .map_err(|_| format_err!("ThreadWatcher {} died", name));
+        let res = self.thread.join();
         match &res {
             Ok(()) => log::info!("... {} terminated with ok", name),
             Err(_) => log::error!("... {} terminated with err", name),