Merge #8910

8910: Don't lower extern block in the ItemTree r=jonas-schievink a=jonas-schievink

The ItemTree lowering code used to attach attributes on an `extern {}` block to all the children. This is wrong and causes problems with attribute resolution that manifested as a hang.

This PR treats extern blocks as first-class items in the ItemTree and lowers its contents in the `ModCollector` instead.

Fixes https://github.com/rust-analyzer/rust-analyzer/issues/8414#issuecomment-845607923
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/8905
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/8909

bors r+

Co-authored-by: Jonas Schievink <[email protected]>

4 files changed, 79 insertions, 20 deletions

diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs
index cad8a7479..797b905d1 100644
--- a/crates/hir_def/src/item_tree.rs
+++ b/crates/hir_def/src/item_tree.rs
@@ -132,6 +132,7 @@ impl ItemTree {
             let ItemTreeData {
                 imports,
                 extern_crates,
+                extern_blocks,
                 functions,
                 params,
                 structs,
@@ -154,6 +155,7 @@ impl ItemTree {
 
             imports.shrink_to_fit();
             extern_crates.shrink_to_fit();
+            extern_blocks.shrink_to_fit();
             functions.shrink_to_fit();
             params.shrink_to_fit();
             structs.shrink_to_fit();
@@ -239,6 +241,7 @@ static VIS_PUB_CRATE: RawVisibility = RawVisibility::Module(ModPath::from_kind(P
 struct ItemTreeData {
     imports: Arena<Import>,
     extern_crates: Arena<ExternCrate>,
+    extern_blocks: Arena<ExternBlock>,
     functions: Arena<Function>,
     params: Arena<Param>,
     structs: Arena<Struct>,
@@ -432,6 +435,7 @@ macro_rules! mod_items {
 mod_items! {
     Import in imports -> ast::Use,
     ExternCrate in extern_crates -> ast::ExternCrate,
+    ExternBlock in extern_blocks -> ast::ExternBlock,
     Function in functions -> ast::Fn,
     Struct in structs -> ast::Struct,
     Union in unions -> ast::Union,
@@ -508,6 +512,13 @@ pub struct ExternCrate {
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
+pub struct ExternBlock {
+    pub abi: Option<Interned<str>>,
+    pub ast_id: FileAstId<ast::ExternBlock>,
+    pub children: Box<[ModItem]>,
+}
+
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct Function {
     pub name: Name,
     pub visibility: RawVisibilityId,
@@ -691,6 +702,7 @@ impl ModItem {
         match self {
             ModItem::Import(_)
             | ModItem::ExternCrate(_)
+            | ModItem::ExternBlock(_)
             | ModItem::Struct(_)
             | ModItem::Union(_)
             | ModItem::Enum(_)
@@ -715,6 +727,7 @@ impl ModItem {
         match self {
             ModItem::Import(it) => tree[it.index].ast_id().upcast(),
             ModItem::ExternCrate(it) => tree[it.index].ast_id().upcast(),
+            ModItem::ExternBlock(it) => tree[it.index].ast_id().upcast(),
             ModItem::Function(it) => tree[it.index].ast_id().upcast(),
             ModItem::Struct(it) => tree[it.index].ast_id().upcast(),
             ModItem::Union(it) => tree[it.index].ast_id().upcast(),
diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs
index fe348091d..a7ffc6364 100644
--- a/crates/hir_def/src/item_tree/lower.rs
+++ b/crates/hir_def/src/item_tree/lower.rs
@@ -147,9 +147,7 @@ impl<'a> Ctx<'a> {
             ast::Item::MacroCall(ast) => self.lower_macro_call(ast).map(Into::into),
             ast::Item::MacroRules(ast) => self.lower_macro_rules(ast).map(Into::into),
             ast::Item::MacroDef(ast) => self.lower_macro_def(ast).map(Into::into),
-            ast::Item::ExternBlock(ast) => {
-                Some(ModItems(self.lower_extern_block(ast).into_iter().collect::<SmallVec<_>>()))
-            }
+            ast::Item::ExternBlock(ast) => Some(self.lower_extern_block(ast).into()),
         };
 
         if !attrs.is_empty() {
@@ -397,19 +395,7 @@ impl<'a> Ctx<'a> {
             ret_type
         };
 
-        let abi = func.abi().map(|abi| {
-            // FIXME: Abi::abi() -> Option<SyntaxToken>?
-            match abi.syntax().last_token() {
-                Some(tok) if tok.kind() == SyntaxKind::STRING => {
-                    // FIXME: Better way to unescape?
-                    Interned::new_str(tok.text().trim_matches('"'))
-                }
-                _ => {
-                    // `extern` default to be `extern "C"`.
-                    Interned::new_str("C")
-                }
-            }
-        });
+        let abi = func.abi().map(lower_abi);
 
         let ast_id = self.source_ast_id_map.ast_id(func);
 
@@ -647,8 +633,10 @@ impl<'a> Ctx<'a> {
         Some(id(self.data().macro_defs.alloc(res)))
     }
 
-    fn lower_extern_block(&mut self, block: &ast::ExternBlock) -> Vec<ModItem> {
-        block.extern_item_list().map_or(Vec::new(), |list| {
+    fn lower_extern_block(&mut self, block: &ast::ExternBlock) -> FileItemTreeId<ExternBlock> {
+        let ast_id = self.source_ast_id_map.ast_id(block);
+        let abi = block.abi().map(lower_abi);
+        let children: Box<[_]> = block.extern_item_list().map_or(Box::new([]), |list| {
             list.extern_items()
                 .filter_map(|item| {
                     self.collect_inner_items(item.syntax());
@@ -673,13 +661,20 @@ impl<'a> Ctx<'a> {
                             self.data().type_aliases[foreign_ty.index].is_extern = true;
                             foreign_ty.into()
                         }
-                        ast::ExternItem::MacroCall(_) => return None,
+                        ast::ExternItem::MacroCall(call) => {
+                            // FIXME: we need some way of tracking that the macro call is in an
+                            // extern block
+                            self.lower_macro_call(&call)?.into()
+                        }
                     };
                     self.add_attrs(id.into(), attrs);
                     Some(id)
                 })
                 .collect()
-        })
+        });
+
+        let res = ExternBlock { abi, ast_id, children };
+        id(self.data().extern_blocks.alloc(res))
     }
 
     /// Lowers generics defined on `node` and collects inner items defined within.
@@ -879,3 +874,17 @@ fn is_intrinsic_fn_unsafe(name: &Name) -> bool {
     ]
     .contains(&name)
 }
+
+fn lower_abi(abi: ast::Abi) -> Interned<str> {
+    // FIXME: Abi::abi() -> Option<SyntaxToken>?
+    match abi.syntax().last_token() {
+        Some(tok) if tok.kind() == SyntaxKind::STRING => {
+            // FIXME: Better way to unescape?
+            Interned::new_str(tok.text().trim_matches('"'))
+        }
+        _ => {
+            // `extern` default to be `extern "C"`.
+            Interned::new_str("C")
+        }
+    }
+}
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index e76d039b8..fa4b135fd 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -1243,6 +1243,7 @@ impl ModCollector<'_, '_> {
                         status: PartialResolvedImport::Unresolved,
                     })
                 }
+                ModItem::ExternBlock(block) => self.collect(&self.item_tree[block].children),
                 ModItem::MacroCall(mac) => self.collect_macro_call(&self.item_tree[mac]),
                 ModItem::MacroRules(id) => self.collect_macro_rules(id),
                 ModItem::MacroDef(id) => self.collect_macro_def(id),
diff --git a/crates/hir_def/src/nameres/tests/macros.rs b/crates/hir_def/src/nameres/tests/macros.rs
index 04de107f5..133b2d818 100644
--- a/crates/hir_def/src/nameres/tests/macros.rs
+++ b/crates/hir_def/src/nameres/tests/macros.rs
@@ -736,6 +736,42 @@ fn unresolved_attributes_fall_back_track_per_file_moditems() {
 }
 
 #[test]
+fn unresolved_attrs_extern_block_hang() {
+    check(
+        r#"
+#[unresolved]
+extern "C" {
+    #[unresolved]
+    fn f();
+}
+    "#,
+        expect![[r#"
+        crate
+        f: v
+    "#]],
+    );
+}
+
+#[test]
+fn macros_in_extern_block() {
+    check(
+        r#"
+macro_rules! m {
+    () => { static S: u8; };
+}
+
+extern {
+    m!();
+}
+    "#,
+        expect![[r#"
+            crate
+            S: v
+        "#]],
+    );
+}
+
+#[test]
 fn resolves_derive_helper() {
     cov_mark::check!(resolved_derive_helper);
     check(