From 01c1b3dc715e6729eae7fcabdff7f2e560d61df3 Mon Sep 17 00:00:00 2001 From: hkalbasi Date: Mon, 24 Apr 2023 00:49:57 +0330 Subject: [PATCH] Fix panic in const eval and parameter destructing --- crates/hir-ty/src/mir.rs | 8 +-- crates/hir-ty/src/mir/eval.rs | 1 + crates/hir-ty/src/mir/lower.rs | 49 +++++++++++++++---- .../hir-ty/src/mir/lower/pattern_matching.rs | 2 +- crates/hir-ty/src/mir/pretty.rs | 5 ++ .../src/handlers/mutability_errors.rs | 9 ++++ 6 files changed, 57 insertions(+), 17 deletions(-) diff --git a/crates/hir-ty/src/mir.rs b/crates/hir-ty/src/mir.rs index d830ae263ba..6d8e73655ed 100644 --- a/crates/hir-ty/src/mir.rs +++ b/crates/hir-ty/src/mir.rs @@ -714,13 +714,8 @@ pub enum Rvalue { /// Creates an array where each element is the value of the operand. /// - /// This is the cause of a bug in the case where the repetition count is zero because the value - /// is not dropped, see [#74836]. - /// /// Corresponds to source code like `[x; 32]`. - /// - /// [#74836]: https://github.com/rust-lang/rust/issues/74836 - //Repeat(Operand, ty::Const), + Repeat(Operand, Const), /// Creates a reference of the indicated kind to the place. /// @@ -932,6 +927,7 @@ fn for_operand(op: &mut Operand, f: &mut impl FnMut(&mut Place)) { Rvalue::ShallowInitBox(o, _) | Rvalue::UnaryOp(_, o) | Rvalue::Cast(_, o, _) + | Rvalue::Repeat(o, _) | Rvalue::Use(o) => for_operand(o, &mut f), Rvalue::CopyForDeref(p) | Rvalue::Discriminant(p) diff --git a/crates/hir-ty/src/mir/eval.rs b/crates/hir-ty/src/mir/eval.rs index 218aab93f12..bb33da97e33 100644 --- a/crates/hir-ty/src/mir/eval.rs +++ b/crates/hir-ty/src/mir/eval.rs @@ -769,6 +769,7 @@ fn eval_rvalue<'a>( } } } + Rvalue::Repeat(_, _) => not_supported!("evaluating repeat rvalue"), Rvalue::ShallowInitBox(_, _) => not_supported!("shallow init box"), Rvalue::CopyForDeref(_) => not_supported!("copy for deref"), Rvalue::Aggregate(kind, values) => { diff --git a/crates/hir-ty/src/mir/lower.rs b/crates/hir-ty/src/mir/lower.rs index 687b9835f57..37b27b39834 100644 --- a/crates/hir-ty/src/mir/lower.rs +++ b/crates/hir-ty/src/mir/lower.rs @@ -964,7 +964,22 @@ fn lower_expr_to_place_without_adjust( self.push_assignment(current, place, r, expr_id.into()); Ok(Some(current)) } - Array::Repeat { .. } => not_supported!("array repeat"), + Array::Repeat { initializer, .. } => { + let Some((init, current)) = self.lower_expr_to_some_operand(*initializer, current)? else { + return Ok(None); + }; + let len = match &self.expr_ty(expr_id).data(Interner).kind { + TyKind::Array(_, len) => len.clone(), + _ => { + return Err(MirLowerError::TypeError( + "Array repeat expression with non array type", + )) + } + }; + let r = Rvalue::Repeat(init, len); + self.push_assignment(current, place, r, expr_id.into()); + Ok(Some(current)) + }, }, Expr::Literal(l) => { let ty = self.expr_ty(expr_id); @@ -1433,7 +1448,12 @@ fn lower_params_and_bindings( fn binding_local(&self, b: BindingId) -> Result { match self.result.binding_locals.get(b) { Some(x) => Ok(*x), - None => Err(MirLowerError::UnaccessableLocal), + None => { + // FIXME: It should never happens, but currently it will happen in `const_dependent_on_local` test, which + // is a hir lowering problem IMO. + // never!("Using unaccessable local for binding is always a bug"); + Err(MirLowerError::UnaccessableLocal) + } } } } @@ -1588,14 +1608,23 @@ pub fn lower_to_mir( } }; // 1 to param_len is for params - let current = if let DefWithBodyId::FunctionId(fid) = owner { - let substs = TyBuilder::placeholder_subst(db, fid); - let callable_sig = db.callable_item_signature(fid.into()).substitute(Interner, &substs); - ctx.lower_params_and_bindings( - body.params.iter().zip(callable_sig.params().iter()).map(|(x, y)| (*x, y.clone())), - binding_picker, - )? - } else { + // FIXME: replace with let chain once it becomes stable + let current = 'b: { + if body.body_expr == root_expr { + // otherwise it's an inline const, and has no parameter + if let DefWithBodyId::FunctionId(fid) = owner { + let substs = TyBuilder::placeholder_subst(db, fid); + let callable_sig = + db.callable_item_signature(fid.into()).substitute(Interner, &substs); + break 'b ctx.lower_params_and_bindings( + body.params + .iter() + .zip(callable_sig.params().iter()) + .map(|(x, y)| (*x, y.clone())), + binding_picker, + )?; + } + } ctx.lower_params_and_bindings([].into_iter(), binding_picker)? }; if let Some(b) = ctx.lower_expr_to_place(root_expr, return_slot().into(), current)? { diff --git a/crates/hir-ty/src/mir/lower/pattern_matching.rs b/crates/hir-ty/src/mir/lower/pattern_matching.rs index a5b48b6b936..86c3ce9eec9 100644 --- a/crates/hir-ty/src/mir/lower/pattern_matching.rs +++ b/crates/hir-ty/src/mir/lower/pattern_matching.rs @@ -129,7 +129,7 @@ pub(super) fn pattern_match( _ => not_supported!("expression path literal"), }, Pat::Bind { id, subpat } => { - let target_place = self.result.binding_locals[*id]; + let target_place = self.binding_local(*id)?; let mode = self.body.bindings[*id].mode; if let Some(subpat) = subpat { (current, current_else) = self.pattern_match( diff --git a/crates/hir-ty/src/mir/pretty.rs b/crates/hir-ty/src/mir/pretty.rs index 3e1f2ecef1b..a7dff396135 100644 --- a/crates/hir-ty/src/mir/pretty.rs +++ b/crates/hir-ty/src/mir/pretty.rs @@ -321,6 +321,11 @@ fn rvalue(&mut self, r: &Rvalue) { self.operand_list(x); w!(self, "]"); } + Rvalue::Repeat(op, len) => { + w!(self, "["); + self.operand(op); + w!(self, "; {}]", len.display(self.db)); + } Rvalue::Aggregate(AggregateKind::Adt(_, _), x) => { w!(self, "Adt("); self.operand_list(x); diff --git a/crates/ide-diagnostics/src/handlers/mutability_errors.rs b/crates/ide-diagnostics/src/handlers/mutability_errors.rs index 9184125286a..d60007b48a6 100644 --- a/crates/ide-diagnostics/src/handlers/mutability_errors.rs +++ b/crates/ide-diagnostics/src/handlers/mutability_errors.rs @@ -544,6 +544,15 @@ fn f(x: i32) { x = 5; //^^^^^ 💡 error: cannot mutate immutable variable `x` } +"#, + ); + check_diagnostics( + r#" +fn f((x, y): (i32, i32)) { + let t = [0; 2]; + x = 5; + //^^^^^ 💡 error: cannot mutate immutable variable `x` +} "#, ); }