Repository: rust-analyzer/rust-analyzer
Pull request #8465 (2021-04-10 23:42:12)

Include more info in assert
This helped find https://github.com/rust-analyzer/rust-analyzer/issues/8464

changelog skip

bors r+


Patch:
diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs
index ed07d69281..bfb75a8a59 100644
--- a/crates/hir_def/src/body/lower.rs
+++ b/crates/hir_def/src/body/lower.rs
@@ -531,9 +531,8 @@ impl ExprCollector<'_> {
                 }
             }
             ast::Expr::MacroCall(e) => {
-                let macro_ptr = AstPtr::new(&e);
                 let mut ids = vec![];
-                self.collect_macro_call(e, macro_ptr, true, |this, expansion| {
+                self.collect_macro_call(e, syntax_ptr.clone(), true, |this, expansion| {
                     ids.push(match expansion {
                         Some(it) => this.collect_expr(it),
                         None => this.alloc_expr(Expr::Missing, syntax_ptr.clone()),
@@ -556,7 +555,7 @@ impl ExprCollector<'_> {
     fn collect_macro_call<F: FnMut(&mut Self, Option<T>), T: ast::AstNode>(
         &mut self,
         e: ast::MacroCall,
-        syntax_ptr: AstPtr<ast::MacroCall>,
+        syntax_ptr: AstPtr<ast::Expr>,
         is_error_recoverable: bool,
         mut collector: F,
     ) {
@@ -644,14 +643,10 @@ impl ExprCollector<'_> {
 
                 // Note that macro could be expended to multiple statements
                 if let Some(ast::Expr::MacroCall(m)) = stmt.expr() {
-                    let macro_ptr = AstPtr::new(&m);
                     let syntax_ptr = AstPtr::new(&stmt.expr().unwrap());
 
-                    self.collect_macro_call(
-                        m,
-                        macro_ptr,
-                        false,
-                        |this, expansion| match expansion {
+                    self.collect_macro_call(m, syntax_ptr.clone(), false, |this, expansion| {
+                        match expansion {
                             Some(expansion) => {
                                 let statements: ast::MacroStmts = expansion;
 
@@ -665,8 +660,8 @@ impl ExprCollector<'_> {
                                 let expr = this.alloc_expr(Expr::Missing, syntax_ptr.clone());
                                 this.statements_in_scope.push(Statement::Expr(expr));
                             }
-                        },
-                    );
+                        }
+                    });
                 } else {
                     let expr = self.collect_expr_opt(stmt.expr());
                     self.statements_in_scope.push(Statement::Expr(expr));
@@ -853,23 +848,8 @@ impl ExprCollector<'_> {
                     Pat::Missing
                 }
             }
-            ast::Pat::MacroPat(mac) => match mac.macro_call() {
-                Some(call) => {
-                    let macro_ptr = AstPtr::new(&call);
-                    let mut pat = None;
-                    self.collect_macro_call(call, macro_ptr, true, |this, expanded_pat| {
-                        pat = Some(this.collect_pat_opt(expanded_pat));
-                    });
-
-                    match pat {
-                        Some(pat) => return pat,
-                        None => Pat::Missing,
-                    }
-                }
-                None => Pat::Missing,
-            },
             // FIXME: implement
-            ast::Pat::RangePat(_) => Pat::Missing,
+            ast::Pat::RangePat(_) | ast::Pat::MacroPat(_) => Pat::Missing,
         };
         let ptr = AstPtr::new(&pat);
         self.alloc_pat(pattern, Either::Left(ptr))
diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs
index 94e08f8359..2406624862 100644
--- a/crates/hir_def/src/item_tree.rs
+++ b/crates/hir_def/src/item_tree.rs
@@ -99,11 +99,6 @@ impl ItemTree {
                     // items.
                     ctx.lower_macro_stmts(stmts)
                 },
-                ast::Pat(_pat) => {
-                    // FIXME: This occurs because macros in pattern position are treated as inner
-                    // items and expanded during block DefMap computation
-                    return Default::default();
-                },
                 ast::Expr(e) => {
                     // Macros can expand to expressions. We return an empty item tree in this case, but
                     // still need to collect inner items.
diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs
index 45b099cf31..c5629af240 100644
--- a/crates/hir_def/src/item_tree/lower.rs
+++ b/crates/hir_def/src/item_tree/lower.rs
@@ -189,7 +189,7 @@ impl Ctx {
                                 block_stack.push(self.source_ast_id_map.ast_id(&block));
                             },
                             ast::Item(item) => {
-                                // FIXME: This triggers for macro calls in expression/pattern/type position
+                                // FIXME: This triggers for macro calls in expression position
                                 let mod_items = self.lower_mod_item(&item, true);
                                 let current_block = block_stack.last();
                                 if let (Some(mod_items), Some(block)) = (mod_items, current_block) {
diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs
index ca705ee9d4..10fe608215 100644
--- a/crates/hir_expand/src/db.rs
+++ b/crates/hir_expand/src/db.rs
@@ -439,7 +439,6 @@ fn to_fragment_kind(db: &dyn AstDatabase, id: MacroCallId) -> FragmentKind {
     match parent.kind() {
         MACRO_ITEMS | SOURCE_FILE => FragmentKind::Items,
         MACRO_STMTS => FragmentKind::Statements,
-        MACRO_PAT => FragmentKind::Pattern,
         ITEM_LIST => FragmentKind::Items,
         LET_STMT => {
             // FIXME: Handle LHS Pattern
diff --git a/crates/hir_ty/src/method_resolution.rs b/crates/hir_ty/src/method_resolution.rs
index ece884241d..6178b36c8a 100644
--- a/crates/hir_ty/src/method_resolution.rs
+++ b/crates/hir_ty/src/method_resolution.rs
@@ -22,8 +22,8 @@ use crate::{
     primitive::{self, FloatTy, IntTy, UintTy},
     static_lifetime,
     utils::all_super_traits,
-    AdtId, Canonical, CanonicalVarKinds, DebruijnIndex, ForeignDefId, InEnvironment, Interner,
-    Scalar, Substitution, TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyExt, TyKind,
+    AdtId, Canonical, CanonicalVarKinds, DebruijnIndex, ForeignDefId, HirDisplay, InEnvironment,
+    Interner, Scalar, Substitution, TraitEnvironment, TraitRefExt, Ty, TyBuilder, TyExt, TyKind,
 };
 
 /// This is used as a key for indexing impls.
@@ -259,7 +259,7 @@ impl InherentImpls {
 
                 let self_ty = db.impl_self_ty(impl_id);
                 let fp = TyFingerprint::for_inherent_impl(self_ty.skip_binders());
-                always!(fp.is_some());
+                always!(fp.is_some(), "no fingerprint for {}", self_ty.skip_binders().display(db));
                 if let Some(fp) = fp {
                     map.entry(fp).or_default().push(impl_id);
                 }
diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs
index d057d5402d..ca8ccb2da1 100644
--- a/crates/ide/src/goto_definition.rs
+++ b/crates/ide/src/goto_definition.rs
@@ -1185,32 +1185,6 @@ pub mod theitem {
 pub fn gimme() -> theitem::TheItem {
     theitem::TheItem
 }
-"#,
-        );
-    }
-
-    #[test]
-    fn goto_ident_from_pat_macro() {
-        check(
-            r#"
-macro_rules! pat {
-    ($name:ident) => { Enum::Variant1($name) }
-}
-
-enum Enum {
-    Variant1(u8),
-    Variant2,
-}
-
-fn f(e: Enum) {
-    match e {
-        pat!(bind) => {
-           //^^^^
-            bind$0
-        }
-        Enum::Variant2 => {}
-    }
-}
 "#,
         );
     }
diff --git a/crates/ide_assists/src/handlers/extract_function.rs b/crates/ide_assists/src/handlers/extract_function.rs
index af95437666..5fdc8bf387 100644
--- a/crates/ide_assists/src/handlers/extract_function.rs
+++ b/crates/ide_assists/src/handlers/extract_function.rs
@@ -75,8 +75,7 @@ pub(crate) fn extract_function(acc: &mut Assists, ctx: &AssistContext) -> Option
     let insert_after = scope_for_fn_insertion(&body, anchor)?;
     let module = ctx.sema.scope(&insert_after).module()?;
 
-    let vars_defined_in_body_and_outlive =
-        vars_defined_in_body_and_outlive(ctx, &body, &node.parent().as_ref().unwrap_or(&node));
+    let vars_defined_in_body_and_outlive = vars_defined_in_body_and_outlive(ctx, &body);
     let ret_ty = body_return_ty(ctx, &body)?;
 
     // FIXME: we compute variables that outlive here just to check `never!` condition
@@ -258,7 +257,7 @@ struct Function {
     control_flow: ControlFlow,
     ret_ty: RetType,
     body: FunctionBody,
-    vars_defined_in_body_and_outlive: Vec<OutlivedLocal>,
+    vars_defined_in_body_and_outlive: Vec<Local>,
 }
 
 #[derive(Debug)]
@@ -297,9 +296,9 @@ impl Function {
             RetType::Expr(ty) => FunType::Single(ty.clone()),
             RetType::Stmt => match self.vars_defined_in_body_and_outlive.as_slice() {
                 [] => FunType::Unit,
-                [var] => FunType::Single(var.local.ty(ctx.db())),
+                [var] => FunType::Single(var.ty(ctx.db())),
                 vars => {
-                    let types = vars.iter().map(|v| v.local.ty(ctx.db())).collect();
+                    let types = vars.iter().map(|v| v.ty(ctx.db())).collect();
                     FunType::Tuple(types)
                 }
             },
@@ -563,12 +562,6 @@ impl HasTokenAtOffset for FunctionBody {
     }
 }
 
-#[derive(Debug)]
-struct OutlivedLocal {
-    local: Local,
-    mut_usage_outside_body: bool,
-}
-
 /// Try to guess what user wants to extract
 ///
 /// We have basically have two cases:
@@ -714,10 +707,10 @@ fn has_exclusive_usages(ctx: &AssistContext, usages: &LocalUsages, body: &Functi
         .any(|reference| reference_is_exclusive(reference, body, ctx))
 }
 
-/// checks if this reference requires `&mut` access inside node
+/// checks if this reference requires `&mut` access inside body
 fn reference_is_exclusive(
     reference: &FileReference,
-    node: &dyn HasTokenAtOffset,
+    body: &FunctionBody,
     ctx: &AssistContext,
 ) -> bool {
     // we directly modify variable with set: `n = 0`, `n += 1`
@@ -726,7 +719,7 @@ fn reference_is_exclusive(
     }
 
     // we take `&mut` reference to variable: `&mut v`
-    let path = match path_element_of_reference(node, reference) {
+    let path = match path_element_of_reference(body, reference) {
         Some(path) => path,
         None => return false,
     };
@@ -827,16 +820,10 @@ fn vars_defined_in_body(body: &FunctionBody, ctx: &AssistContext) -> Vec<Local>
 }
 
 /// list local variables defined inside `body` that should be returned from extracted function
-fn vars_defined_in_body_and_outlive(
-    ctx: &AssistContext,
-    body: &FunctionBody,
-    parent: &SyntaxNode,
-) -> Vec<OutlivedLocal> {
-    let vars_defined_in_body = vars_defined_in_body(&body, ctx);
+fn vars_defined_in_body_and_outlive(ctx: &AssistContext, body: &FunctionBody) -> Vec<Local> {
+    let mut vars_defined_in_body = vars_defined_in_body(&body, ctx);
+    vars_defined_in_body.retain(|var| var_outlives_body(ctx, body, var));
     vars_defined_in_body
-        .into_iter()
-        .filter_map(|var| var_outlives_body(ctx, body, var, parent))
-        .collect()
 }
 
 /// checks if the relevant local was defined before(outside of) body
@@ -856,23 +843,11 @@ fn either_syntax(value: &Either<ast::IdentPat, ast::SelfParam>) -> &SyntaxNode {
     }
 }
 
-/// returns usage details if local variable is used after(outside of) body
-fn var_outlives_body(
-    ctx: &AssistContext,
-    body: &FunctionBody,
-    var: Local,
-    parent: &SyntaxNode,
-) -> Option<OutlivedLocal> {
-    let usages = LocalUsages::find(ctx, var);
+/// checks if local variable is used after(outside of) body
+fn var_outlives_body(ctx: &AssistContext, body: &FunctionBody, var: &Local) -> bool {
+    let usages = LocalUsages::find(ctx, *var);
     let has_usages = usages.iter().any(|reference| body.preceedes_range(reference.range));
-    if !has_usages {
-        return None;
-    }
-    let has_mut_usages = usages
-        .iter()
-        .filter(|reference| body.preceedes_range(reference.range))
-        .any(|reference| reference_is_exclusive(reference, parent, ctx));
-    Some(OutlivedLocal { local: var, mut_usage_outside_body: has_mut_usages })
+    has_usages
 }
 
 fn body_return_ty(ctx: &AssistContext, body: &FunctionBody) -> Option<RetType> {
@@ -952,25 +927,16 @@ fn format_replacement(ctx: &AssistContext, fun: &Function, indent: IndentLevel)
     let mut buf = String::new();
     match fun.vars_defined_in_body_and_outlive.as_slice() {
         [] => {}
-        [var] => {
-            format_to!(buf, "let {}{} = ", mut_modifier(var), var.local.name(ctx.db()).unwrap())
-        }
+        [var] => format_to!(buf, "let {} = ", var.name(ctx.db()).unwrap()),
         [v0, vs @ ..] => {
             buf.push_str("let (");
-            format_to!(buf, "{}{}", mut_modifier(v0), v0.local.name(ctx.db()).unwrap());
+            format_to!(buf, "{}", v0.name(ctx.db()).unwrap());
             for var in vs {
-                format_to!(buf, ", {}{}", mut_modifier(var), var.local.name(ctx.db()).unwrap());
+                format_to!(buf, ", {}", var.name(ctx.db()).unwrap());
             }
             buf.push_str(") = ");
         }
     }
-    fn mut_modifier(var: &OutlivedLocal) -> &'static str {
-        if var.mut_usage_outside_body {
-            "mut "
-        } else {
-            ""
-        }
-    }
     format_to!(buf, "{}", expr);
     if fun.ret_ty.is_unit()
         && (!fun.vars_defined_in_body_and_outlive.is_empty() || !expr.is_block_like())
@@ -1233,10 +1199,10 @@ fn make_body(
                 match fun.vars_defined_in_body_and_outlive.as_slice() {
                     [] => {}
                     [var] => {
-                        tail_expr = Some(path_expr_from_local(ctx, var.local));
+                        tail_expr = Some(path_expr_from_local(ctx, *var));
                     }
                     vars => {
-                        let exprs = vars.iter().map(|var| path_expr_from_local(ctx, var.local));
+                        let exprs = vars.iter().map(|var| path_expr_from_local(ctx, *var));
                         let expr = make::expr_tuple(exprs);
                         tail_expr = Some(expr);
                     }
@@ -2144,30 +2110,6 @@ fn $0fun_name(n: i32) -> i32 {
         );
     }
 
-    #[test]
-    fn variable_defined_inside_and_used_after_mutably_no_ret() {
-        check_assist(
-            extract_function,
-            r"
-fn foo() {
-    let n = 1;
-    $0let mut k = n * n;$0
-    k += 1;
-}",
-            r"
-fn foo() {
-    let n = 1;
-    let mut k = fun_name(n);
-    k += 1;
-}
-
-fn $0fun_name(n: i32) -> i32 {
-    let mut k = n * n;
-    k
-}",
-        );
-    }
-
     #[test]
     fn two_variables_defined_inside_and_used_after_no_ret() {
         check_assist(
@@ -2194,38 +2136,6 @@ fn $0fun_name(n: i32) -> (i32, i32) {
         );
     }
 
-    #[test]
-    fn multi_variables_defined_inside_and_used_after_mutably_no_ret() {
-        check_assist(
-            extract_function,
-            r"
-fn foo() {
-    let n = 1;
-    $0let mut k = n * n;
-    let mut m = k + 2;
-    let mut o = m + 3;
-    o += 1;$0
-    k += o;
-    m = 1;
-}",
-            r"
-fn foo() {
-    let n = 1;
-    let (mut k, mut m, o) = fun_name(n);
-    k += o;
-    m = 1;
-}
-
-fn $0fun_name(n: i32) -> (i32, i32, i32) {
-    let mut k = n * n;
-    let mut m = k + 2;
-    let mut o = m + 3;
-    o += 1;
-    (k, m, o)
-}",
-        );
-    }
-
     #[test]
     fn nontrivial_patterns_define_variables() {
         check_assist(


Test patch:
diff --git a/crates/hir_ty/src/tests/macros.rs b/crates/hir_ty/src/tests/macros.rs
index b8e373ed88..86e3d8b86e 100644
--- a/crates/hir_ty/src/tests/macros.rs
+++ b/crates/hir_ty/src/tests/macros.rs
@@ -1065,11 +1065,11 @@ fn macro_in_arm() {
         }
         "#,
         expect![[r#"
-            !0..2 '()': ()
             51..110 '{     ...  }; }': ()
             61..62 'x': u32
             65..107 'match ...     }': u32
             71..73 '()': ()
+            84..91 'unit!()': ()
             95..100 '92u32': u32
         "#]],
     );
diff --git a/crates/hir_ty/src/tests/patterns.rs b/crates/hir_ty/src/tests/patterns.rs
index f514b3efe6..85a28e76b2 100644
--- a/crates/hir_ty/src/tests/patterns.rs
+++ b/crates/hir_ty/src/tests/patterns.rs
@@ -1,6 +1,6 @@
 use expect_test::expect;
 
-use super::{check_infer, check_infer_with_mismatches, check_types};
+use super::{check_infer, check_infer_with_mismatches};
 
 #[test]
 fn infer_pattern() {
@@ -825,29 +825,3 @@ fn foo(foo: Foo) {
         "#]],
     );
 }
-
-#[test]
-fn macro_pat() {
-    check_types(
-        r#"
-macro_rules! pat {
-    ($name:ident) => { Enum::Variant1($name) }
-}
-
-enum Enum {
-    Variant1(u8),
-    Variant2,
-}
-
-fn f(e: Enum) {
-    match e {
-        pat!(bind) => {
-            bind;
-          //^^^^ u8
-        }
-        Enum::Variant2 => {}
-    }
-}
-    "#,
-    )
-}