Repository: rust-analyzer/rust-analyzer
Pull request #5063 (2020-06-25 15:01:25)

Store field/variant attrs in ItemTree and use it for adt.rs queries


Patch:
diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs
index 9e8205ae79..3e73cf6fff 100644
--- a/crates/flycheck/src/lib.rs
+++ b/crates/flycheck/src/lib.rs
@@ -10,7 +10,7 @@ use std::{
     time::Instant,
 };
 
-use crossbeam_channel::{never, select, unbounded, Receiver, Sender};
+use crossbeam_channel::{never, select, unbounded, Receiver, RecvError, Sender};
 
 pub use cargo_metadata::diagnostic::{
     Applicability, Diagnostic, DiagnosticLevel, DiagnosticSpan, DiagnosticSpanMacroExpansion,
@@ -61,7 +61,7 @@ impl FlycheckHandle {
     ) -> FlycheckHandle {
         let (cmd_send, cmd_recv) = unbounded::<Restart>();
         let handle = jod_thread::spawn(move || {
-            FlycheckActor::new(sender, config, workspace_root).run(cmd_recv);
+            FlycheckActor::new(sender, config, workspace_root).run(&cmd_recv);
         });
         FlycheckHandle { cmd_send, handle }
     }
@@ -98,18 +98,14 @@ struct FlycheckActor {
     config: FlycheckConfig,
     workspace_root: PathBuf,
     last_update_req: Option<Instant>,
+    // XXX: drop order is significant
+    message_recv: Receiver<CheckEvent>,
     /// WatchThread exists to wrap around the communication needed to be able to
     /// run `cargo check` without blocking. Currently the Rust standard library
     /// doesn't provide a way to read sub-process output without blocking, so we
     /// have to wrap sub-processes output handling in a thread and pass messages
     /// back over a channel.
-    // XXX: drop order is significant
-    check_process: Option<(Receiver<CheckEvent>, jod_thread::JoinHandle)>,
-}
-
-enum Event {
-    Restart(Restart),
-    CheckEvent(Option<CheckEvent>),
+    check_process: Option<jod_thread::JoinHandle>,
 }
 
 impl FlycheckActor {
@@ -118,48 +114,40 @@ impl FlycheckActor {
         config: FlycheckConfig,
         workspace_root: PathBuf,
     ) -> FlycheckActor {
-        FlycheckActor { sender, config, workspace_root, last_update_req: None, check_process: None }
+        FlycheckActor {
+            sender,
+            config,
+            workspace_root,
+            last_update_req: None,
+            message_recv: never(),
+            check_process: None,
+        }
     }
 
-    fn run(&mut self, inbox: Receiver<Restart>) {
+    fn run(&mut self, cmd_recv: &Receiver<Restart>) {
         // If we rerun the thread, we need to discard the previous check results first
-        self.send(Message::ClearDiagnostics);
-        self.send(Message::Progress(Progress::End));
-
-        while let Some(event) = self.next_event(&inbox) {
-            match event {
-                Event::Restart(Restart) => self.last_update_req = Some(Instant::now()),
-                Event::CheckEvent(None) => {
-                    // Watcher finished, replace it with a never channel to
-                    // avoid busy-waiting.
-                    self.check_process = None;
+        self.clean_previous_results();
+
+        loop {
+            select! {
+                recv(&cmd_recv) -> cmd => match cmd {
+                    Ok(Restart) => self.last_update_req = Some(Instant::now()),
+                    Err(RecvError) => {
+                        // Command channel has closed, so shut down
+                        break;
+                    },
+                },
+                recv(self.message_recv) -> msg => match msg {
+                    Ok(msg) => self.handle_message(msg),
+                    Err(RecvError) => {
+                        // Watcher finished, replace it with a never channel to
+                        // avoid busy-waiting.
+                        self.message_recv = never();
+                        self.check_process = None;
+                    },
                 }
-                Event::CheckEvent(Some(event)) => match event {
-                    CheckEvent::Begin => {
-                        self.send(Message::Progress(Progress::Being));
-                    }
-
-                    CheckEvent::End => {
-                        self.send(Message::Progress(Progress::End));
-                    }
-
-                    CheckEvent::Msg(cargo_metadata::Message::CompilerArtifact(msg)) => {
-                        self.send(Message::Progress(Progress::DidCheckCrate(msg.target.name)));
-                    }
-
-                    CheckEvent::Msg(cargo_metadata::Message::CompilerMessage(msg)) => {
-                        self.send(Message::AddDiagnostic {
-                            workspace_root: self.workspace_root.clone(),
-                            diagnostic: msg.message,
-                        });
-                    }
+            };
 
-                    CheckEvent::Msg(cargo_metadata::Message::BuildScriptExecuted(_))
-                    | CheckEvent::Msg(cargo_metadata::Message::BuildFinished(_))
-                    | CheckEvent::Msg(cargo_metadata::Message::TextLine(_))
-                    | CheckEvent::Msg(cargo_metadata::Message::Unknown) => {}
-                },
-            }
             if self.should_recheck() {
                 self.last_update_req = None;
                 self.send(Message::ClearDiagnostics);
@@ -168,12 +156,9 @@ impl FlycheckActor {
         }
     }
 
-    fn next_event(&self, inbox: &Receiver<Restart>) -> Option<Event> {
-        let check_chan = self.check_process.as_ref().map(|(chan, _thread)| chan);
-        select! {
-            recv(inbox) -> msg => msg.ok().map(Event::Restart),
-            recv(check_chan.unwrap_or(&never())) -> msg => Some(Event::CheckEvent(msg.ok())),
-        }
+    fn clean_previous_results(&self) {
+        self.send(Message::ClearDiagnostics);
+        self.send(Message::Progress(Progress::End));
     }
 
     fn should_recheck(&mut self) -> bool {
@@ -186,8 +171,37 @@ impl FlycheckActor {
         false
     }
 
+    fn handle_message(&self, msg: CheckEvent) {
+        match msg {
+            CheckEvent::Begin => {
+                self.send(Message::Progress(Progress::Being));
+            }
+
+            CheckEvent::End => {
+                self.send(Message::Progress(Progress::End));
+            }
+
+            CheckEvent::Msg(cargo_metadata::Message::CompilerArtifact(msg)) => {
+                self.send(Message::Progress(Progress::DidCheckCrate(msg.target.name)));
+            }
+
+            CheckEvent::Msg(cargo_metadata::Message::CompilerMessage(msg)) => {
+                self.send(Message::AddDiagnostic {
+                    workspace_root: self.workspace_root.clone(),
+                    diagnostic: msg.message,
+                });
+            }
+
+            CheckEvent::Msg(cargo_metadata::Message::BuildScriptExecuted(_))
+            | CheckEvent::Msg(cargo_metadata::Message::BuildFinished(_))
+            | CheckEvent::Msg(cargo_metadata::Message::TextLine(_))
+            | CheckEvent::Msg(cargo_metadata::Message::Unknown) => {}
+        }
+    }
+
     fn restart_check_process(&mut self) {
         // First, clear and cancel the old thread
+        self.message_recv = never();
         self.check_process = None;
 
         let mut cmd = match &self.config {
@@ -223,7 +237,8 @@ impl FlycheckActor {
         cmd.current_dir(&self.workspace_root);
 
         let (message_send, message_recv) = unbounded();
-        let thread = jod_thread::spawn(move || {
+        self.message_recv = message_recv;
+        self.check_process = Some(jod_thread::spawn(move || {
             // If we trigger an error here, we will do so in the loop instead,
             // which will break out of the loop, and continue the shutdown
             let _ = message_send.send(CheckEvent::Begin);
@@ -252,8 +267,7 @@ impl FlycheckActor {
             // We can ignore any error here, as we are already in the progress
             // of shutting down.
             let _ = message_send.send(CheckEvent::End);
-        });
-        self.check_process = Some((message_recv, thread))
+        }))
     }
 
     fn send(&self, check_task: Message) {
diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs
index 2bc34d449f..4994a21253 100644
--- a/crates/ra_hir_def/src/adt.rs
+++ b/crates/ra_hir_def/src/adt.rs
@@ -8,12 +8,12 @@ use hir_expand::{
     InFile,
 };
 use ra_arena::{map::ArenaMap, Arena};
-use ra_prof::profile;
 use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner};
 
 use crate::{
     body::{CfgExpander, LowerCtx},
     db::DefDatabase,
+    item_tree::{Field, Fields, ItemTree},
     src::HasChildSource,
     src::HasSource,
     trace::Trace,
@@ -22,6 +22,7 @@ use crate::{
     EnumId, HasModule, LocalEnumVariantId, LocalFieldId, Lookup, ModuleId, StructId, UnionId,
     VariantId,
 };
+use ra_cfg::CfgOptions;
 
 /// Note that we use `StructData` for unions as well!
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -59,39 +60,48 @@ pub struct FieldData {
 
 impl StructData {
     pub(crate) fn struct_data_query(db: &dyn DefDatabase, id: StructId) -> Arc<StructData> {
-        let src = id.lookup(db).source(db);
+        let loc = id.lookup(db);
+        let item_tree = db.item_tree(loc.id.file_id);
+        let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone();
 
-        let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
-        let variant_data =
-            VariantData::new(db, src.map(|s| s.kind()), id.lookup(db).container.module(db));
-        let variant_data = Arc::new(variant_data);
-        Arc::new(StructData { name, variant_data })
+        let strukt = &item_tree[loc.id.value];
+        let variant_data = lower_fields(&item_tree, &cfg_options, &strukt.fields);
+
+        Arc::new(StructData { name: strukt.name.clone(), variant_data: Arc::new(variant_data) })
     }
     pub(crate) fn union_data_query(db: &dyn DefDatabase, id: UnionId) -> Arc<StructData> {
-        let src = id.lookup(db).source(db);
-        let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
-        let variant_data = VariantData::new(
-            db,
-            src.map(|s| {
-                s.record_field_def_list()
-                    .map(ast::StructKind::Record)
-                    .unwrap_or(ast::StructKind::Unit)
-            }),
-            id.lookup(db).container.module(db),
-        );
-        let variant_data = Arc::new(variant_data);
-        Arc::new(StructData { name, variant_data })
+        let loc = id.lookup(db);
+        let item_tree = db.item_tree(loc.id.file_id);
+        let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone();
+
+        let union = &item_tree[loc.id.value];
+        let variant_data = lower_fields(&item_tree, &cfg_options, &union.fields);
+
+        Arc::new(StructData { name: union.name.clone(), variant_data: Arc::new(variant_data) })
     }
 }
 
 impl EnumData {
     pub(crate) fn enum_data_query(db: &dyn DefDatabase, e: EnumId) -> Arc<EnumData> {
-        let _p = profile("enum_data_query");
-        let src = e.lookup(db).source(db);
-        let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
-        let mut trace = Trace::new_for_arena();
-        lower_enum(db, &mut trace, &src, e.lookup(db).container.module(db));
-        Arc::new(EnumData { name, variants: trace.into_arena() })
+        let loc = e.lookup(db);
+        let item_tree = db.item_tree(loc.id.file_id);
+        let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone();
+
+        let enum_ = &item_tree[loc.id.value];
+        let mut variants = Arena::new();
+        for var_id in enum_.variants.clone() {
+            if item_tree.attrs(var_id.into()).is_cfg_enabled(&cfg_options) {
+                let var = &item_tree[var_id];
+                let var_data = lower_fields(&item_tree, &cfg_options, &var.fields);
+
+                variants.alloc(EnumVariantData {
+                    name: var.name.clone(),
+                    variant_data: Arc::new(var_data),
+                });
+            }
+        }
+
+        Arc::new(EnumData { name: enum_.name.clone(), variants })
     }
 
     pub fn variant(&self, name: &Name) -> Option<LocalEnumVariantId> {
@@ -251,3 +261,35 @@ fn lower_struct(
         ast::StructKind::Unit => StructKind::Unit,
     }
 }
+
+fn lower_fields(item_tree: &ItemTree, cfg_options: &CfgOptions, fields: &Fields) -> VariantData {
+    match fields {
+        Fields::Record(flds) => {
+            let mut arena = Arena::new();
+            for field_id in flds.clone() {
+                if item_tree.attrs(field_id.into()).is_cfg_enabled(cfg_options) {
+                    arena.alloc(lower_field(item_tree, &item_tree[field_id]));
+                }
+            }
+            VariantData::Record(arena)
+        }
+        Fields::Tuple(flds) => {
+            let mut arena = Arena::new();
+            for field_id in flds.clone() {
+                if item_tree.attrs(field_id.into()).is_cfg_enabled(cfg_options) {
+                    arena.alloc(lower_field(item_tree, &item_tree[field_id]));
+                }
+            }
+            VariantData::Tuple(arena)
+        }
+        Fields::Unit => VariantData::Unit,
+    }
+}
+
+fn lower_field(item_tree: &ItemTree, field: &Field) -> FieldData {
+    FieldData {
+        name: field.name.clone(),
+        type_ref: field.type_ref.clone(),
+        visibility: item_tree[field.visibility].clone(),
+    }
+}
diff --git a/crates/ra_hir_def/src/attr.rs b/crates/ra_hir_def/src/attr.rs
index 197737ffce..e228e2145b 100644
--- a/crates/ra_hir_def/src/attr.rs
+++ b/crates/ra_hir_def/src/attr.rs
@@ -208,5 +208,5 @@ where
 fn attrs_from_item_tree<N: ItemTreeNode>(id: ItemTreeId<N>, db: &dyn DefDatabase) -> Attrs {
     let tree = db.item_tree(id.file_id);
     let mod_item = N::id_to_mod_item(id.value);
-    tree.attrs(mod_item).clone()
+    tree.attrs(mod_item.into()).clone()
 }
diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs
index f9e5701db2..282ade2a3a 100644
--- a/crates/ra_hir_def/src/data.rs
+++ b/crates/ra_hir_def/src/data.rs
@@ -40,7 +40,7 @@ impl FunctionData {
             name: func.name.clone(),
             params: func.params.to_vec(),
             ret_type: func.ret_type.clone(),
-            attrs: item_tree.attrs(loc.id.value.into()).clone(),
+            attrs: item_tree.attrs(ModItem::from(loc.id.value).into()).clone(),
             has_self_param: func.has_self_param,
             is_unsafe: func.is_unsafe,
             visibility: item_tree[func.visibility].clone(),
@@ -224,7 +224,7 @@ fn collect_items(
         match item {
             AssocItem::Function(id) => {
                 let item = &item_tree[id];
-                let attrs = item_tree.attrs(id.into());
+                let attrs = item_tree.attrs(ModItem::from(id).into());
                 if !attrs.is_cfg_enabled(&cfg_options) {
                     continue;
                 }
diff --git a/crates/ra_hir_def/src/item_tree.rs b/crates/ra_hir_def/src/item_tree.rs
index d7bc64e6ca..3e603bd558 100644
--- a/crates/ra_hir_def/src/item_tree.rs
+++ b/crates/ra_hir_def/src/item_tree.rs
@@ -5,6 +5,7 @@ mod lower;
 mod tests;
 
 use std::{
+    any::type_name,
     fmt::{self, Debug},
     hash::{Hash, Hasher},
     marker::PhantomData,
@@ -178,8 +179,8 @@ impl ItemTree {
         self.attrs.get(&AttrOwner::TopLevel).unwrap_or(&Attrs::EMPTY)
     }
 
-    pub fn attrs(&self, of: ModItem) -> &Attrs {
-        self.attrs.get(&AttrOwner::ModItem(of)).unwrap_or(&Attrs::EMPTY)
+    pub fn attrs(&self, of: AttrOwner) -> &Attrs {
+        self.attrs.get(&of).unwrap_or(&Attrs::EMPTY)
     }
 
     /// Returns the lowered inner items that `ast` corresponds to.
@@ -282,15 +283,32 @@ struct ItemTreeData {
 }
 
 #[derive(Debug, Eq, PartialEq, Hash)]
-enum AttrOwner {
+pub enum AttrOwner {
     /// Attributes on an item.
     ModItem(ModItem),
     /// Inner attributes of the source file.
     TopLevel,
+
+    Variant(Idx<Variant>),
+    Field(Idx<Field>),
     // FIXME: Store variant and field attrs, and stop reparsing them in `attrs_query`.
 }
 
-/// Trait implemented by all nodes in the item tree.
+macro_rules! from_attrs {
+    ( $( $var:ident($t:ty) ),+ ) => {
+        $(
+            impl From<$t> for AttrOwner {
+                fn from(t: $t) -> AttrOwner {
+                    AttrOwner::$var(t)
+                }
+            }
+        )+
+    };
+}
+
+from_attrs!(ModItem(ModItem), Variant(Idx<Variant>), Field(Idx<Field>));
+
+/// Trait implemented by all item nodes in the item tree.
 pub trait ItemTreeNode: Clone {
     type Source: AstNode + Into<ast::ModuleItem>;
 
@@ -523,7 +541,7 @@ pub struct Enum {
     pub name: Name,
     pub visibility: RawVisibilityId,
     pub generic_params: GenericParamsId,
-    pub variants: Range<Idx<Variant>>,
+    pub variants: IdRange<Variant>,
     pub ast_id: FileAstId<ast::EnumDef>,
 }
 
@@ -681,10 +699,48 @@ pub struct Variant {
     pub fields: Fields,
 }
 
+pub struct IdRange<T> {
+    range: Range<u32>,
+    _p: PhantomData<T>,
+}
+
+impl<T> IdRange<T> {
+    fn new(range: Range<Idx<T>>) -> Self {
+        Self { range: range.start.into_raw().into()..range.end.into_raw().into(), _p: PhantomData }
+    }
+}
+
+impl<T> Iterator for IdRange<T> {
+    type Item = Idx<T>;
+    fn next(&mut self) -> Option<Self::Item> {
+        self.range.next().map(|raw| Idx::from_raw(raw.into()))
+    }
+}
+
+impl<T> fmt::Debug for IdRange<T> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_tuple(&format!("IdRange::<{}>", type_name::<T>())).field(&self.range).finish()
+    }
+}
+
+impl<T> Clone for IdRange<T> {
+    fn clone(&self) -> Self {
+        Self { range: self.range.clone(), _p: PhantomData }
+    }
+}
+
+impl<T> PartialEq for IdRange<T> {
+    fn eq(&self, other: &Self) -> bool {
+        self.range == other.range
+    }
+}
+
+impl<T> Eq for IdRange<T> {}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Fields {
-    Record(Range<Idx<Field>>),
-    Tuple(Range<Idx<Field>>),
+    Record(IdRange<Field>),
+    Tuple(IdRange<Field>),
     Unit,
 }
 
diff --git a/crates/ra_hir_def/src/item_tree/lower.rs b/crates/ra_hir_def/src/item_tree/lower.rs
index f10ad25f70..5149dd141f 100644
--- a/crates/ra_hir_def/src/item_tree/lower.rs
+++ b/crates/ra_hir_def/src/item_tree/lower.rs
@@ -126,15 +126,15 @@ impl Ctx {
 
         if !attrs.is_empty() {
             for item in items.iter().flat_map(|items| &items.0) {
-                self.add_attrs(*item, attrs.clone());
+                self.add_attrs((*item).into(), attrs.clone());
             }
         }
 
         items
     }
 
-    fn add_attrs(&mut self, item: ModItem, attrs: Attrs) {
-        match self.tree.attrs.entry(AttrOwner::ModItem(item)) {
+    fn add_attrs(&mut self, item: AttrOwner, attrs: Attrs) {
+        match self.tree.attrs.entry(item) {
             Entry::Occupied(mut entry) => {
                 *entry.get_mut() = entry.get().merge(attrs);
             }
@@ -196,15 +196,16 @@ impl Ctx {
         }
     }
 
-    fn lower_record_fields(&mut self, fields: &ast::RecordFieldDefList) -> Range<Idx<Field>> {
+    fn lower_record_fields(&mut self, fields: &ast::RecordFieldDefList) -> IdRange<Field> {
         let start = self.next_field_idx();
         for field in fields.fields() {
             if let Some(data) = self.lower_record_field(&field) {
-                self.data().fields.alloc(data);
+                let idx = self.data().fields.alloc(data);
+                self.add_attrs(idx.into(), Attrs::new(&field, &self.hygiene));
             }
         }
         let end = self.next_field_idx();
-        start..end
+        IdRange::new(start..end)
     }
 
     fn lower_record_field(&mut self, field: &ast::RecordFieldDef) -> Option<Field> {
@@ -215,15 +216,16 @@ impl Ctx {
         Some(res)
     }
 
-    fn lower_tuple_fields(&mut self, fields: &ast::TupleFieldDefList) -> Range<Idx<Field>> {
+    fn lower_tuple_fields(&mut self, fields: &ast::TupleFieldDefList) -> IdRange<Field> {
         let start = self.next_field_idx();
         for (i, field) in fields.fields().enumerate() {
             if let Some(data) = self.lower_tuple_field(i, &field) {
-                self.data().fields.alloc(data);
+                let idx = self.data().fields.alloc(data);
+                self.add_attrs(idx.into(), Attrs::new(&field, &self.hygiene));
             }
         }
         let end = self.next_field_idx();
-        start..end
+        IdRange::new(start..end)
     }
 
     fn lower_tuple_field(&mut self, idx: usize, field: &ast::TupleFieldDef) -> Option<Field> {
@@ -242,7 +244,7 @@ impl Ctx {
             Some(record_field_def_list) => {
                 self.lower_fields(&StructKind::Record(record_field_def_list))
             }
-            None => Fields::Record(self.next_field_idx()..self.next_field_idx()),
+            None => Fields::Record(IdRange::new(self.next_field_idx()..self.next_field_idx())),
         };
         let ast_id = self.source_ast_id_map.ast_id(union);
         let res = Union { name, visibility, generic_params, fields, ast_id };
@@ -255,22 +257,23 @@ impl Ctx {
         let generic_params = self.lower_generic_params(GenericsOwner::Enum, enum_);
         let variants = match &enum_.variant_list() {
             Some(variant_list) => self.lower_variants(variant_list),
-            None => self.next_variant_idx()..self.next_variant_idx(),
+            None => IdRange::new(self.next_variant_idx()..self.next_variant_idx()),
         };
         let ast_id = self.source_ast_id_map.ast_id(enum_);
         let res = Enum { name, visibility, generic_params, variants, ast_id };
         Some(id(self.data().enums.alloc(res)))
     }
 
-    fn lower_variants(&mut self, variants: &ast::EnumVariantList) -> Range<Idx<Variant>> {
+    fn lower_variants(&mut self, variants: &ast::EnumVariantList) -> IdRange<Variant> {
         let start = self.next_variant_idx();
         for variant in variants.variants() {
             if let Some(data) = self.lower_variant(&variant) {
-                self.data().variants.alloc(data);
+                let idx = self.data().variants.alloc(data);
+                self.add_attrs(idx.into(), Attrs::new(&variant, &self.hygiene));
             }
         }
         let end = self.next_variant_idx();
-        start..end
+        IdRange::new(start..end)
     }
 
     fn lower_variant(&mut self, variant: &ast::EnumVariant) -> Option<Variant> {
@@ -419,7 +422,7 @@ impl Ctx {
                         let attrs = Attrs::new(&item, &this.hygiene);
                         this.collect_inner_items(item.syntax());
                         this.lower_assoc_item(&item).map(|item| {
-                            this.add_attrs(item.into(), attrs);
+                            this.add_attrs(ModItem::from(item).into(), attrs);
                             item
                         })
                     })
@@ -453,7 +456,7 @@ impl Ctx {
                 self.collect_inner_items(item.syntax());
                 let assoc = self.lower_assoc_item(&item)?;
                 let attrs = Attrs::new(&item, &self.hygiene);
-                self.add_attrs(assoc.into(), attrs);
+                self.add_attrs(ModItem::from(assoc).into(), attrs);
                 Some(assoc)
             })
             .collect();
@@ -539,7 +542,7 @@ impl Ctx {
                 .filter_map(|item| {
                     self.collect_inner_items(item.syntax());
                     let attrs = Attrs::new(&item, &self.hygiene);
-                    let id = match item {
+                    let id: ModItem = match item {
                         ast::ExternItem::FnDef(ast) => {
                             let func = self.lower_function(&ast)?;
                             func.into()
@@ -549,7 +552,7 @@ impl Ctx {
                             statik.into()
                         }
                     };
-                    self.add_attrs(id, attrs);
+                    self.add_attrs(id.into(), attrs);
                     Some(id)
                 })
                 .collect()
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index 94da700ad3..2ced4f66bb 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -742,7 +742,7 @@ impl ModCollector<'_, '_> {
         // `#[macro_use] extern crate` is hoisted to imports macros before collecting
         // any other items.
         for item in items {
-            if self.is_cfg_enabled(self.item_tree.attrs(*item)) {
+            if self.is_cfg_enabled(self.item_tree.attrs((*item).into())) {
                 if let ModItem::ExternCrate(id) = item {
                     let import = self.item_tree[*id].clone();
                     if import.is_macro_use {
@@ -753,7 +753,7 @@ impl ModCollector<'_, '_> {
         }
 
         for &item in items {
-            let attrs = self.item_tree.attrs(item);
+            let attrs = self.item_tree.attrs(item.into());
             if self.is_cfg_enabled(attrs) {
                 let module =
                     ModuleId { krate: self.def_collector.def_map.krate, local_id: self.module_id };
diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs
index de6b956866..446207e9e5 100644
--- a/crates/rust-analyzer/src/global_state.rs
+++ b/crates/rust-analyzer/src/global_state.rs
@@ -20,9 +20,8 @@ use crate::{
     diagnostics::{CheckFixes, DiagnosticCollection},
     from_proto,
     line_endings::LineEndings,
-    main_loop::{ReqQueue, Task},
+    main_loop::ReqQueue,
     request_metrics::{LatestRequests, RequestMetrics},
-    thread_pool::TaskPool,
     to_proto::url_from_abs_path,
     Result,
 };
@@ -67,7 +66,6 @@ impl Default for Status {
 /// incremental salsa database.
 pub(crate) struct GlobalState {
     pub(crate) config: Config,
-    pub(crate) task_pool: (TaskPool<Task>, Receiver<Task>),
     pub(crate) analysis_host: AnalysisHost,
     pub(crate) loader: Box<dyn vfs::loader::Handle>,
     pub(crate) task_receiver: Receiver<vfs::loader::Message>,
@@ -155,15 +153,8 @@ impl GlobalState {
 
         let mut analysis_host = AnalysisHost::new(lru_capacity);
         analysis_host.apply_change(change);
-
-        let task_pool = {
-            let (sender, receiver) = unbounded();
-            (TaskPool::new(sender), receiver)
-        };
-
         let mut res = GlobalState {
             config,
-            task_pool,
             analysis_host,
             loader,
             task_receiver,
diff --git a/crates/rust-analyzer/src/lib.rs b/crates/rust-analyzer/src/lib.rs
index ca788dd3cf..7942866726 100644
--- a/crates/rust-analyzer/src/lib.rs
+++ b/crates/rust-analyzer/src/lib.rs
@@ -30,7 +30,6 @@ mod diagnostics;
 mod line_endings;
 mod request_metrics;
 mod lsp_utils;
-mod thread_pool;
 pub mod lsp_ext;
 pub mod config;
 
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index 1a9c5ee2ce..3b3b832091 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -2,17 +2,19 @@
 //! requests/replies and notifications back to the client.
 use std::{
     env, fmt, panic,
+    sync::Arc,
     time::{Duration, Instant},
 };
 
-use crossbeam_channel::{never, select, RecvError, Sender};
-use lsp_server::{Connection, ErrorCode, Notification, Request, RequestId, Response};
+use crossbeam_channel::{never, select, unbounded, RecvError, Sender};
+use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response};
 use lsp_types::{request::Request as _, NumberOrString};
 use ra_db::VfsPath;
 use ra_ide::{Canceled, FileId};
 use ra_prof::profile;
 use ra_project_model::{PackageRoot, ProjectWorkspace};
 use serde::{de::DeserializeOwned, Serialize};
+use threadpool::ThreadPool;
 
 use crate::{
     config::{Config, FilesWatcher, LinkedProject},
@@ -116,51 +118,65 @@ pub fn main_loop(config: Config, connection: Connection) -> Result<()> {
         GlobalState::new(workspaces, config.lru_capacity, config, req_queue)
     };
 
+    let pool = ThreadPool::default();
+    let (task_sender, task_receiver) = unbounded::<Task>();
+
     log::info!("server initialized, serving requests");
     {
+        let task_sender = task_sender;
         loop {
             log::trace!("selecting");
             let event = select! {
                 recv(&connection.receiver) -> msg => match msg {
-                    Ok(msg) => Event::Lsp(msg),
+                    Ok(msg) => Event::Msg(msg),
                     Err(RecvError) => return Err("client exited without shutdown".into()),
                 },
-                recv(&global_state.task_pool.1) -> task => Event::Task(task.unwrap()),
+                recv(task_receiver) -> task => Event::Task(task.unwrap()),
                 recv(global_state.task_receiver) -> task => match task {
                     Ok(task) => Event::Vfs(task),
                     Err(RecvError) => return Err("vfs died".into()),
                 },
                 recv(global_state.flycheck.as_ref().map_or(&never(), |it| &it.1)) -> task => match task {
-                    Ok(task) => Event::Flycheck(task),
+                    Ok(task) => Event::CheckWatcher(task),
                     Err(RecvError) => return Err("check watcher died".into()),
                 },
             };
-            if let Event::Lsp(lsp_server::Message::Request(req)) = &event {
+            if let Event::Msg(Message::Request(req)) = &event {
                 if connection.handle_shutdown(&req)? {
                     break;
                 };
             }
             assert!(!global_state.vfs.read().0.has_changes());
-            loop_turn(&connection, &mut global_state, event)?;
+            loop_turn(&pool, &task_sender, &connection, &mut global_state, event)?;
             assert!(!global_state.vfs.read().0.has_changes());
         }
     }
     global_state.analysis_host.request_cancellation();
+    log::info!("waiting for tasks to finish...");
+    task_receiver.into_iter().for_each(|task| on_task(task, &connection.sender, &mut global_state));
+    log::info!("...tasks have finished");
+    log::info!("joining threadpool...");
+    pool.join();
+    drop(pool);
+    log::info!("...threadpool has finished");
+
+    let vfs = Arc::try_unwrap(global_state.vfs).expect("all snapshots should be dead");
+    drop(vfs);
+
     Ok(())
 }
 
 #[derive(Debug)]
-pub(crate) enum Task {
+enum Task {
     Respond(Response),
-    Diagnostics(Vec<DiagnosticTask>),
-    Unit,
+    Diagnostic(DiagnosticTask),
 }
 
 enum Event {
-    Lsp(lsp_server::Message),
+    Msg(Message),
     Task(Task),
     Vfs(vfs::loader::Message),
-    Flycheck(flycheck::Message),
+    CheckWatcher(flycheck::Message),
 }
 
 impl fmt::Debug for Event {
@@ -170,7 +186,7 @@ impl fmt::Debug for Event {
         };
 
         match self {
-            Event::Lsp(lsp_server::Message::Notification(not)) => {
+            Event::Msg(Message::Notification(not)) => {
                 if notification_is::<lsp_types::notification::DidOpenTextDocument>(not)
                     || notification_is::<lsp_types::notification::DidChangeTextDocument>(not)
                 {
@@ -187,10 +203,10 @@ impl fmt::Debug for Event {
             _ => (),
         }
         match self {
-            Event::Lsp(it) => fmt::Debug::fmt(it, f),
+            Event::Msg(it) => fmt::Debug::fmt(it, f),
             Event::Task(it) => fmt::Debug::fmt(it, f),
             Event::Vfs(it) => fmt::Debug::fmt(it, f),
-            Event::Flycheck(it) => fmt::Debug::fmt(it, f),
+            Event::CheckWatcher(it) => fmt::Debug::fmt(it, f),
         }
     }
 }
@@ -199,13 +215,19 @@ pub(crate) type ReqHandler = fn(&mut GlobalState, Response);
 pub(crate) type ReqQueue = lsp_server::ReqQueue<(&'static str, Instant), ReqHandler>;
 const DO_NOTHING: ReqHandler = |_, _| ();
 
-fn loop_turn(connection: &Connection, global_state: &mut GlobalState, event: Event) -> Result<()> {
+fn loop_turn(
+    pool: &ThreadPool,
+    task_sender: &Sender<Task>,
+    connection: &Connection,
+    global_state: &mut GlobalState,
+    event: Event,
+) -> Result<()> {
     let loop_start = Instant::now();
 
     // NOTE: don't count blocking select! call as a loop-turn time
     let _p = profile("main_loop_inner/loop-turn");
     log::info!("loop turn = {:?}", event);
-    let queue_count = global_state.task_pool.0.len();
+    let queue_count = pool.queued_count();
     if queue_count > 0 {
         log::info!("queued count = {}", queue_count);
     }
@@ -247,15 +269,17 @@ fn loop_turn(connection: &Connection, global_state: &mut GlobalState, event: Eve
                 )
             }
         },
-        Event::Flycheck(task) => on_check_task(task, global_state, &connection.sender)?,
-        Event::Lsp(msg) => match msg {
-            lsp_server::Message::Request(req) => {
-                on_request(global_state, &connection.sender, loop_start, req)?
+        Event::CheckWatcher(task) => {
+            on_check_task(task, global_state, task_sender, &connection.sender)?
+        }
+        Event::Msg(msg) => match msg {
+            Message::Request(req) => {
+                on_request(global_state, pool, task_sender, &connection.sender, loop_start, req)?
             }
-            lsp_server::Message::Notification(not) => {
+            Message::Notification(not) => {
                 on_notification(&connection.sender, global_state, not)?;
             }
-            lsp_server::Message::Response(resp) => {
+            Message::Response(resp) => {
                 let handler = global_state.req_queue.outgoing.complete(resp.id.clone());
                 handler(global_state, resp)
             }
@@ -277,14 +301,16 @@ fn loop_turn(connection: &Connection, global_state: &mut GlobalState, event: Eve
             .map(|path| global_state.vfs.read().0.file_id(&path).unwrap())
             .collect::<Vec<_>>();
 
-        update_file_notifications_on_threadpool(global_state, subscriptions.clone());
-        global_state.task_pool.0.spawn({
+        update_file_notifications_on_threadpool(
+            pool,
+            global_state.snapshot(),
+            task_sender.clone(),
+            subscriptions.clone(),
+        );
+        pool.execute({
             let subs = subscriptions;
             let snap = global_state.snapshot();
-            move || {
-                snap.analysis.prime_caches(subs).unwrap_or_else(|_: Canceled| ());
-                Task::Unit
-            }
+            move || snap.analysis.prime_caches(subs).unwrap_or_else(|_: Canceled| ())
         });
     }
 
@@ -303,7 +329,7 @@ fn loop_turn(connection: &Connection, global_state: &mut GlobalState, event: Eve
     Ok(())
 }
 
-fn on_task(task: Task, msg_sender: &Sender<lsp_server::Message>, global_state: &mut GlobalState) {
+fn on_task(task: Task, msg_sender: &Sender<Message>, global_state: &mut GlobalState) {
     match task {
         Task::Respond(response) => {
             if let Some((method, start)) =
@@ -319,21 +345,26 @@ fn on_task(task: Task, msg_sender: &Sender<lsp_server::Message>, global_state: &
                 msg_sender.send(response.into()).unwrap();
             }
         }
-        Task::Diagnostics(tasks) => {
-            tasks.into_iter().for_each(|task| on_diagnostic_task(task, msg_sender, global_state))
-        }
-        Task::Unit => (),
+        Task::Diagnostic(task) => on_diagnostic_task(task, msg_sender, global_state),
     }
 }
 
 fn on_request(
     global_state: &mut GlobalState,
-    msg_sender: &Sender<lsp_server::Message>,
+    pool: &ThreadPool,
+    task_sender: &Sender<Task>,
+    msg_sender: &Sender<Message>,
     request_received: Instant,
     req: Request,
 ) -> Result<()> {
-    let mut pool_dispatcher =
-        PoolDispatcher { req: Some(req), global_state, msg_sender, request_received };
+    let mut pool_dispatcher = PoolDispatcher {
+        req: Some(req),
+        pool,
+        global_state,
+        task_sender,
+        msg_sender,
+        request_received,
+    };
     pool_dispatcher
         .on_sync::<lsp_ext::CollectGarbage>(|s, ()| Ok(s.collect_garbage()))?
         .on_sync::<lsp_ext::JoinLines>(|s, p| handlers::handle_join_lines(s.snapshot(), p))?
@@ -384,7 +415,7 @@ fn on_request(
 }
 
 fn on_notification(
-    msg_sender: &Sender<lsp_server::Message>,
+    msg_sender: &Sender<Message>,
     global_state: &mut GlobalState,
     not: Notification,
 ) -> Result<()> {
@@ -521,11 +552,12 @@ fn on_notification(
 fn on_check_task(
     task: flycheck::Message,
     global_state: &mut GlobalState,
-    msg_sender: &Sender<lsp_server::Message>,
+    task_sender: &Sender<Task>,
+    msg_sender: &Sender<Message>,
 ) -> Result<()> {
     match task {
         flycheck::Message::ClearDiagnostics => {
-            on_diagnostic_task(DiagnosticTask::ClearCheck, msg_sender, global_state)
+            task_sender.send(Task::Diagnostic(DiagnosticTask::ClearCheck))?;
         }
 
         flycheck::Message::AddDiagnostic { workspace_root, diagnostic } => {
@@ -544,15 +576,11 @@ fn on_check_task(
                     }
                 };
 
-                on_diagnostic_task(
-                    DiagnosticTask::AddCheck(
-                        file_id,
-                        diag.diagnostic,
-                        diag.fixes.into_iter().map(|it| it.into()).collect(),
-                    ),
-                    msg_sender,
-                    global_state,
-                )
+                task_sender.send(Task::Diagnostic(DiagnosticTask::AddCheck(
+                    file_id,
+                    diag.diagnostic,
+                    diag.fixes.into_iter().map(|it| it.into()).collect(),
+                )))?;
             }
         }
 
@@ -570,11 +598,7 @@ fn on_check_task(
     Ok(())
 }
 
-fn on_diagnostic_task(
-    task: DiagnosticTask,
-    msg_sender: &Sender<lsp_server::Message>,
-    state: &mut GlobalState,
-) {
+fn on_diagnostic_task(task: DiagnosticTask, msg_sender: &Sender<Message>, state: &mut GlobalState) {
     let subscriptions = state.diagnostics.handle_task(task);
 
     for file_id in subscriptions {
@@ -599,7 +623,7 @@ fn percentage(done: usize, total: usize) -> f64 {
 
 fn report_progress(
     global_state: &mut GlobalState,
-    sender: &Sender<lsp_server::Message>,
+    sender: &Sender<Message>,
     title: &str,
     state: Progress,
     message: Option<String>,
@@ -646,8 +670,10 @@ fn report_progress(
 
 struct PoolDispatcher<'a> {
     req: Option<Request>,
+    pool: &'a ThreadPool,
     global_state: &'a mut GlobalState,
-    msg_sender: &'a Sender<lsp_server::Message>,
+    msg_sender: &'a Sender<Message>,
+    task_sender: &'a Sender<Task>,
     request_received: Instant,
 }
 
@@ -695,11 +721,13 @@ impl<'a> PoolDispatcher<'a> {
             }
         };
 
-        self.global_state.task_pool.0.spawn({
+        self.pool.execute({
             let world = self.global_state.snapshot();
+            let sender = self.task_sender.clone();
             move || {
                 let result = f(world, params);
-                result_to_task::<R>(id, result)
+                let task = result_to_task::<R>(id, result);
+                sender.send(task).unwrap();
             }
         });
 
@@ -769,27 +797,26 @@ where
 }
 
 fn update_file_notifications_on_threadpool(
-    global_state: &mut GlobalState,
+    pool: &ThreadPool,
+    world: GlobalStateSnapshot,
+    task_sender: Sender<Task>,
     subscriptions: Vec<FileId>,
 ) {
     log::trace!("updating notifications for {:?}", subscriptions);
-    if global_state.config.publish_diagnostics {
-        let snapshot = global_state.snapshot();
-        global_state.task_pool.0.spawn(move || {
-            let diagnostics = subscriptions
-                .into_iter()
-                .filter_map(|file_id| {
-                    handlers::publish_diagnostics(&snapshot, file_id)
-                        .map_err(|err| {
-                            if !is_canceled(&*err) {
-                                log::error!("failed to compute diagnostics: {:?}", err);
-                            }
-                            ()
-                        })
-                        .ok()
-                })
-                .collect::<Vec<_>>();
-            Task::Diagnostics(diagnostics)
+    if world.config.publish_diagnostics {
+        pool.execute(move || {
+            for file_id in subscriptions {
+                match handlers::publish_diagnostics(&world, file_id) {
+                    Err(e) => {
+                        if !is_canceled(&*e) {
+                            log::error!("failed to compute diagnostics: {:?}", e);
+                        }
+                    }
+                    Ok(task) => {
+                        task_sender.send(Task::Diagnostic(task)).unwrap();
+                    }
+                }
+            }
         })
     }
 }
diff --git a/crates/rust-analyzer/src/thread_pool.rs b/crates/rust-analyzer/src/thread_pool.rs
deleted file mode 100644
index 4fa5029253..0000000000
--- a/crates/rust-analyzer/src/thread_pool.rs
+++ /dev/null
@@ -1,35 +0,0 @@
-//! A thin wrapper around `ThreadPool` to make sure that we join all things
-//! properly.
-use crossbeam_channel::Sender;
-
-pub(crate) struct TaskPool<T> {
-    sender: Sender<T>,
-    inner: threadpool::ThreadPool,
-}
-
-impl<T> TaskPool<T> {
-    pub(crate) fn new(sender: Sender<T>) -> TaskPool<T> {
-        TaskPool { sender, inner: threadpool::ThreadPool::default() }
-    }
-
-    pub(crate) fn spawn<F>(&mut self, task: F)
-    where
-        F: FnOnce() -> T + Send + 'static,
-        T: Send + 'static,
-    {
-        self.inner.execute({
-            let sender = self.sender.clone();
-            move || sender.send(task()).unwrap()
-        })
-    }
-
-    pub(crate) fn len(&self) -> usize {
-        self.inner.queued_count()
-    }
-}
-
-impl<T> Drop for TaskPool<T> {
-    fn drop(&mut self) {
-        self.inner.join()
-    }
-}
diff --git a/crates/vfs-notify/src/lib.rs b/crates/vfs-notify/src/lib.rs
index 68fdb8cb09..282cf03580 100644
--- a/crates/vfs-notify/src/lib.rs
+++ b/crates/vfs-notify/src/lib.rs
@@ -83,8 +83,8 @@ impl NotifyActor {
         }
     }
 
-    fn run(mut self, inbox: Receiver<Message>) {
-        while let Some(event) = self.next_event(&inbox) {
+    fn run(mut self, receiver: Receiver<Message>) {
+        while let Some(event) = self.next_event(&receiver) {
             log::debug!("vfs-notify event: {:?}", event);
             match event {
                 Event::Message(msg) => match msg {


Test patch:
diff --git a/crates/ra_hir_def/src/item_tree/tests.rs b/crates/ra_hir_def/src/item_tree/tests.rs
index dc035d809e..08559fb92b 100644
--- a/crates/ra_hir_def/src/item_tree/tests.rs
+++ b/crates/ra_hir_def/src/item_tree/tests.rs
@@ -92,7 +92,7 @@ fn print_item_tree(ra_fixture: &str) -> String {
 }
 
 fn fmt_mod_item(out: &mut String, tree: &ItemTree, item: ModItem) {
-    let attrs = tree.attrs(item);
+    let attrs = tree.attrs(item.into());
     if !attrs.is_empty() {
         format_to!(out, "#[{:?}]\n", attrs);
     }
@@ -237,13 +237,13 @@ Trait { name: Name(Text("Tr")), visibility: RawVisibilityId("pub(self)"), generi
 #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("struct0"))] }, input: None }]) }]
 Struct { name: Name(Text("Struct0")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(1), fields: Unit, ast_id: FileAstId::<ra_syntax::ast::generated::nodes::StructDef>(3), kind: Unit }
 #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("struct1"))] }, input: None }]) }]
-Struct { name: Name(Text("Struct1")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(2), fields: Tuple(Idx::<Field>(0)..Idx::<Field>(1)), ast_id: FileAstId::<ra_syntax::ast::generated::nodes::StructDef>(4), kind: Tuple }
+Struct { name: Name(Text("Struct1")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(2), fields: Tuple(IdRange::<ra_hir_def::item_tree::Field>(0..1)), ast_id: FileAstId::<ra_syntax::ast::generated::nodes::StructDef>(4), kind: Tuple }
 #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("struct2"))] }, input: None }]) }]
-Struct { name: Name(Text("Struct2")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(3), fields: Record(Idx::<Field>(1)..Idx::<Field>(2)), ast_id: FileAstId::<ra_syntax::ast::generated::nodes::StructDef>(5), kind: Record }
+Struct { name: Name(Text("Struct2")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(3), fields: Record(IdRange::<ra_hir_def::item_tree::Field>(1..2)), ast_id: FileAstId::<ra_syntax::ast::generated::nodes::StructDef>(5), kind: Record }
 #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("en"))] }, input: None }]) }]
-Enum { name: Name(Text("En")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), variants: Idx::<Variant>(0)..Idx::<Variant>(1), ast_id: FileAstId::<ra_syntax::ast::generated::nodes::EnumDef>(6) }
+Enum { name: Name(Text("En")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), variants: IdRange::<ra_hir_def::item_tree::Variant>(0..1), ast_id: FileAstId::<ra_syntax::ast::generated::nodes::EnumDef>(6) }
 #[Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("un"))] }, input: None }]) }]
-Union { name: Name(Text("Un")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), fields: Record(Idx::<Field>(3)..Idx::<Field>(4)), ast_id: FileAstId::<ra_syntax::ast::generated::nodes::UnionDef>(7) }
+Union { name: Name(Text("Un")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), fields: Record(IdRange::<ra_hir_def::item_tree::Field>(3..4)), ast_id: FileAstId::<ra_syntax::ast::generated::nodes::UnionDef>(7) }
     "###);
 }