Skip to content

Commit e6728a8

Browse files
bors[bot]Brandon
andauthored
Merge #8415
8415: Fix faulty assertion when extracting function with macro call r=matklad a=brandondong **Reproduction:** ```rust fn main() { let n = 1; let k = n * n; dbg!(n); } ``` 1. Select the second and third lines of the main function. Use the "Extract into function" code assist. 2. Panic occurs in debug, error is logged in release: "[ERROR ide_assists::handlers::extract_function] assertion failed: matches!(path, ast :: Expr :: PathExpr(_))". 3. Function generates successfully on release where the panic was bypassed. ```rust fn fun_name(n: i32) { let k = n * n; dbg!(n); } ``` **Cause:** - The generated function will take `n` as a parameter. The extraction logic needs to search the usages of `n` to determine whether it is used mutably or not. The helper `path_element_of_reference` is called for each usage but the second usage is a macro call and fails the `Expr::PathExpr(_)` match assertion. - The caller of `path_element_of_reference` does implicitly assume it to be a `Expr::PathExpr(_)` in how it looks at its parent node for determining whether it is used mutably. This logic will not work for macros. - I'm not sure if there are any other cases besides macros where it could be something other than a `Expr::PathExpr(_)`. I tried various examples and could not find any. **Fix:** - Update assertion to include the macro case. - Add a FIXME to properly handle checking if a macro usage requires mutable access. For now, return false instead of running the existing logic that is tailored for `Expr::PathExpr(_)`'s. Co-authored-by: Brandon <brandondong604@hotmail.com>
2 parents 0c02208 + 09a78ca commit e6728a8

File tree

1 file changed

+46
-2
lines changed

1 file changed

+46
-2
lines changed

crates/ide_assists/src/handlers/extract_function.rs

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,14 @@ fn reference_is_exclusive(
736736

737737
/// checks if this expr requires `&mut` access, recurses on field access
738738
fn expr_require_exclusive_access(ctx: &AssistContext, expr: &ast::Expr) -> Option<bool> {
739+
match expr {
740+
ast::Expr::MacroCall(_) => {
741+
// FIXME: expand macro and check output for mutable usages of the variable?
742+
return None;
743+
}
744+
_ => (),
745+
}
746+
739747
let parent = expr.syntax().parent()?;
740748

741749
if let Some(bin_expr) = ast::BinExpr::cast(parent.clone()) {
@@ -794,7 +802,7 @@ impl HasTokenAtOffset for SyntaxNode {
794802
}
795803
}
796804

797-
/// find relevant `ast::PathExpr` for reference
805+
/// find relevant `ast::Expr` for reference
798806
///
799807
/// # Preconditions
800808
///
@@ -811,7 +819,11 @@ fn path_element_of_reference(
811819
stdx::never!(false, "cannot find path parent of variable usage: {:?}", token);
812820
None
813821
})?;
814-
stdx::always!(matches!(path, ast::Expr::PathExpr(_)));
822+
stdx::always!(
823+
matches!(path, ast::Expr::PathExpr(_) | ast::Expr::MacroCall(_)),
824+
"unexpected expression type for variable usage: {:?}",
825+
path
826+
);
815827
Some(path)
816828
}
817829

@@ -3462,4 +3474,36 @@ fn foo() -> Result<(), i64> {
34623474
}"##,
34633475
);
34643476
}
3477+
3478+
#[test]
3479+
fn param_usage_in_macro() {
3480+
check_assist(
3481+
extract_function,
3482+
r"
3483+
macro_rules! m {
3484+
($val:expr) => { $val };
3485+
}
3486+
3487+
fn foo() {
3488+
let n = 1;
3489+
$0let k = n * m!(n);$0
3490+
let m = k + 1;
3491+
}",
3492+
r"
3493+
macro_rules! m {
3494+
($val:expr) => { $val };
3495+
}
3496+
3497+
fn foo() {
3498+
let n = 1;
3499+
let k = fun_name(n);
3500+
let m = k + 1;
3501+
}
3502+
3503+
fn $0fun_name(n: i32) -> i32 {
3504+
let k = n * m!(n);
3505+
k
3506+
}",
3507+
);
3508+
}
34653509
}

0 commit comments

Comments
 (0)