From 71f3e4b08ca0f4dc63fa6a58f921e8e1addf404d Mon Sep 17 00:00:00 2001 From: hkalbasi Date: Sun, 4 Jun 2023 12:39:36 +0330 Subject: [PATCH] Detect "bound more than once" error and suppress `need-mut` for it. --- crates/hir-def/src/body/lower.rs | 62 ++++++++++++++++--- crates/hir-def/src/hir.rs | 11 ++++ crates/hir/src/lib.rs | 6 +- .../src/handlers/mutability_errors.rs | 13 ++++ 4 files changed, 84 insertions(+), 8 deletions(-) diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index d8ecde71025..7b88e525bf1 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -30,9 +30,9 @@ use crate::{ db::DefDatabase, expander::Expander, hir::{ - dummy_expr_id, Array, Binding, BindingAnnotation, BindingId, CaptureBy, ClosureKind, Expr, - ExprId, Label, LabelId, Literal, LiteralOrConst, MatchArm, Movability, Pat, PatId, - RecordFieldPat, RecordLitField, Statement, + dummy_expr_id, Array, Binding, BindingAnnotation, BindingId, BindingProblems, CaptureBy, + ClosureKind, Expr, ExprId, Label, LabelId, Literal, LiteralOrConst, MatchArm, Movability, + Pat, PatId, RecordFieldPat, RecordLitField, Statement, }, item_scope::BuiltinShadowMode, lang_item::LangItem, @@ -141,6 +141,8 @@ impl RibKind { #[derive(Debug, Default)] struct BindingList { map: FxHashMap, + is_used: FxHashMap, + reject_new: bool, } impl BindingList { @@ -150,7 +152,27 @@ impl BindingList { name: Name, mode: BindingAnnotation, ) -> BindingId { - *self.map.entry(name).or_insert_with_key(|n| ec.alloc_binding(n.clone(), mode)) + let id = *self.map.entry(name).or_insert_with_key(|n| ec.alloc_binding(n.clone(), mode)); + if ec.body.bindings[id].mode != mode { + ec.body.bindings[id].problems = Some(BindingProblems::BoundInconsistently); + } + self.check_is_used(ec, id); + id + } + + fn check_is_used(&mut self, ec: &mut ExprCollector<'_>, id: BindingId) { + match self.is_used.get(&id) { + None => { + if self.reject_new { + ec.body.bindings[id].problems = Some(BindingProblems::NotBoundAcrossAll); + } + } + Some(true) => { + ec.body.bindings[id].problems = Some(BindingProblems::BoundMoreThanOnce); + } + Some(false) => {} + } + self.is_used.insert(id, true); } } @@ -1208,9 +1230,34 @@ impl ExprCollector<'_> { p.path().and_then(|path| self.expander.parse_path(self.db, path)).map(Box::new); path.map(Pat::Path).unwrap_or(Pat::Missing) } - ast::Pat::OrPat(p) => { - let pats = p.pats().map(|p| self.collect_pat(p, binding_list)).collect(); - Pat::Or(pats) + ast::Pat::OrPat(p) => 'b: { + let prev_is_used = mem::take(&mut binding_list.is_used); + let prev_reject_new = mem::take(&mut binding_list.reject_new); + let mut pats = Vec::with_capacity(p.pats().count()); + let mut it = p.pats(); + let Some(first) = it.next() else { + break 'b Pat::Or(Box::new([])); + }; + pats.push(self.collect_pat(first, binding_list)); + binding_list.reject_new = true; + for rest in it { + for (_, x) in binding_list.is_used.iter_mut() { + *x = false; + } + pats.push(self.collect_pat(rest, binding_list)); + for (&id, &x) in binding_list.is_used.iter() { + if !x { + self.body.bindings[id].problems = + Some(BindingProblems::NotBoundAcrossAll); + } + } + } + binding_list.reject_new = prev_reject_new; + let current_is_used = mem::replace(&mut binding_list.is_used, prev_is_used); + for (id, _) in current_is_used.into_iter() { + binding_list.check_is_used(self, id); + } + Pat::Or(pats.into()) } ast::Pat::ParenPat(p) => return self.collect_pat_opt(p.pat(), binding_list), ast::Pat::TuplePat(p) => { @@ -1499,6 +1546,7 @@ impl ExprCollector<'_> { mode, definitions: SmallVec::new(), owner: self.current_binding_owner, + problems: None, }) } diff --git a/crates/hir-def/src/hir.rs b/crates/hir-def/src/hir.rs index 8102efdba30..4ad8a7aa8eb 100644 --- a/crates/hir-def/src/hir.rs +++ b/crates/hir-def/src/hir.rs @@ -486,6 +486,16 @@ impl BindingAnnotation { } } +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum BindingProblems { + /// https://doc.rust-lang.org/stable/error_codes/E0416.html + BoundMoreThanOnce, + /// https://doc.rust-lang.org/stable/error_codes/E0409.html + BoundInconsistently, + /// https://doc.rust-lang.org/stable/error_codes/E0408.html + NotBoundAcrossAll, +} + #[derive(Debug, Clone, Eq, PartialEq)] pub struct Binding { pub name: Name, @@ -494,6 +504,7 @@ pub struct Binding { /// Id of the closure/generator that owns this binding. If it is owned by the /// top level expression, this field would be `None`. pub owner: Option, + pub problems: Option, } impl Binding { diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 2db4b483b69..5926d865421 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1643,7 +1643,11 @@ impl DefWithBody { ) } let mol = &borrowck_result.mutability_of_locals; - for (binding_id, _) in hir_body.bindings.iter() { + for (binding_id, binding_data) in hir_body.bindings.iter() { + if binding_data.problems.is_some() { + // We should report specific diagnostics for these problems, not `need-mut` and `unused-mut`. + continue; + } let Some(&local) = mir_body.binding_locals.get(binding_id) else { continue; }; diff --git a/crates/ide-diagnostics/src/handlers/mutability_errors.rs b/crates/ide-diagnostics/src/handlers/mutability_errors.rs index 743ef0c6f52..b95c8c573b5 100644 --- a/crates/ide-diagnostics/src/handlers/mutability_errors.rs +++ b/crates/ide-diagnostics/src/handlers/mutability_errors.rs @@ -620,6 +620,19 @@ fn f((x, y): (i32, i32)) { ); } + #[test] + fn no_diagnostics_in_case_of_multiple_bounds() { + check_diagnostics( + r#" +fn f() { + let (b, a, b) = (2, 3, 5); + a = 8; + //^^^^^ 💡 error: cannot mutate immutable variable `a` +} +"#, + ); + } + #[test] fn for_loop() { check_diagnostics(