Repository: rust-lang/rust-analyzer
Pull request #13027 (2022-08-15 15:45:55)

fix: Fix incorrect type mismatch with `cfg_if!` and other macros in expression position
Fixes https://github.com/rust-lang/rust-analyzer/issues/12940

This is a bit of a hack, ideally `MacroStmts` would not exist at all after HIR lowering, but that requires changing how the lowering code works.

jonas-schievink: @bors r+
bors: :pushpin: Commit 13009d3a446719bb2861c2565741ffb9cfbe04f5 has been approved by `jonas-schievink`

It is now in the [queue](https://bors.rust-lang.org/homu/queue/rust-analyzer) for this repository.

<!-- @bors r=jonas-schievink 13009d3a446719bb2861c2565741ffb9cfbe04f5 -->
<!-- homu: {"type":"Approved","sha":"13009d3a446719bb2861c2565741ffb9cfbe04f5","approver":"jonas-schievink","queue":"https://bors.rust-lang.org/homu/queue/rust-analyzer"} -->
bors: :hourglass: Testing commit 13009d3a446719bb2861c2565741ffb9cfbe04f5 with merge 7a1403e51aaf879319772aff1ddee602b82040f0...
<!-- homu: {"type":"BuildStarted","head_sha":"13009d3a446719bb2861c2565741ffb9cfbe04f5","merge_sha":"7a1403e51aaf879319772aff1ddee602b82040f0"} -->
bors: :broken_heart: Test failed - [checks-actions](https://github.com/rust-lang/rust-analyzer/runs/7841274298?check_suite_focus=true)
<!-- homu: {"type":"BuildFailed","builder_url":"https://github.com/rust-lang/rust-analyzer/runs/7841274298?check_suite_focus=true","builder_name":"checks-actions"} -->

Patch:
diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs
index 66f9c24e87..f6ec8bf7e9 100644
--- a/crates/hir-def/src/body/lower.rs
+++ b/crates/hir-def/src/body/lower.rs
@@ -551,9 +551,17 @@ impl ExprCollector<'_> {
                 }
             }
             ast::Expr::MacroStmts(e) => {
-                let statements = e.statements().filter_map(|s| self.collect_stmt(s)).collect();
+                let statements: Box<[_]> =
+                    e.statements().filter_map(|s| self.collect_stmt(s)).collect();
                 let tail = e.expr().map(|e| self.collect_expr(e));
 
+                if e.syntax().children().next().is_none() {
+                    // HACK: make sure that macros that expand to nothing aren't treated as a `()`
+                    // expression when used in block tail position.
+                    cov_mark::hit!(empty_macro_in_trailing_position_is_removed);
+                    return None;
+                }
+
                 self.alloc_expr(Expr::MacroStmts { tail, statements }, syntax_ptr)
             }
             ast::Expr::UnderscoreExpr(_) => self.alloc_expr(Expr::Underscore, syntax_ptr),
diff --git a/crates/hir-def/src/nameres/path_resolution.rs b/crates/hir-def/src/nameres/path_resolution.rs
index 8dfda6df64..c579bc9194 100644
--- a/crates/hir-def/src/nameres/path_resolution.rs
+++ b/crates/hir-def/src/nameres/path_resolution.rs
@@ -399,15 +399,14 @@ impl DefMap {
                 Some(_) | None => from_scope.or(from_builtin),
             },
         };
+        let from_extern_prelude = self
+            .extern_prelude
+            .get(name)
+            .map_or(PerNs::none(), |&it| PerNs::types(it.into(), Visibility::Public));
 
-        let extern_prelude = || {
-            self.extern_prelude
-                .get(name)
-                .map_or(PerNs::none(), |&it| PerNs::types(it.into(), Visibility::Public))
-        };
-        let prelude = || self.resolve_in_prelude(db, name);
+        let from_prelude = self.resolve_in_prelude(db, name);
 
-        from_legacy_macro.or(from_scope_or_builtin).or_else(extern_prelude).or_else(prelude)
+        from_legacy_macro.or(from_scope_or_builtin).or(from_extern_prelude).or(from_prelude)
     }
 
     fn resolve_name_in_crate_root_or_extern_prelude(
@@ -415,19 +414,20 @@ impl DefMap {
         db: &dyn DefDatabase,
         name: &Name,
     ) -> PerNs {
-        let from_crate_root = match self.block {
+        let arc;
+        let crate_def_map = match self.block {
             Some(_) => {
-                let def_map = self.crate_root(db).def_map(db);
-                def_map[def_map.root].scope.get(name)
+                arc = self.crate_root(db).def_map(db);
+                &arc
             }
-            None => self[self.root].scope.get(name),
-        };
-        let from_extern_prelude = || {
-            self.resolve_name_in_extern_prelude(db, name)
-                .map_or(PerNs::none(), |it| PerNs::types(it.into(), Visibility::Public))
+            None => self,
         };
+        let from_crate_root = crate_def_map[crate_def_map.root].scope.get(name);
+        let from_extern_prelude = self
+            .resolve_name_in_extern_prelude(db, name)
+            .map_or(PerNs::none(), |it| PerNs::types(it.into(), Visibility::Public));
 
-        from_crate_root.or_else(from_extern_prelude)
+        from_crate_root.or(from_extern_prelude)
     }
 
     fn resolve_in_prelude(&self, db: &dyn DefDatabase, name: &Name) -> PerNs {
diff --git a/crates/hir-def/src/per_ns.rs b/crates/hir-def/src/per_ns.rs
index 2bc1f8e926..bf5bf10c4c 100644
--- a/crates/hir-def/src/per_ns.rs
+++ b/crates/hir-def/src/per_ns.rs
@@ -43,10 +43,6 @@ impl PerNs {
         self.types.is_none() && self.values.is_none() && self.macros.is_none()
     }
 
-    pub fn is_full(&self) -> bool {
-        self.types.is_some() && self.values.is_some() && self.macros.is_some()
-    }
-
     pub fn take_types(self) -> Option<ModuleDefId> {
         self.types.map(|it| it.0)
     }
@@ -88,14 +84,6 @@ impl PerNs {
         }
     }
 
-    pub fn or_else(self, f: impl FnOnce() -> PerNs) -> PerNs {
-        if self.is_full() {
-            self
-        } else {
-            self.or(f())
-        }
-    }
-
     pub fn iter_items(self) -> impl Iterator<Item = ItemInNs> {
         let _p = profile::span("PerNs::iter_items");
         self.types
diff --git a/crates/hir-expand/src/ast_id_map.rs b/crates/hir-expand/src/ast_id_map.rs
index 11c0a6764e..c1ddef03ba 100644
--- a/crates/hir-expand/src/ast_id_map.rs
+++ b/crates/hir-expand/src/ast_id_map.rs
@@ -15,7 +15,7 @@ use std::{
 use la_arena::{Arena, Idx};
 use profile::Count;
 use rustc_hash::FxHasher;
-use syntax::{ast, AstNode, AstPtr, SyntaxNode, SyntaxNodePtr};
+use syntax::{ast, match_ast, AstNode, AstPtr, SyntaxNode, SyntaxNodePtr};
 
 /// `AstId` points to an AST node in a specific file.
 pub struct FileAstId<N: AstNode> {
@@ -92,12 +92,18 @@ impl AstIdMap {
         // change parent's id. This means that, say, adding a new function to a
         // trait does not change ids of top-level items, which helps caching.
         bdfs(node, |it| {
-            let kind = it.kind();
-            if ast::Item::can_cast(kind) || ast::BlockExpr::can_cast(kind) {
-                res.alloc(&it);
-                true
-            } else {
-                false
+            match_ast! {
+                match it {
+                    ast::Item(module_item) => {
+                        res.alloc(module_item.syntax());
+                        true
+                    },
+                    ast::BlockExpr(block) => {
+                        res.alloc(block.syntax());
+                        true
+                    },
+                    _ => false,
+                }
             }
         });
         res.map = hashbrown::HashMap::with_capacity_and_hasher(res.arena.len(), ());
@@ -117,7 +123,6 @@ impl AstIdMap {
         let raw = self.erased_ast_id(item.syntax());
         FileAstId { raw, _ty: PhantomData }
     }
-
     fn erased_ast_id(&self, item: &SyntaxNode) -> ErasedFileAstId {
         let ptr = SyntaxNodePtr::new(item);
         let hash = hash_ptr(&ptr);
diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs
index bc97ee15c7..bd60c3d268 100644
--- a/crates/hir-expand/src/db.rs
+++ b/crates/hir-expand/src/db.rs
@@ -321,11 +321,7 @@ fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet<Sy
                 ast::Item::cast(node.clone())?
                     .attrs()
                     .take(derive_attr_index as usize + 1)
-                    // FIXME, this resolution should not be done syntactically
-                    // derive is a proper macro now, no longer builtin
-                    // But we do not have resolution at this stage, this means
-                    // we need to know about all macro calls for the given ast item here
-                    // so we require some kind of mapping...
+                    // FIXME
                     .filter(|attr| attr.simple_name().as_deref() == Some("derive"))
                     .map(|it| it.syntax().clone())
                     .collect()
diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
index f6b97fd540..252293090b 100644
--- a/crates/hir-expand/src/lib.rs
+++ b/crates/hir-expand/src/lib.rs
@@ -130,6 +130,7 @@ pub struct MacroDefId {
 pub enum MacroDefKind {
     Declarative(AstId<ast::Macro>),
     BuiltIn(BuiltinFnLikeExpander, AstId<ast::Macro>),
+    // FIXME: maybe just Builtin and rename BuiltinFnLikeExpander to BuiltinExpander
     BuiltInAttr(BuiltinAttrExpander, AstId<ast::Macro>),
     BuiltInDerive(BuiltinDeriveExpander, AstId<ast::Macro>),
     BuiltInEager(EagerExpander, AstId<ast::Macro>),
diff --git a/crates/ide/src/hover.rs b/crates/ide/src/hover.rs
index fa2c6f09ab..3ada181f1e 100644
--- a/crates/ide/src/hover.rs
+++ b/crates/ide/src/hover.rs
@@ -119,8 +119,6 @@ pub(crate) fn hover(
     }
 
     let in_attr = matches!(original_token.parent().and_then(ast::TokenTree::cast), Some(tt) if tt.syntax().ancestors().any(|it| ast::Meta::can_cast(it.kind())));
-    // prefer descending the same token kind in attribute expansions, in normal macros text
-    // equivalency is more important
     let descended = if in_attr {
         [sema.descend_into_macros_with_kind_preference(original_token.clone())].into()
     } else {


Test patch:
diff --git a/crates/hir-ty/src/tests/regression.rs b/crates/hir-ty/src/tests/regression.rs
index 93a88ab58e..1b5ed0603b 100644
--- a/crates/hir-ty/src/tests/regression.rs
+++ b/crates/hir-ty/src/tests/regression.rs
@@ -1648,3 +1648,20 @@ fn main() {
         "#]],
     );
 }
+
+#[test]
+fn trailing_empty_macro() {
+    cov_mark::check!(empty_macro_in_trailing_position_is_removed);
+    check_no_mismatches(
+        r#"
+macro_rules! m2 {
+    ($($t:tt)*) => {$($t)*};
+}
+
+fn macrostmts() -> u8 {
+    m2! { 0 }
+    m2! {}
+}
+    "#,
+    );
+}