From ac04bfd7a7223cc7077f6745c35f8b655ea13c73 Mon Sep 17 00:00:00 2001 From: hkalbasi Date: Sun, 26 Feb 2023 16:04:41 +0330 Subject: [PATCH] Add `View Mir` command and fix some bugs --- crates/hir-ty/src/db.rs | 5 +- crates/hir-ty/src/display.rs | 1 + crates/hir-ty/src/mir.rs | 39 +- crates/hir-ty/src/mir/borrowck.rs | 73 ++-- crates/hir-ty/src/mir/lower.rs | 70 ++-- crates/hir-ty/src/mir/pretty.rs | 348 ++++++++++++++++++ crates/hir/src/has_source.rs | 12 +- crates/hir/src/lib.rs | 58 +-- crates/ide-db/src/rename.rs | 3 +- .../src/handlers/mutability_errors.rs | 257 +++++++++++-- crates/ide/src/lib.rs | 5 + crates/ide/src/rename.rs | 22 ++ crates/ide/src/view_mir.rs | 29 ++ crates/rust-analyzer/src/handlers.rs | 10 + crates/rust-analyzer/src/lsp_ext.rs | 8 + crates/rust-analyzer/src/main_loop.rs | 1 + crates/syntax/src/ast/traits.rs | 2 + docs/dev/lsp-extensions.md | 13 +- editors/code/package.json | 5 + editors/code/src/commands.ts | 27 +- editors/code/src/lsp_ext.ts | 3 + editors/code/src/main.ts | 1 + lib/la-arena/src/map.rs | 6 + 23 files changed, 876 insertions(+), 122 deletions(-) create mode 100644 crates/hir-ty/src/mir/pretty.rs create mode 100644 crates/ide/src/view_mir.rs diff --git a/crates/hir-ty/src/db.rs b/crates/hir-ty/src/db.rs index 60e51b65f6..304c78767f 100644 --- a/crates/hir-ty/src/db.rs +++ b/crates/hir-ty/src/db.rs @@ -18,7 +18,7 @@ use crate::{ chalk_db, consteval::ConstEvalError, method_resolution::{InherentImpls, TraitImpls, TyFingerprint}, - mir::{MirBody, MirLowerError}, + mir::{BorrowckResult, MirBody, MirLowerError}, Binders, CallableDefId, Const, FnDefId, GenericArg, ImplTraitId, InferenceResult, Interner, PolyFnSig, QuantifiedWhereClause, ReturnTypeImplTraits, Substitution, TraitRef, Ty, TyDefId, ValueTyDefId, @@ -38,6 +38,9 @@ pub trait HirDatabase: DefDatabase + Upcast { #[salsa::cycle(crate::mir::mir_body_recover)] fn mir_body(&self, def: DefWithBodyId) -> Result, MirLowerError>; + #[salsa::invoke(crate::mir::borrowck_query)] + fn borrowck(&self, def: DefWithBodyId) -> Result, MirLowerError>; + #[salsa::invoke(crate::lower::ty_query)] #[salsa::cycle(crate::lower::ty_recover)] fn ty(&self, def: TyDefId) -> Binders; diff --git a/crates/hir-ty/src/display.rs b/crates/hir-ty/src/display.rs index 6dde477360..bd3eccfe43 100644 --- a/crates/hir-ty/src/display.rs +++ b/crates/hir-ty/src/display.rs @@ -531,6 +531,7 @@ fn render_const_scalar( hir_def::AdtId::UnionId(u) => write!(f, "{}", f.db.union_data(u).name), hir_def::AdtId::EnumId(_) => f.write_str(""), }, + chalk_ir::TyKind::FnDef(..) => ty.hir_fmt(f), _ => f.write_str(""), } } diff --git a/crates/hir-ty/src/mir.rs b/crates/hir-ty/src/mir.rs index 5d8a81a3ee..c18a34a192 100644 --- a/crates/hir-ty/src/mir.rs +++ b/crates/hir-ty/src/mir.rs @@ -1,6 +1,6 @@ //! MIR definitions and implementation -use std::iter; +use std::{fmt::Display, iter}; use crate::{ infer::PointerCast, Const, ConstScalar, InferenceResult, Interner, MemoryMap, Substitution, Ty, @@ -14,8 +14,10 @@ use la_arena::{Arena, ArenaMap, Idx, RawIdx}; mod eval; mod lower; -pub mod borrowck; +mod borrowck; +mod pretty; +pub use borrowck::{borrowck_query, BorrowckResult, MutabilityReason}; pub use eval::{interpret_mir, pad16, Evaluator, MirEvalError}; pub use lower::{lower_to_mir, mir_body_query, mir_body_recover, MirLowerError}; use smallvec::{smallvec, SmallVec}; @@ -32,13 +34,7 @@ fn return_slot() -> LocalId { #[derive(Debug, PartialEq, Eq)] pub struct Local { - pub mutability: Mutability, - //pub local_info: Option>, - //pub internal: bool, - //pub is_block_tail: Option, pub ty: Ty, - //pub user_ty: Option>, - //pub source_info: SourceInfo, } /// An operand in MIR represents a "value" in Rust, the definition of which is undecided and part of @@ -564,6 +560,30 @@ pub enum BinOp { Offset, } +impl Display for BinOp { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + BinOp::Add => "+", + BinOp::Sub => "-", + BinOp::Mul => "*", + BinOp::Div => "/", + BinOp::Rem => "%", + BinOp::BitXor => "^", + BinOp::BitAnd => "&", + BinOp::BitOr => "|", + BinOp::Shl => "<<", + BinOp::Shr => ">>", + BinOp::Eq => "==", + BinOp::Lt => "<", + BinOp::Le => "<=", + BinOp::Ne => "!=", + BinOp::Ge => ">=", + BinOp::Gt => ">", + BinOp::Offset => "`offset`", + }) + } +} + impl From for BinOp { fn from(value: hir_def::expr::ArithOp) -> Self { match value { @@ -822,10 +842,9 @@ pub struct MirBody { pub owner: DefWithBodyId, pub arg_count: usize, pub binding_locals: ArenaMap, + pub param_locals: Vec, } -impl MirBody {} - fn const_as_usize(c: &Const) -> usize { try_const_usize(c).unwrap() as usize } diff --git a/crates/hir-ty/src/mir/borrowck.rs b/crates/hir-ty/src/mir/borrowck.rs index fcf9a67fe8..7bd0f888b3 100644 --- a/crates/hir-ty/src/mir/borrowck.rs +++ b/crates/hir-ty/src/mir/borrowck.rs @@ -1,22 +1,43 @@ //! MIR borrow checker, which is used in diagnostics like `unused_mut` // Currently it is an ad-hoc implementation, only useful for mutability analysis. Feel free to remove all of these -// and implement a proper borrow checker. +// if needed for implementing a proper borrow checker. +use std::sync::Arc; + +use hir_def::DefWithBodyId; use la_arena::ArenaMap; use stdx::never; +use crate::db::HirDatabase; + use super::{ - BasicBlockId, BorrowKind, LocalId, MirBody, MirSpan, Place, ProjectionElem, Rvalue, - StatementKind, Terminator, + BasicBlockId, BorrowKind, LocalId, MirBody, MirLowerError, MirSpan, Place, ProjectionElem, + Rvalue, StatementKind, Terminator, }; -#[derive(Debug)] -pub enum Mutability { - Mut { span: MirSpan }, +#[derive(Debug, Clone, PartialEq, Eq)] +/// Stores spans which implies that the local should be mutable. +pub enum MutabilityReason { + Mut { spans: Vec }, Not, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BorrowckResult { + pub mir_body: Arc, + pub mutability_of_locals: ArenaMap, +} + +pub fn borrowck_query( + db: &dyn HirDatabase, + def: DefWithBodyId, +) -> Result, MirLowerError> { + let body = db.mir_body(def)?; + let r = BorrowckResult { mutability_of_locals: mutability_of_locals(&body), mir_body: body }; + Ok(Arc::new(r)) +} + fn is_place_direct(lvalue: &Place) -> bool { !lvalue.projection.iter().any(|x| *x == ProjectionElem::Deref) } @@ -116,28 +137,28 @@ fn ever_initialized_map(body: &MirBody) -> ArenaMap ArenaMap { - let mut result: ArenaMap = - body.locals.iter().map(|x| (x.0, Mutability::Not)).collect(); +fn mutability_of_locals(body: &MirBody) -> ArenaMap { + let mut result: ArenaMap = + body.locals.iter().map(|x| (x.0, MutabilityReason::Not)).collect(); + let mut push_mut_span = |local, span| match &mut result[local] { + MutabilityReason::Mut { spans } => spans.push(span), + x @ MutabilityReason::Not => *x = MutabilityReason::Mut { spans: vec![span] }, + }; let ever_init_maps = ever_initialized_map(body); - for (block_id, ever_init_map) in ever_init_maps.iter() { - let mut ever_init_map = ever_init_map.clone(); + for (block_id, mut ever_init_map) in ever_init_maps.into_iter() { let block = &body.basic_blocks[block_id]; for statement in &block.statements { match &statement.kind { @@ -145,20 +166,20 @@ pub fn mutability_of_locals(body: &MirBody) -> ArenaMap { match place_case(place) { ProjectionCase::Direct => { if ever_init_map.get(place.local).copied().unwrap_or_default() { - result[place.local] = Mutability::Mut { span: statement.span }; + push_mut_span(place.local, statement.span); } else { ever_init_map.insert(place.local, true); } } ProjectionCase::DirectPart => { // Partial initialization is not supported, so it is definitely `mut` - result[place.local] = Mutability::Mut { span: statement.span }; + push_mut_span(place.local, statement.span); } ProjectionCase::Indirect => (), } if let Rvalue::Ref(BorrowKind::Mut { .. }, p) = value { if is_place_direct(p) { - result[p.local] = Mutability::Mut { span: statement.span }; + push_mut_span(p.local, statement.span); } } } @@ -189,7 +210,7 @@ pub fn mutability_of_locals(body: &MirBody) -> ArenaMap { Terminator::Call { destination, .. } => { if destination.projection.len() == 0 { if ever_init_map.get(destination.local).copied().unwrap_or_default() { - result[destination.local] = Mutability::Mut { span: MirSpan::Unknown }; + push_mut_span(destination.local, MirSpan::Unknown); } else { ever_init_map.insert(destination.local, true); } diff --git a/crates/hir-ty/src/mir/lower.rs b/crates/hir-ty/src/mir/lower.rs index 73ae5eaeef..e89e16079d 100644 --- a/crates/hir-ty/src/mir/lower.rs +++ b/crates/hir-ty/src/mir/lower.rs @@ -16,7 +16,7 @@ use hir_def::{ use la_arena::ArenaMap; use crate::{ - consteval::ConstEvalError, db::HirDatabase, infer::TypeMismatch, + consteval::ConstEvalError, db::HirDatabase, display::HirDisplay, infer::TypeMismatch, inhabitedness::is_ty_uninhabited_from, layout::layout_of_ty, mapping::ToChalk, utils::generics, Adjust, AutoBorrow, CallableDefId, TyBuilder, TyExt, }; @@ -46,6 +46,7 @@ pub enum MirLowerError { LayoutError(LayoutError), IncompleteExpr, UnresolvedName(String), + RecordLiteralWithoutPath, UnresolvedMethod, UnresolvedField, MissingFunctionDefinition, @@ -88,7 +89,7 @@ impl MirLowerCtx<'_> { if matches!(ty.kind(Interner), TyKind::Slice(_) | TyKind::Dyn(_)) { not_supported!("unsized temporaries"); } - Ok(self.result.locals.alloc(Local { mutability: Mutability::Not, ty })) + Ok(self.result.locals.alloc(Local { ty })) } fn lower_expr_as_place(&self, expr_id: ExprId) -> Option { @@ -251,7 +252,7 @@ impl MirLowerCtx<'_> { match &self.body.exprs[expr_id] { Expr::Missing => Err(MirLowerError::IncompleteExpr), Expr::Path(p) => { - let unresolved_name = || MirLowerError::UnresolvedName("".to_string()); + let unresolved_name = || MirLowerError::UnresolvedName(p.display(self.db).to_string()); let resolver = resolver_for_expr(self.db.upcast(), self.owner, expr_id); let pr = resolver .resolve_path_in_value_ns(self.db.upcast(), p.mod_path()) @@ -259,19 +260,29 @@ impl MirLowerCtx<'_> { let pr = match pr { ResolveValueResult::ValueNs(v) => v, ResolveValueResult::Partial(..) => { - return match self + if let Some(assoc) = self .infer .assoc_resolutions_for_expr(expr_id) - .ok_or_else(unresolved_name)? - .0 - //.ok_or(ConstEvalError::SemanticError("unresolved assoc item"))? { - hir_def::AssocItemId::ConstId(c) => { - self.lower_const(c, current, place, expr_id.into())?; - Ok(Some(current)) - }, - _ => return Err(unresolved_name()), - }; + match assoc.0 { + hir_def::AssocItemId::ConstId(c) => { + self.lower_const(c, current, place, expr_id.into())?; + return Ok(Some(current)) + }, + _ => not_supported!("associated functions and types"), + } + } else if let Some(variant) = self + .infer + .variant_resolution_for_expr(expr_id) + { + match variant { + VariantId::EnumVariantId(e) => ValueNs::EnumVariantId(e), + VariantId::StructId(s) => ValueNs::StructId(s), + VariantId::UnionId(_) => return Err(MirLowerError::ImplementationError("Union variant as path")), + } + } else { + return Err(unresolved_name()); + } } }; match pr { @@ -597,11 +608,14 @@ impl MirLowerCtx<'_> { Ok(None) } Expr::Yield { .. } => not_supported!("yield"), - Expr::RecordLit { fields, .. } => { + Expr::RecordLit { fields, path, .. } => { let variant_id = self .infer .variant_resolution_for_expr(expr_id) - .ok_or_else(|| MirLowerError::UnresolvedName("".to_string()))?; + .ok_or_else(|| match path { + Some(p) => MirLowerError::UnresolvedName(p.display(self.db).to_string()), + None => MirLowerError::RecordLiteralWithoutPath, + })?; let subst = match self.expr_ty(expr_id).kind(Interner) { TyKind::Adt(_, s) => s.clone(), _ => not_supported!("Non ADT record literal"), @@ -1437,17 +1451,17 @@ pub fn lower_to_mir( basic_blocks.alloc(BasicBlock { statements: vec![], terminator: None, is_cleanup: false }); let mut locals = Arena::new(); // 0 is return local - locals.alloc(Local { mutability: Mutability::Mut, ty: infer[root_expr].clone() }); + locals.alloc(Local { ty: infer[root_expr].clone() }); let mut binding_locals: ArenaMap = ArenaMap::new(); - let param_locals: ArenaMap = if let DefWithBodyId::FunctionId(fid) = owner { + // 1 to param_len is for params + let param_locals: Vec = 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); - // 1 to param_len is for params body.params .iter() .zip(callable_sig.params().iter()) .map(|(&x, ty)| { - let local_id = locals.alloc(Local { mutability: Mutability::Not, ty: ty.clone() }); + let local_id = locals.alloc(Local { ty: ty.clone() }); if let Pat::Bind { id, subpat: None } = body[x] { if matches!( body.bindings[id].mode, @@ -1456,22 +1470,19 @@ pub fn lower_to_mir( binding_locals.insert(id, local_id); } } - (x, local_id) + local_id }) .collect() } else { if !body.params.is_empty() { return Err(MirLowerError::TypeError("Unexpected parameter for non function body")); } - ArenaMap::new() + vec![] }; // and then rest of bindings for (id, _) in body.bindings.iter() { if !binding_locals.contains_idx(id) { - binding_locals.insert( - id, - locals.alloc(Local { mutability: Mutability::Not, ty: infer[id].clone() }), - ); + binding_locals.insert(id, locals.alloc(Local { ty: infer[id].clone() })); } } let mir = MirBody { @@ -1479,6 +1490,7 @@ pub fn lower_to_mir( locals, start_block, binding_locals, + param_locals, owner, arg_count: body.params.len(), }; @@ -1492,17 +1504,17 @@ pub fn lower_to_mir( discr_temp: None, }; let mut current = start_block; - for ¶m in &body.params { + for (¶m, local) in body.params.iter().zip(ctx.result.param_locals.clone().into_iter()) { if let Pat::Bind { id, .. } = body[param] { - if param_locals[param] == ctx.result.binding_locals[id] { + if local == ctx.result.binding_locals[id] { continue; } } let r = ctx.pattern_match( current, None, - param_locals[param].into(), - ctx.result.locals[param_locals[param]].ty.clone(), + local.into(), + ctx.result.locals[local].ty.clone(), param, BindingAnnotation::Unannotated, )?; diff --git a/crates/hir-ty/src/mir/pretty.rs b/crates/hir-ty/src/mir/pretty.rs new file mode 100644 index 0000000000..ffc08b7e34 --- /dev/null +++ b/crates/hir-ty/src/mir/pretty.rs @@ -0,0 +1,348 @@ +//! A pretty-printer for MIR. + +use std::fmt::{Display, Write}; + +use hir_def::{body::Body, expr::BindingId}; +use hir_expand::name::Name; +use la_arena::ArenaMap; + +use crate::{ + db::HirDatabase, + display::HirDisplay, + mir::{PlaceElem, ProjectionElem, StatementKind, Terminator}, +}; + +use super::{ + AggregateKind, BasicBlockId, BorrowKind, LocalId, MirBody, Operand, Place, Rvalue, UnOp, +}; + +impl MirBody { + pub fn pretty_print(&self, db: &dyn HirDatabase) -> String { + let hir_body = db.body(self.owner); + let mut ctx = MirPrettyCtx::new(self, &hir_body, db); + ctx.for_body(); + ctx.result + } +} + +struct MirPrettyCtx<'a> { + body: &'a MirBody, + hir_body: &'a Body, + db: &'a dyn HirDatabase, + result: String, + ident: String, + local_to_binding: ArenaMap, +} + +macro_rules! w { + ($dst:expr, $($arg:tt)*) => { + { let _ = write!($dst, $($arg)*); } + }; +} + +macro_rules! wln { + ($dst:expr) => { + { let _ = writeln!($dst); } + }; + ($dst:expr, $($arg:tt)*) => { + { let _ = writeln!($dst, $($arg)*); } + }; +} + +impl Write for MirPrettyCtx<'_> { + fn write_str(&mut self, s: &str) -> std::fmt::Result { + let mut it = s.split('\n'); // note: `.lines()` is wrong here + self.write(it.next().unwrap_or_default()); + for line in it { + self.write_line(); + self.write(line); + } + Ok(()) + } +} + +enum LocalName { + Unknown(LocalId), + Binding(Name, LocalId), +} + +impl Display for LocalName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + LocalName::Unknown(l) => write!(f, "_{}", u32::from(l.into_raw())), + LocalName::Binding(n, l) => write!(f, "{n}_{}", u32::from(l.into_raw())), + } + } +} + +impl<'a> MirPrettyCtx<'a> { + fn for_body(&mut self) { + self.with_block(|this| { + this.locals(); + wln!(this); + this.blocks(); + }); + } + + fn with_block(&mut self, f: impl FnOnce(&mut MirPrettyCtx<'_>)) { + self.ident += " "; + wln!(self, "{{"); + f(self); + for _ in 0..4 { + self.result.pop(); + self.ident.pop(); + } + wln!(self, "}}"); + } + + fn new(body: &'a MirBody, hir_body: &'a Body, db: &'a dyn HirDatabase) -> Self { + let local_to_binding = body.binding_locals.iter().map(|(x, y)| (*y, x)).collect(); + MirPrettyCtx { + body, + db, + result: String::new(), + ident: String::new(), + local_to_binding, + hir_body, + } + } + + fn write_line(&mut self) { + self.result.push('\n'); + self.result += &self.ident; + } + + fn write(&mut self, line: &str) { + self.result += line; + } + + fn locals(&mut self) { + for (id, local) in self.body.locals.iter() { + wln!(self, "let {}: {};", self.local_name(id), local.ty.display(self.db)); + } + } + + fn local_name(&self, local: LocalId) -> LocalName { + match self.local_to_binding.get(local) { + Some(b) => LocalName::Binding(self.hir_body.bindings[*b].name.clone(), local), + None => LocalName::Unknown(local), + } + } + + fn basic_block_id(&self, basic_block_id: BasicBlockId) -> String { + format!("'bb{}", u32::from(basic_block_id.into_raw())) + } + + fn blocks(&mut self) { + for (id, block) in self.body.basic_blocks.iter() { + wln!(self); + w!(self, "{}: ", self.basic_block_id(id)); + self.with_block(|this| { + for statement in &block.statements { + match &statement.kind { + StatementKind::Assign(l, r) => { + this.place(l); + w!(this, " = "); + this.rvalue(r); + wln!(this, ";"); + } + StatementKind::StorageDead(p) => { + wln!(this, "StorageDead({})", this.local_name(*p)); + } + StatementKind::StorageLive(p) => { + wln!(this, "StorageLive({})", this.local_name(*p)); + } + StatementKind::Deinit(p) => { + w!(this, "Deinit("); + this.place(p); + wln!(this, ");"); + } + StatementKind::Nop => wln!(this, "Nop;"), + } + } + match &block.terminator { + Some(terminator) => match terminator { + Terminator::Goto { target } => { + wln!(this, "goto 'bb{};", u32::from(target.into_raw())) + } + Terminator::SwitchInt { discr, targets } => { + w!(this, "switch "); + this.operand(discr); + w!(this, " "); + this.with_block(|this| { + for (c, b) in targets.iter() { + wln!(this, "{c} => {},", this.basic_block_id(b)); + } + wln!(this, "_ => {},", this.basic_block_id(targets.otherwise())); + }); + } + Terminator::Call { func, args, destination, target, .. } => { + w!(this, "Call "); + this.with_block(|this| { + w!(this, "func: "); + this.operand(func); + wln!(this, ","); + w!(this, "args: ["); + this.operand_list(args); + wln!(this, "],"); + w!(this, "destination: "); + this.place(destination); + wln!(this, ","); + w!(this, "target: "); + match target { + Some(t) => w!(this, "{}", this.basic_block_id(*t)), + None => w!(this, ""), + } + wln!(this, ","); + }); + } + _ => wln!(this, "{:?};", terminator), + }, + None => wln!(this, ";"), + } + }) + } + } + + fn place(&mut self, p: &Place) { + fn f(this: &mut MirPrettyCtx<'_>, local: LocalId, projections: &[PlaceElem]) { + let Some((last, head)) = projections.split_last() else { + // no projection + w!(this, "{}", this.local_name(local)); + return; + }; + match last { + ProjectionElem::Deref => { + w!(this, "(*"); + f(this, local, head); + w!(this, ")"); + } + ProjectionElem::Field(field) => { + let variant_data = field.parent.variant_data(this.db.upcast()); + let name = &variant_data.fields()[field.local_id].name; + match field.parent { + hir_def::VariantId::EnumVariantId(e) => { + w!(this, "("); + f(this, local, head); + let variant_name = + &this.db.enum_data(e.parent).variants[e.local_id].name; + w!(this, " as {}).{}", variant_name, name); + } + hir_def::VariantId::StructId(_) | hir_def::VariantId::UnionId(_) => { + f(this, local, head); + w!(this, ".{name}"); + } + } + } + ProjectionElem::TupleField(x) => { + f(this, local, head); + w!(this, ".{}", x); + } + ProjectionElem::Index(l) => { + f(this, local, head); + w!(this, "[{}]", this.local_name(*l)); + } + x => { + f(this, local, head); + w!(this, ".{:?}", x); + } + } + } + f(self, p.local, &p.projection); + } + + fn operand(&mut self, r: &Operand) { + match r { + Operand::Copy(p) | Operand::Move(p) => { + // MIR at the time of writing doesn't have difference between move and copy, so we show them + // equally. Feel free to change it. + self.place(p); + } + Operand::Constant(c) => w!(self, "Const({})", c.display(self.db)), + } + } + + fn rvalue(&mut self, r: &Rvalue) { + match r { + Rvalue::Use(op) => self.operand(op), + Rvalue::Ref(r, p) => { + match r { + BorrowKind::Shared => w!(self, "&"), + BorrowKind::Shallow => w!(self, "&shallow "), + BorrowKind::Unique => w!(self, "&uniq "), + BorrowKind::Mut { .. } => w!(self, "&mut "), + } + self.place(p); + } + Rvalue::Aggregate(AggregateKind::Tuple(_), x) => { + w!(self, "("); + self.operand_list(x); + w!(self, ")"); + } + Rvalue::Aggregate(AggregateKind::Array(_), x) => { + w!(self, "["); + self.operand_list(x); + w!(self, "]"); + } + Rvalue::Aggregate(AggregateKind::Adt(_, _), x) => { + w!(self, "Adt("); + self.operand_list(x); + w!(self, ")"); + } + Rvalue::Aggregate(AggregateKind::Union(_, _), x) => { + w!(self, "Union("); + self.operand_list(x); + w!(self, ")"); + } + Rvalue::Len(p) => { + w!(self, "Len("); + self.place(p); + w!(self, ")"); + } + Rvalue::Cast(ck, op, ty) => { + w!(self, "Discriminant({ck:?}"); + self.operand(op); + w!(self, "{})", ty.display(self.db)); + } + Rvalue::CheckedBinaryOp(b, o1, o2) => { + self.operand(o1); + w!(self, " {b} "); + self.operand(o2); + } + Rvalue::UnaryOp(u, o) => { + let u = match u { + UnOp::Not => "!", + UnOp::Neg => "-", + }; + w!(self, "{u} "); + self.operand(o); + } + Rvalue::Discriminant(p) => { + w!(self, "Discriminant("); + self.place(p); + w!(self, ")"); + } + Rvalue::ShallowInitBox(op, _) => { + w!(self, "ShallowInitBox("); + self.operand(op); + w!(self, ")"); + } + Rvalue::CopyForDeref(p) => { + w!(self, "CopyForDeref("); + self.place(p); + w!(self, ")"); + } + } + } + + fn operand_list(&mut self, x: &[Operand]) { + let mut it = x.iter(); + if let Some(first) = it.next() { + self.operand(first); + for op in it { + w!(self, ", "); + self.operand(op); + } + } + } +} diff --git a/crates/hir/src/has_source.rs b/crates/hir/src/has_source.rs index a28b2d577a..e84f5ebb36 100644 --- a/crates/hir/src/has_source.rs +++ b/crates/hir/src/has_source.rs @@ -10,8 +10,8 @@ use hir_expand::InFile; use syntax::ast; use crate::{ - db::HirDatabase, Adt, Const, Enum, Field, FieldSource, Function, Impl, LifetimeParam, Macro, - Module, Static, Struct, Trait, TraitAlias, TypeAlias, TypeOrConstParam, Union, Variant, + db::HirDatabase, Adt, Const, Enum, Field, FieldSource, Function, Impl, LifetimeParam, + LocalSource, Macro, Module, Static, Struct, Trait, TypeAlias, TraitAlias, TypeOrConstParam, Union, Variant, }; pub trait HasSource { @@ -178,3 +178,11 @@ impl HasSource for LifetimeParam { Some(child_source.map(|it| it[self.id.local_id].clone())) } } + +impl HasSource for LocalSource { + type Ast = Either; + + fn source(self, _: &dyn HirDatabase) -> Option> { + Some(self.source) + } +} diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 4b65a93cac..248f4ff858 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1327,6 +1327,15 @@ impl DefWithBody { body.pretty_print(db.upcast(), self.id()) } + /// A textual representation of the MIR of this def's body for debugging purposes. + pub fn debug_mir(self, db: &dyn HirDatabase) -> String { + let body = db.mir_body(self.id()); + match body { + Ok(body) => body.pretty_print(db), + Err(e) => format!("error:\n{e:?}"), + } + } + pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec) { let krate = self.module(db).id.krate(); @@ -1502,32 +1511,35 @@ impl DefWithBody { let hir_body = db.body(self.into()); - if let Ok(mir_body) = db.mir_body(self.into()) { - let mol = mir::borrowck::mutability_of_locals(&mir_body); + if let Ok(borrowck_result) = db.borrowck(self.into()) { + let mir_body = &borrowck_result.mir_body; + let mol = &borrowck_result.mutability_of_locals; for (binding_id, _) in hir_body.bindings.iter() { let need_mut = &mol[mir_body.binding_locals[binding_id]]; let local = Local { parent: self.into(), binding_id }; match (need_mut, local.is_mut(db)) { - (mir::borrowck::Mutability::Mut { .. }, true) - | (mir::borrowck::Mutability::Not, false) => (), - (mir::borrowck::Mutability::Mut { span }, false) => { - let span: InFile = match span { - mir::MirSpan::ExprId(e) => match source_map.expr_syntax(*e) { - Ok(s) => s.map(|x| x.into()), - Err(_) => continue, - }, - mir::MirSpan::PatId(p) => match source_map.pat_syntax(*p) { - Ok(s) => s.map(|x| match x { - Either::Left(e) => e.into(), - Either::Right(e) => e.into(), - }), - Err(_) => continue, - }, - mir::MirSpan::Unknown => continue, - }; - acc.push(NeedMut { local, span }.into()); + (mir::MutabilityReason::Mut { .. }, true) + | (mir::MutabilityReason::Not, false) => (), + (mir::MutabilityReason::Mut { spans }, false) => { + for span in spans { + let span: InFile = match span { + mir::MirSpan::ExprId(e) => match source_map.expr_syntax(*e) { + Ok(s) => s.map(|x| x.into()), + Err(_) => continue, + }, + mir::MirSpan::PatId(p) => match source_map.pat_syntax(*p) { + Ok(s) => s.map(|x| match x { + Either::Left(e) => e.into(), + Either::Right(e) => e.into(), + }), + Err(_) => continue, + }, + mir::MirSpan::Unknown => continue, + }; + acc.push(NeedMut { local, span }.into()); + } } - (mir::borrowck::Mutability::Not, true) => acc.push(UnusedMut { local }.into()), + (mir::MutabilityReason::Not, true) => acc.push(UnusedMut { local }.into()), } } } @@ -2519,6 +2531,10 @@ impl LocalSource { self.source.file_id.original_file(db.upcast()) } + pub fn name(&self) -> Option { + self.source.value.name() + } + pub fn syntax(&self) -> &SyntaxNode { self.source.value.syntax() } diff --git a/crates/ide-db/src/rename.rs b/crates/ide-db/src/rename.rs index 298c714139..f710211c8c 100644 --- a/crates/ide-db/src/rename.rs +++ b/crates/ide-db/src/rename.rs @@ -121,8 +121,7 @@ impl Definition { Definition::Trait(it) => name_range(it, sema), Definition::TraitAlias(it) => name_range(it, sema), Definition::TypeAlias(it) => name_range(it, sema), - // A local might be `self` or have multiple definitons like `let (a | a) = 2`, so it should be handled as a special case - Definition::Local(_) => return None, + Definition::Local(it) => name_range(it.primary_source(sema.db), sema), Definition::GenericParam(generic_param) => match generic_param { hir::GenericParam::LifetimeParam(lifetime_param) => { let src = lifetime_param.source(sema.db)?; diff --git a/crates/ide-diagnostics/src/handlers/mutability_errors.rs b/crates/ide-diagnostics/src/handlers/mutability_errors.rs index a78b58fdc8..4f5d958354 100644 --- a/crates/ide-diagnostics/src/handlers/mutability_errors.rs +++ b/crates/ide-diagnostics/src/handlers/mutability_errors.rs @@ -1,31 +1,85 @@ -use crate::{Diagnostic, DiagnosticsContext, Severity}; +use ide_db::source_change::SourceChange; +use syntax::{AstNode, SyntaxKind, SyntaxNode, SyntaxToken, T}; +use text_edit::TextEdit; + +use crate::{fix, Diagnostic, DiagnosticsContext, Severity}; // Diagnostic: need-mut // // This diagnostic is triggered on mutating an immutable variable. pub(crate) fn need_mut(ctx: &DiagnosticsContext<'_>, d: &hir::NeedMut) -> Diagnostic { + let fixes = (|| { + if d.local.is_ref(ctx.sema.db) { + // There is no simple way to add `mut` to `ref x` and `ref mut x` + return None; + } + let file_id = d.span.file_id.file_id()?; + let mut edit_builder = TextEdit::builder(); + let use_range = d.span.value.text_range(); + for source in d.local.sources(ctx.sema.db) { + let Some(ast) = source.name() else { continue }; + edit_builder.insert(ast.syntax().text_range().start(), "mut ".to_string()); + } + let edit = edit_builder.finish(); + Some(vec![fix( + "remove_mut", + "Remove unnecessary `mut`", + SourceChange::from_text_edit(file_id, edit), + use_range, + )]) + })(); Diagnostic::new( "need-mut", format!("cannot mutate immutable variable `{}`", d.local.name(ctx.sema.db)), ctx.sema.diagnostics_display_range(d.span.clone()).range, ) + .with_fixes(fixes) } // Diagnostic: unused-mut // // This diagnostic is triggered when a mutable variable isn't actually mutated. pub(crate) fn unused_mut(ctx: &DiagnosticsContext<'_>, d: &hir::UnusedMut) -> Diagnostic { + let ast = d.local.primary_source(ctx.sema.db).syntax_ptr(); + let fixes = (|| { + let file_id = ast.file_id.file_id()?; + let mut edit_builder = TextEdit::builder(); + let use_range = ast.value.text_range(); + for source in d.local.sources(ctx.sema.db) { + let ast = source.syntax(); + let Some(mut_token) = token(ast, T![mut]) else { continue }; + edit_builder.delete(mut_token.text_range()); + if let Some(token) = mut_token.next_token() { + if token.kind() == SyntaxKind::WHITESPACE { + edit_builder.delete(token.text_range()); + } + } + } + let edit = edit_builder.finish(); + Some(vec![fix( + "remove_mut", + "Remove unnecessary `mut`", + SourceChange::from_text_edit(file_id, edit), + use_range, + )]) + })(); + let ast = d.local.primary_source(ctx.sema.db).syntax_ptr(); Diagnostic::new( "unused-mut", "remove this `mut`", - ctx.sema.diagnostics_display_range(d.local.primary_source(ctx.sema.db).syntax_ptr()).range, + ctx.sema.diagnostics_display_range(ast).range, ) .severity(Severity::WeakWarning) + .with_fixes(fixes) +} + +pub(super) fn token(parent: &SyntaxNode, kind: SyntaxKind) -> Option { + parent.children_with_tokens().filter_map(|it| it.into_token()).find(|it| it.kind() == kind) } #[cfg(test)] mod tests { - use crate::tests::check_diagnostics; + use crate::tests::{check_diagnostics, check_fix}; #[test] fn unused_mut_simple() { @@ -34,7 +88,7 @@ mod tests { fn f(_: i32) {} fn main() { let mut x = 2; - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` f(x); } "#, @@ -64,6 +118,144 @@ fn main() { ); } + #[test] + fn multiple_errors_for_single_variable() { + check_diagnostics( + r#" +fn f(_: i32) {} +fn main() { + let x = 2; + x = 10; + //^^^^^^ 💡 error: cannot mutate immutable variable `x` + x = 5; + //^^^^^ 💡 error: cannot mutate immutable variable `x` + &mut x; + //^^^^^^ 💡 error: cannot mutate immutable variable `x` + f(x); +} +"#, + ); + } + + #[test] + fn unused_mut_fix() { + check_fix( + r#" +fn f(_: i32) {} +fn main() { + let mu$0t x = 2; + f(x); +} +"#, + r#" +fn f(_: i32) {} +fn main() { + let x = 2; + f(x); +} +"#, + ); + check_fix( + r#" +fn f(_: i32) {} +fn main() { + let ((mu$0t x, _) | (_, mut x)) = (2, 3); + f(x); +} +"#, + r#" +fn f(_: i32) {} +fn main() { + let ((x, _) | (_, x)) = (2, 3); + f(x); +} +"#, + ); + } + + #[test] + fn need_mut_fix() { + check_fix( + r#" +fn f(_: i32) {} +fn main() { + let x = 2; + x$0 = 5; + f(x); +} +"#, + r#" +fn f(_: i32) {} +fn main() { + let mut x = 2; + x = 5; + f(x); +} +"#, + ); + check_fix( + r#" +fn f(_: i32) {} +fn main() { + let ((x, _) | (_, x)) = (2, 3); + x =$0 4; + f(x); +} +"#, + r#" +fn f(_: i32) {} +fn main() { + let ((mut x, _) | (_, mut x)) = (2, 3); + x = 4; + f(x); +} +"#, + ); + + check_fix( + r#" +struct Foo(i32); + +impl Foo { + fn foo(self) { + self = Fo$0o(5); + } +} +"#, + r#" +struct Foo(i32); + +impl Foo { + fn foo(mut self) { + self = Foo(5); + } +} +"#, + ); + } + + #[test] + fn need_mut_fix_not_applicable_on_ref() { + check_diagnostics( + r#" +fn main() { + let ref x = 2; + x = &5; + //^^^^^^ error: cannot mutate immutable variable `x` +} +"#, + ); + check_diagnostics( + r#" +fn main() { + let ref mut x = 2; + x = &mut 5; + //^^^^^^^^^^ error: cannot mutate immutable variable `x` +} +"#, + ); + } + #[test] fn field_mutate() { check_diagnostics( @@ -71,7 +263,7 @@ fn main() { fn f(_: i32) {} fn main() { let mut x = (2, 7); - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` f(x.1); } "#, @@ -92,7 +284,7 @@ fn f(_: i32) {} fn main() { let x = (2, 7); x.0 = 5; - //^^^^^^^ error: cannot mutate immutable variable `x` + //^^^^^^^ 💡 error: cannot mutate immutable variable `x` f(x.1); } "#, @@ -105,7 +297,7 @@ fn main() { r#" fn main() { let mut x = &mut 2; - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` *x = 5; } "#, @@ -115,7 +307,7 @@ fn main() { fn main() { let x = 2; &mut x; - //^^^^^^ error: cannot mutate immutable variable `x` + //^^^^^^ 💡 error: cannot mutate immutable variable `x` } "#, ); @@ -124,7 +316,7 @@ fn main() { fn main() { let x_own = 2; let ref mut x_ref = x_own; - //^^^^^^^^^^^^^ error: cannot mutate immutable variable `x_own` + //^^^^^^^^^^^^^ 💡 error: cannot mutate immutable variable `x_own` } "#, ); @@ -137,7 +329,7 @@ impl Foo { fn main() { let x = Foo; x.method(2); - //^ error: cannot mutate immutable variable `x` + //^ 💡 error: cannot mutate immutable variable `x` } "#, ); @@ -150,9 +342,9 @@ fn main() { fn main() { match (2, 3) { (x, mut y) => { - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` x = 7; - //^^^^^ error: cannot mutate immutable variable `x` + //^^^^^ 💡 error: cannot mutate immutable variable `x` } } } @@ -171,7 +363,7 @@ fn main() { fn main() { return; let mut x = 2; - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` &mut x; } "#, @@ -181,7 +373,7 @@ fn main() { fn main() { loop {} let mut x = 2; - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` &mut x; } "#, @@ -202,7 +394,7 @@ fn main(b: bool) { g(); } let mut x = 2; - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` &mut x; } "#, @@ -216,7 +408,7 @@ fn main(b: bool) { return; } let mut x = 2; - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` &mut x; } "#, @@ -230,7 +422,7 @@ fn main(b: bool) { fn f(_: i32) {} fn main() { let mut x; - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` x = 5; f(x); } @@ -241,7 +433,7 @@ fn main() { fn f(_: i32) {} fn main(b: bool) { let mut x; - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` if b { x = 1; } else { @@ -260,7 +452,7 @@ fn main(b: bool) { x = 1; } x = 3; - //^^^^^ error: cannot mutate immutable variable `x` + //^^^^^ 💡 error: cannot mutate immutable variable `x` f(x); } "#, @@ -272,7 +464,7 @@ fn main() { let x; loop { x = 1; - //^^^^^ error: cannot mutate immutable variable `x` + //^^^^^ 💡 error: cannot mutate immutable variable `x` f(x); } } @@ -284,18 +476,37 @@ fn f(_: i32) {} fn main() { loop { let mut x = 1; - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` f(x); if let mut y = 2 { - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` f(y); } match 3 { mut z => f(z), - //^^^^^ weak: remove this `mut` + //^^^^^ 💡 weak: remove this `mut` } } } +"#, + ); + } + + #[test] + fn function_arguments_are_initialized() { + check_diagnostics( + r#" +fn f(mut x: i32) { + //^^^^^ 💡 weak: remove this `mut` +} +"#, + ); + check_diagnostics( + r#" +fn f(x: i32) { + x = 5; + //^^^^^ 💡 error: cannot mutate immutable variable `x` +} "#, ); } diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index f2b535bdc7..078b66dd39 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -55,6 +55,7 @@ mod syntax_tree; mod typing; mod view_crate_graph; mod view_hir; +mod view_mir; mod view_item_tree; mod shuffle_crate_graph; @@ -308,6 +309,10 @@ impl Analysis { self.with_db(|db| view_hir::view_hir(db, position)) } + pub fn view_mir(&self, position: FilePosition) -> Cancellable { + self.with_db(|db| view_mir::view_mir(db, position)) + } + pub fn view_item_tree(&self, file_id: FileId) -> Cancellable { self.with_db(|db| view_item_tree::view_item_tree(db, file_id)) } diff --git a/crates/ide/src/rename.rs b/crates/ide/src/rename.rs index c0237e1edd..e10c463810 100644 --- a/crates/ide/src/rename.rs +++ b/crates/ide/src/rename.rs @@ -353,6 +353,11 @@ mod tests { fn check(new_name: &str, ra_fixture_before: &str, ra_fixture_after: &str) { let ra_fixture_after = &trim_indent(ra_fixture_after); let (analysis, position) = fixture::position(ra_fixture_before); + if !ra_fixture_after.starts_with("error: ") { + if let Err(err) = analysis.prepare_rename(position).unwrap() { + panic!("Prepare rename to '{new_name}' was failed: {err}") + } + } let rename_result = analysis .rename(position, new_name) .unwrap_or_else(|err| panic!("Rename to '{new_name}' was cancelled: {err}")); @@ -1709,6 +1714,23 @@ fn foo(bar: i32) -> Foo { ); } + #[test] + fn test_rename_local_simple() { + check( + "i", + r#" +fn foo(bar$0: i32) -> i32 { + bar +} +"#, + r#" +fn foo(i: i32) -> i32 { + i +} +"#, + ); + } + #[test] fn test_rename_local_put_init_shorthand() { cov_mark::check!(test_rename_local_put_init_shorthand); diff --git a/crates/ide/src/view_mir.rs b/crates/ide/src/view_mir.rs new file mode 100644 index 0000000000..a36aba58bc --- /dev/null +++ b/crates/ide/src/view_mir.rs @@ -0,0 +1,29 @@ +use hir::{DefWithBody, Semantics}; +use ide_db::base_db::FilePosition; +use ide_db::RootDatabase; +use syntax::{algo::find_node_at_offset, ast, AstNode}; + +// Feature: View Mir +// +// |=== +// | Editor | Action Name +// +// | VS Code | **rust-analyzer: View Mir** +// |=== +pub(crate) fn view_mir(db: &RootDatabase, position: FilePosition) -> String { + body_mir(db, position).unwrap_or_else(|| "Not inside a function body".to_string()) +} + +fn body_mir(db: &RootDatabase, position: FilePosition) -> Option { + let sema = Semantics::new(db); + let source_file = sema.parse(position.file_id); + + let item = find_node_at_offset::(source_file.syntax(), position.offset)?; + let def: DefWithBody = match item { + ast::Item::Fn(it) => sema.to_def(&it)?.into(), + ast::Item::Const(it) => sema.to_def(&it)?.into(), + ast::Item::Static(it) => sema.to_def(&it)?.into(), + _ => return None, + }; + Some(def.debug_mir(db)) +} diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 4e08bd0a72..32ac9a42de 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -134,6 +134,16 @@ pub(crate) fn handle_view_hir( Ok(res) } +pub(crate) fn handle_view_mir( + snap: GlobalStateSnapshot, + params: lsp_types::TextDocumentPositionParams, +) -> Result { + let _p = profile::span("handle_view_mir"); + let position = from_proto::file_position(&snap, params)?; + let res = snap.analysis.view_mir(position)?; + Ok(res) +} + pub(crate) fn handle_view_file_text( snap: GlobalStateSnapshot, params: lsp_types::TextDocumentIdentifier, diff --git a/crates/rust-analyzer/src/lsp_ext.rs b/crates/rust-analyzer/src/lsp_ext.rs index e33589cc53..c7b513db98 100644 --- a/crates/rust-analyzer/src/lsp_ext.rs +++ b/crates/rust-analyzer/src/lsp_ext.rs @@ -74,6 +74,14 @@ impl Request for ViewHir { const METHOD: &'static str = "rust-analyzer/viewHir"; } +pub enum ViewMir {} + +impl Request for ViewMir { + type Params = lsp_types::TextDocumentPositionParams; + type Result = String; + const METHOD: &'static str = "rust-analyzer/viewMir"; +} + pub enum ViewFileText {} impl Request for ViewFileText { diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index d1e38b33c7..d279769066 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -634,6 +634,7 @@ impl GlobalState { .on::(handlers::handle_analyzer_status) .on::(handlers::handle_syntax_tree) .on::(handlers::handle_view_hir) + .on::(handlers::handle_view_mir) .on::(handlers::handle_view_file_text) .on::(handlers::handle_view_crate_graph) .on::(handlers::handle_view_item_tree) diff --git a/crates/syntax/src/ast/traits.rs b/crates/syntax/src/ast/traits.rs index bc7b558231..3e43df2d0d 100644 --- a/crates/syntax/src/ast/traits.rs +++ b/crates/syntax/src/ast/traits.rs @@ -134,3 +134,5 @@ impl Iterator for AttrDocCommentIter { }) } } + +impl HasName for Either {} diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index c3623a5cc4..de14220320 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -1,5 +1,5 @@