Repository: rust-lang/rust-analyzer
Pull request #16911 (2024-03-21 15:57:13)

fix: Fix projects depending on `rustc_private` hanging
If loading the root fails, we'll hang up in this loop as we never inserted the entry that asserts we already visited a package. This fixes that
Fixes https://github.com/rust-lang/rust-analyzer/issues/16902


Patch:
diff --git a/crates/hir-def/src/attr.rs b/crates/hir-def/src/attr.rs
index fa7730f302..21536098b8 100644
--- a/crates/hir-def/src/attr.rs
+++ b/crates/hir-def/src/attr.rs
@@ -148,12 +148,12 @@ impl Attrs {
         }
     }
 
-    pub fn lang(&self) -> Option<&str> {
+    pub fn lang(&self) -> Option<&SmolStr> {
         self.by_key("lang").string_value()
     }
 
     pub fn lang_item(&self) -> Option<LangItem> {
-        self.by_key("lang").string_value().and_then(LangItem::from_str)
+        self.by_key("lang").string_value().and_then(|it| LangItem::from_str(it))
     }
 
     pub fn has_doc_hidden(&self) -> bool {
@@ -178,7 +178,7 @@ impl Attrs {
         self.doc_exprs().flat_map(|doc_expr| doc_expr.aliases().to_vec())
     }
 
-    pub fn export_name(&self) -> Option<&str> {
+    pub fn export_name(&self) -> Option<&SmolStr> {
         self.by_key("export_name").string_value()
     }
 
@@ -565,7 +565,7 @@ impl<'attr> AttrQuery<'attr> {
         self.attrs().filter_map(|attr| attr.token_tree_value())
     }
 
-    pub fn string_value(self) -> Option<&'attr str> {
+    pub fn string_value(self) -> Option<&'attr SmolStr> {
         self.attrs().find_map(|attr| attr.string_value())
     }
 
diff --git a/crates/hir-def/src/lang_item.rs b/crates/hir-def/src/lang_item.rs
index 3a07c67842..7d98f6cfe8 100644
--- a/crates/hir-def/src/lang_item.rs
+++ b/crates/hir-def/src/lang_item.rs
@@ -192,7 +192,7 @@ impl LangItems {
 
 pub(crate) fn lang_attr(db: &dyn DefDatabase, item: AttrDefId) -> Option<LangItem> {
     let attrs = db.attrs(item);
-    attrs.by_key("lang").string_value().and_then(LangItem::from_str)
+    attrs.by_key("lang").string_value().and_then(|it| LangItem::from_str(it))
 }
 
 pub(crate) fn notable_traits_in_deps(
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index 3cfbf6c89c..312938e2c1 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs
@@ -24,7 +24,7 @@ use limit::Limit;
 use rustc_hash::{FxHashMap, FxHashSet};
 use span::{Edition, ErasedFileAstId, FileAstId, Span, SyntaxContextId};
 use stdx::always;
-use syntax::ast;
+use syntax::{ast, SmolStr};
 use triomphe::Arc;
 
 use crate::{
@@ -312,7 +312,7 @@ impl DefCollector<'_> {
                     }
                 }
                 () if *attr_name == hir_expand::name![crate_type] => {
-                    if let Some("proc-macro") = attr.string_value() {
+                    if let Some("proc-macro") = attr.string_value().map(SmolStr::as_str) {
                         self.is_proc_macro = true;
                     }
                 }
@@ -1902,7 +1902,7 @@ impl ModCollector<'_, '_> {
     }
 
     fn collect_module(&mut self, module_id: FileItemTreeId<Mod>, attrs: &Attrs) {
-        let path_attr = attrs.by_key("path").string_value();
+        let path_attr = attrs.by_key("path").string_value().map(SmolStr::as_str);
         let is_macro_use = attrs.by_key("macro_use").exists();
         let module = &self.item_tree[module_id];
         match &module.kind {
@@ -2146,7 +2146,7 @@ impl ModCollector<'_, '_> {
                 Some(it) => {
                     // FIXME: a hacky way to create a Name from string.
                     name = tt::Ident {
-                        text: it.into(),
+                        text: it.clone(),
                         span: Span {
                             range: syntax::TextRange::empty(syntax::TextSize::new(0)),
                             anchor: span::SpanAnchor {
diff --git a/crates/hir-expand/src/attrs.rs b/crates/hir-expand/src/attrs.rs
index f1540498f2..af3ecdcd5e 100644
--- a/crates/hir-expand/src/attrs.rs
+++ b/crates/hir-expand/src/attrs.rs
@@ -8,8 +8,8 @@ use intern::Interned;
 use mbe::{syntax_node_to_token_tree, DelimiterKind, Punct};
 use smallvec::{smallvec, SmallVec};
 use span::{Span, SyntaxContextId};
-use syntax::{ast, format_smolstr, match_ast, AstNode, AstToken, SmolStr, SyntaxNode};
-use triomphe::ThinArc;
+use syntax::{ast, match_ast, AstNode, AstToken, SmolStr, SyntaxNode};
+use triomphe::Arc;
 
 use crate::{
     db::ExpandDatabase,
@@ -22,7 +22,8 @@ use crate::{
 /// Syntactical attributes, without filtering of `cfg_attr`s.
 #[derive(Default, Debug, Clone, PartialEq, Eq)]
 pub struct RawAttrs {
-    entries: Option<ThinArc<(), Attr>>,
+    // FIXME: Make this a ThinArc
+    entries: Option<Arc<[Attr]>>,
 }
 
 impl ops::Deref for RawAttrs {
@@ -30,7 +31,7 @@ impl ops::Deref for RawAttrs {
 
     fn deref(&self) -> &[Attr] {
         match &self.entries {
-            Some(it) => &it.slice,
+            Some(it) => it,
             None => &[],
         }
     }
@@ -44,34 +45,20 @@ impl RawAttrs {
         owner: &dyn ast::HasAttrs,
         span_map: SpanMapRef<'_>,
     ) -> Self {
-        let entries: Vec<_> = collect_attrs(owner)
-            .filter_map(|(id, attr)| match attr {
-                Either::Left(attr) => {
-                    attr.meta().and_then(|meta| Attr::from_src(db, meta, span_map, id))
-                }
-                Either::Right(comment) => comment.doc_comment().map(|doc| {
-                    let span = span_map.span_for_range(comment.syntax().text_range());
-                    Attr {
-                        id,
-                        input: Some(Interned::new(AttrInput::Literal(tt::Literal {
-                            // FIXME: Escape quotes from comment content
-                            text: SmolStr::new(format_smolstr!("\"{doc}\"",)),
-                            span,
-                        }))),
-                        path: Interned::new(ModPath::from(crate::name!(doc))),
-                        ctxt: span.ctx,
-                    }
-                }),
-            })
-            .collect();
-
-        let entries = if entries.is_empty() {
-            None
-        } else {
-            Some(ThinArc::from_header_and_iter((), entries.into_iter()))
-        };
+        let entries = collect_attrs(owner).filter_map(|(id, attr)| match attr {
+            Either::Left(attr) => {
+                attr.meta().and_then(|meta| Attr::from_src(db, meta, span_map, id))
+            }
+            Either::Right(comment) => comment.doc_comment().map(|doc| Attr {
+                id,
+                input: Some(Interned::new(AttrInput::Literal(SmolStr::new(doc)))),
+                path: Interned::new(ModPath::from(crate::name!(doc))),
+                ctxt: span_map.span_for_range(comment.syntax().text_range()).ctx,
+            }),
+        });
+        let entries: Arc<[Attr]> = Arc::from_iter(entries);
 
-        RawAttrs { entries }
+        Self { entries: if entries.is_empty() { None } else { Some(entries) } }
     }
 
     pub fn from_attrs_owner(
@@ -88,20 +75,16 @@ impl RawAttrs {
             (None, entries @ Some(_)) => Self { entries },
             (Some(entries), None) => Self { entries: Some(entries.clone()) },
             (Some(a), Some(b)) => {
-                let last_ast_index = a.slice.last().map_or(0, |it| it.id.ast_index() + 1) as u32;
-                let items = a
-                    .slice
-                    .iter()
-                    .cloned()
-                    .chain(b.slice.iter().map(|it| {
+                let last_ast_index = a.last().map_or(0, |it| it.id.ast_index() + 1) as u32;
+                Self {
+                    entries: Some(Arc::from_iter(a.iter().cloned().chain(b.iter().map(|it| {
                         let mut it = it.clone();
                         it.id.id = (it.id.ast_index() as u32 + last_ast_index)
                             | (it.id.cfg_attr_index().unwrap_or(0) as u32)
                                 << AttrId::AST_INDEX_BITS;
                         it
-                    }))
-                    .collect::<Vec<_>>();
-                Self { entries: Some(ThinArc::from_header_and_iter((), items.into_iter())) }
+                    })))),
+                }
             }
         }
     }
@@ -117,47 +100,41 @@ impl RawAttrs {
         }
 
         let crate_graph = db.crate_graph();
-        let new_attrs =
-            self.iter()
-                .flat_map(|attr| -> SmallVec<[_; 1]> {
-                    let is_cfg_attr =
-                        attr.path.as_ident().map_or(false, |name| *name == crate::name![cfg_attr]);
-                    if !is_cfg_attr {
-                        return smallvec![attr.clone()];
-                    }
+        let new_attrs = Arc::from_iter(self.iter().flat_map(|attr| -> SmallVec<[_; 1]> {
+            let is_cfg_attr =
+                attr.path.as_ident().map_or(false, |name| *name == crate::name![cfg_attr]);
+            if !is_cfg_attr {
+                return smallvec![attr.clone()];
+            }
 
-                    let subtree = match attr.token_tree_value() {
-                        Some(it) => it,
-                        _ => return smallvec![attr.clone()],
-                    };
-
-                    let (cfg, parts) = match parse_cfg_attr_input(subtree) {
-                        Some(it) => it,
-                        None => return smallvec![attr.clone()],
-                    };
-                    let index = attr.id;
-                    let attrs = parts.enumerate().take(1 << AttrId::CFG_ATTR_BITS).filter_map(
-                        |(idx, attr)| Attr::from_tt(db, attr, index.with_cfg_attr(idx)),
-                    );
-
-                    let cfg_options = &crate_graph[krate].cfg_options;
-                    let cfg = Subtree { delimiter: subtree.delimiter, token_trees: Box::from(cfg) };
-                    let cfg = CfgExpr::parse(&cfg);
-                    if cfg_options.check(&cfg) == Some(false) {
-                        smallvec![]
-                    } else {
-                        cov_mark::hit!(cfg_attr_active);
-
-                        attrs.collect()
-                    }
-                })
-                .collect::<Vec<_>>();
-        let entries = if new_attrs.is_empty() {
-            None
-        } else {
-            Some(ThinArc::from_header_and_iter((), new_attrs.into_iter()))
-        };
-        RawAttrs { entries }
+            let subtree = match attr.token_tree_value() {
+                Some(it) => it,
+                _ => return smallvec![attr.clone()],
+            };
+
+            let (cfg, parts) = match parse_cfg_attr_input(subtree) {
+                Some(it) => it,
+                None => return smallvec![attr.clone()],
+            };
+            let index = attr.id;
+            let attrs = parts
+                .enumerate()
+                .take(1 << AttrId::CFG_ATTR_BITS)
+                .filter_map(|(idx, attr)| Attr::from_tt(db, attr, index.with_cfg_attr(idx)));
+
+            let cfg_options = &crate_graph[krate].cfg_options;
+            let cfg = Subtree { delimiter: subtree.delimiter, token_trees: Box::from(cfg) };
+            let cfg = CfgExpr::parse(&cfg);
+            if cfg_options.check(&cfg) == Some(false) {
+                smallvec![]
+            } else {
+                cov_mark::hit!(cfg_attr_active);
+
+                attrs.collect()
+            }
+        }));
+
+        RawAttrs { entries: Some(new_attrs) }
     }
 }
 
@@ -202,7 +179,8 @@ pub struct Attr {
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum AttrInput {
     /// `#[attr = "string"]`
-    Literal(tt::Literal),
+    // FIXME: This is losing span
+    Literal(SmolStr),
     /// `#[attr(subtree)]`
     TokenTree(Box<tt::Subtree>),
 }
@@ -210,7 +188,7 @@ pub enum AttrInput {
 impl fmt::Display for AttrInput {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            AttrInput::Literal(lit) => write!(f, " = {lit}"),
+            AttrInput::Literal(lit) => write!(f, " = \"{}\"", lit.escape_debug()),
             AttrInput::TokenTree(tt) => tt.fmt(f),
         }
     }
@@ -230,10 +208,11 @@ impl Attr {
         })?);
         let span = span_map.span_for_range(range);
         let input = if let Some(ast::Expr::Literal(lit)) = ast.expr() {
-            Some(Interned::new(AttrInput::Literal(tt::Literal {
-                text: lit.token().text().into(),
-                span,
-            })))
+            let value = match lit.kind() {
+                ast::LiteralKind::String(string) => string.value()?.into(),
+                _ => lit.syntax().first_token()?.text().trim_matches('"').into(),
+            };
+            Some(Interned::new(AttrInput::Literal(value)))
         } else if let Some(tt) = ast.token_tree() {
             let tree = syntax_node_to_token_tree(tt.syntax(), span_map, span);
             Some(Interned::new(AttrInput::TokenTree(Box::new(tree))))
@@ -266,8 +245,9 @@ impl Attr {
             }
             Some(tt::TokenTree::Leaf(tt::Leaf::Punct(tt::Punct { char: '=', .. }))) => {
                 let input = match input.get(1) {
-                    Some(tt::TokenTree::Leaf(tt::Leaf::Literal(lit))) => {
-                        Some(Interned::new(AttrInput::Literal(lit.clone())))
+                    Some(tt::TokenTree::Leaf(tt::Leaf::Literal(tt::Literal { text, .. }))) => {
+                        //FIXME the trimming here isn't quite right, raw strings are not handled
+                        Some(Interned::new(AttrInput::Literal(text.trim_matches('"').into())))
                     }
                     _ => None,
                 };
@@ -285,14 +265,9 @@ impl Attr {
 
 impl Attr {
     /// #[path = "string"]
-    pub fn string_value(&self) -> Option<&str> {
+    pub fn string_value(&self) -> Option<&SmolStr> {
         match self.input.as_deref()? {
-            AttrInput::Literal(it) => match it.text.strip_prefix('r') {
-                Some(it) => it.trim_matches('#'),
-                None => it.text.as_str(),
-            }
-            .strip_prefix('"')?
-            .strip_suffix('"'),
+            AttrInput::Literal(it) => Some(it),
             _ => None,
         }
     }
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 321ff1eb39..15a0967b3d 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -2008,7 +2008,8 @@ impl Function {
         }
         let data = db.function_data(self.id);
 
-        data.name.to_smol_str() == "main" || data.attrs.export_name() == Some("main")
+        data.name.to_smol_str() == "main"
+            || data.attrs.export_name().map(core::ops::Deref::deref) == Some("main")
     }
 
     /// Does this function have the ignore attribute?
diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs
index 995e3f4825..aa22155fef 100644
--- a/crates/ide-completion/src/context.rs
+++ b/crates/ide-completion/src/context.rs
@@ -540,7 +540,7 @@ impl CompletionContext<'_> {
     /// Whether the given trait is an operator trait or not.
     pub(crate) fn is_ops_trait(&self, trait_: hir::Trait) -> bool {
         match trait_.attrs(self.db).lang() {
-            Some(lang) => OP_TRAIT_LANG_NAMES.contains(&lang),
+            Some(lang) => OP_TRAIT_LANG_NAMES.contains(&lang.as_str()),
             None => false,
         }
     }
diff --git a/crates/ide/src/status.rs b/crates/ide/src/status.rs
index 8e7767c8e5..c3d85e3893 100644
--- a/crates/ide/src/status.rs
+++ b/crates/ide/src/status.rs
@@ -10,7 +10,7 @@ use ide_db::{
             debug::{DebugQueryTable, TableEntry},
             Query, QueryTable,
         },
-        CompressedFileTextQuery, CrateData, FileId, ParseQuery, SourceDatabase, SourceRootId,
+        CrateData, FileId, FileTextQuery, ParseQuery, SourceDatabase, SourceRootId,
     },
     symbol_index::ModuleSymbolsQuery,
 };
@@ -38,7 +38,7 @@ use triomphe::Arc;
 pub(crate) fn status(db: &RootDatabase, file_id: Option<FileId>) -> String {
     let mut buf = String::new();
 
-    format_to!(buf, "{}\n", collect_query(CompressedFileTextQuery.in_db(db)));
+    format_to!(buf, "{}\n", collect_query(FileTextQuery.in_db(db)));
     format_to!(buf, "{}\n", collect_query(ParseQuery.in_db(db)));
     format_to!(buf, "{}\n", collect_query(ParseMacroExpansionQuery.in_db(db)));
     format_to!(buf, "{}\n", collect_query(LibrarySymbolsQuery.in_db(db)));
@@ -160,7 +160,7 @@ impl QueryCollect for ParseMacroExpansionQuery {
     type Collector = SyntaxTreeStats<true>;
 }
 
-impl QueryCollect for CompressedFileTextQuery {
+impl QueryCollect for FileTextQuery {
     type Collector = FilesStats;
 }
 
@@ -188,8 +188,8 @@ impl fmt::Display for FilesStats {
     }
 }
 
-impl StatCollect<FileId, Arc<[u8]>> for FilesStats {
-    fn collect_entry(&mut self, _: FileId, value: Option<Arc<[u8]>>) {
+impl StatCollect<FileId, Arc<str>> for FilesStats {
+    fn collect_entry(&mut self, _: FileId, value: Option<Arc<str>>) {
         self.total += 1;
         self.size += value.unwrap().len();
     }
diff --git a/crates/project-model/src/workspace.rs b/crates/project-model/src/workspace.rs
index 1556e9b549..ce9f292fd4 100644
--- a/crates/project-model/src/workspace.rs
+++ b/crates/project-model/src/workspace.rs
@@ -1250,6 +1250,7 @@ fn handle_rustc_crates(
                 let kind @ TargetKind::Lib { is_proc_macro } = rustc_workspace[tgt].kind else {
                     continue;
                 };
+                let pkg_crates = &mut rustc_pkg_crates.entry(pkg).or_insert_with(Vec::new);
                 if let Some(file_id) = load(&rustc_workspace[tgt].root) {
                     let crate_id = add_target_crate_root(
                         crate_graph,
@@ -1268,7 +1269,7 @@ fn handle_rustc_crates(
                     if let Some(proc_macro) = libproc_macro {
                         add_proc_macro_dep(crate_graph, crate_id, proc_macro, is_proc_macro);
                     }
-                    rustc_pkg_crates.entry(pkg).or_insert_with(Vec::new).push(crate_id);
+                    pkg_crates.push(crate_id);
                 }
             }
         }


Test patch:
diff --git a/crates/ide-diagnostics/src/tests.rs b/crates/ide-diagnostics/src/tests.rs
index bb5c2b7913..dcaa212089 100644
--- a/crates/ide-diagnostics/src/tests.rs
+++ b/crates/ide-diagnostics/src/tests.rs
@@ -283,10 +283,6 @@ fn test_disabled_diagnostics() {
 
 #[test]
 fn minicore_smoke_test() {
-    if test_utils::skip_slow_tests() {
-        return;
-    }
-
     fn check(minicore: MiniCore) {
         let source = minicore.source_code();
         let mut config = DiagnosticsConfig::test_sample();