From 0e7117900c06ef2ae7f113f61e08a3234c3aee79 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 6 Apr 2023 12:50:16 +0200 Subject: [PATCH] internal: Resolve labels in body lowering --- crates/hir-def/src/body.rs | 5 +- crates/hir-def/src/body/lower.rs | 275 +++++++++++++----- crates/hir-def/src/body/pretty.rs | 8 +- crates/hir-def/src/expr.rs | 4 +- crates/hir-expand/src/name.rs | 3 +- crates/hir-ty/src/infer.rs | 23 +- crates/hir-ty/src/infer/expr.rs | 45 +-- crates/hir-ty/src/mir/lower.rs | 18 +- crates/hir/src/diagnostics.rs | 21 +- crates/hir/src/lib.rs | 27 +- .../src/handlers/undeclared_label.rs | 61 ++++ .../src/handlers/unreachable_label.rs | 87 ++++++ crates/ide-diagnostics/src/lib.rs | 25 +- 13 files changed, 420 insertions(+), 182 deletions(-) create mode 100644 crates/ide-diagnostics/src/handlers/undeclared_label.rs create mode 100644 crates/ide-diagnostics/src/handlers/unreachable_label.rs diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index bd87eaa221..0bea905e11 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -13,7 +13,8 @@ use cfg::{CfgExpr, CfgOptions}; use drop_bomb::DropBomb; use either::Either; use hir_expand::{ - attrs::RawAttrs, hygiene::Hygiene, ExpandError, ExpandResult, HirFileId, InFile, MacroCallId, + attrs::RawAttrs, hygiene::Hygiene, name::Name, ExpandError, ExpandResult, HirFileId, InFile, + MacroCallId, }; use la_arena::{Arena, ArenaMap}; use limit::Limit; @@ -343,6 +344,8 @@ pub enum BodyDiagnostic { MacroError { node: InFile>, message: String }, UnresolvedProcMacro { node: InFile>, krate: CrateId }, UnresolvedMacroCall { node: InFile>, path: ModPath }, + UnreachableLabel { node: InFile>, name: Name }, + UndeclaredLabel { node: InFile>, name: Name }, } impl Body { diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 5362737583..91eeff5b0e 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -102,9 +102,10 @@ pub(super) fn lower( _c: Count::new(), }, expander, - current_try_block: None, + current_try_block_label: None, is_lowering_assignee_expr: false, is_lowering_generator: false, + label_ribs: Vec::new(), } .collect(params, body, is_async_fn) } @@ -116,9 +117,43 @@ struct ExprCollector<'a> { body: Body, krate: CrateId, source_map: BodySourceMap, - current_try_block: Option, + current_try_block_label: Option, is_lowering_assignee_expr: bool, is_lowering_generator: bool, + label_ribs: Vec, +} + +#[derive(Clone, Debug)] +struct LabelRib { + kind: RibKind, + // Once we handle macro hygiene this will need to be a map + label: Option<(Name, LabelId)>, +} + +impl LabelRib { + fn new(kind: RibKind) -> Self { + LabelRib { kind, label: None } + } + fn new_normal(label: (Name, LabelId)) -> Self { + LabelRib { kind: RibKind::Normal, label: Some(label) } + } +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum RibKind { + Normal, + Closure, + Constant, +} + +impl RibKind { + /// This rib forbids referring to labels defined in upwards ribs. + fn is_label_barrier(self) -> bool { + match self { + RibKind::Normal => false, + RibKind::Closure | RibKind::Constant => true, + } + } } #[derive(Debug, Default)] @@ -171,26 +206,24 @@ impl ExprCollector<'_> { self.body.params.push(param_pat); } }; + self.body.body_expr = self.with_label_rib(RibKind::Closure, |this| { + if is_async_fn { + match body { + Some(e) => { + let expr = this.collect_expr(e); + this.alloc_expr_desugared(Expr::Async { + id: None, + statements: Box::new([]), + tail: Some(expr), + }) + } + None => this.missing_expr(), + } + } else { + this.collect_expr_opt(body) + } + }); - self.body.body_expr = if is_async_fn { - self.current_try_block = - Some(self.alloc_label_desugared(Label { name: Name::generate_new_name() })); - let expr = self.collect_expr_opt(body); - let expr = self.alloc_expr_desugared(Expr::Block { - id: None, - statements: Box::new([]), - tail: Some(expr), - label: self.current_try_block, - }); - let expr = self.alloc_expr_desugared(Expr::Async { - id: None, - statements: Box::new([]), - tail: Some(expr), - }); - expr - } else { - self.collect_expr_opt(body) - }; (self.body, self.source_map) } @@ -264,6 +297,7 @@ impl ExprCollector<'_> { let syntax_ptr = AstPtr::new(&expr); self.check_cfg(&expr)?; + // FIXME: Move some of these arms out into separate methods for clarity Some(match expr { ast::Expr::IfExpr(e) => { let then_branch = self.collect_block_opt(e.then_branch()); @@ -286,7 +320,7 @@ impl ExprCollector<'_> { self.alloc_expr(Expr::Let { pat, expr }, syntax_ptr) } ast::Expr::BlockExpr(e) => match e.modifier() { - Some(ast::BlockModifier::Try(_)) => self.collect_try_block(e), + Some(ast::BlockModifier::Try(_)) => self.desugar_try_block(e), Some(ast::BlockModifier::Unsafe(_)) => { self.collect_block_(e, |id, statements, tail| Expr::Unsafe { id, @@ -296,28 +330,43 @@ impl ExprCollector<'_> { } Some(ast::BlockModifier::Label(label)) => { let label = self.collect_label(label); - self.collect_block_(e, |id, statements, tail| Expr::Block { - id, - statements, - tail, - label: Some(label), + self.with_labeled_rib(label, |this| { + this.collect_block_(e, |id, statements, tail| Expr::Block { + id, + statements, + tail, + label: Some(label), + }) + }) + } + Some(ast::BlockModifier::Async(_)) => { + self.with_label_rib(RibKind::Closure, |this| { + this.collect_block_(e, |id, statements, tail| Expr::Async { + id, + statements, + tail, + }) + }) + } + Some(ast::BlockModifier::Const(_)) => { + self.with_label_rib(RibKind::Constant, |this| { + this.collect_block_(e, |id, statements, tail| Expr::Const { + id, + statements, + tail, + }) }) } - Some(ast::BlockModifier::Async(_)) => self - .collect_block_(e, |id, statements, tail| Expr::Async { id, statements, tail }), - Some(ast::BlockModifier::Const(_)) => self - .collect_block_(e, |id, statements, tail| Expr::Const { id, statements, tail }), None => self.collect_block(e), }, ast::Expr::LoopExpr(e) => { let label = e.label().map(|label| self.collect_label(label)); - let body = self.collect_block_opt(e.loop_body()); + let body = self.collect_labelled_block_opt(label, e.loop_body()); self.alloc_expr(Expr::Loop { body, label }, syntax_ptr) } ast::Expr::WhileExpr(e) => { let label = e.label().map(|label| self.collect_label(label)); - let body = self.collect_block_opt(e.loop_body()); - + let body = self.collect_labelled_block_opt(label, e.loop_body()); let condition = self.collect_expr_opt(e.condition()); self.alloc_expr(Expr::While { condition, body, label }, syntax_ptr) @@ -326,7 +375,7 @@ impl ExprCollector<'_> { let label = e.label().map(|label| self.collect_label(label)); let iterable = self.collect_expr_opt(e.iterable()); let pat = self.collect_pat_opt(e.pat()); - let body = self.collect_block_opt(e.loop_body()); + let body = self.collect_labelled_block_opt(label, e.loop_body()); self.alloc_expr(Expr::For { iterable, pat, body, label }, syntax_ptr) } ast::Expr::CallExpr(e) => { @@ -386,16 +435,20 @@ impl ExprCollector<'_> { .unwrap_or(Expr::Missing); self.alloc_expr(path, syntax_ptr) } - ast::Expr::ContinueExpr(e) => self.alloc_expr( - Expr::Continue { label: e.lifetime().map(|l| Name::new_lifetime(&l)) }, - syntax_ptr, - ), + ast::Expr::ContinueExpr(e) => { + let label = self.resolve_label(e.lifetime()).unwrap_or_else(|e| { + self.source_map.diagnostics.push(e); + None + }); + self.alloc_expr(Expr::Continue { label }, syntax_ptr) + } ast::Expr::BreakExpr(e) => { + let label = self.resolve_label(e.lifetime()).unwrap_or_else(|e| { + self.source_map.diagnostics.push(e); + None + }); let expr = e.expr().map(|e| self.collect_expr(e)); - self.alloc_expr( - Expr::Break { expr, label: e.lifetime().map(|l| Name::new_lifetime(&l)) }, - syntax_ptr, - ) + self.alloc_expr(Expr::Break { expr, label }, syntax_ptr) } ast::Expr::ParenExpr(e) => { let inner = self.collect_expr_opt(e.expr()); @@ -496,14 +549,14 @@ impl ExprCollector<'_> { None => self.alloc_expr(Expr::Missing, syntax_ptr), } } - ast::Expr::ClosureExpr(e) => { + ast::Expr::ClosureExpr(e) => self.with_label_rib(RibKind::Closure, |this| { let mut args = Vec::new(); let mut arg_types = Vec::new(); if let Some(pl) = e.param_list() { for param in pl.params() { - let pat = self.collect_pat_opt(param.pat()); + let pat = this.collect_pat_opt(param.pat()); let type_ref = - param.ty().map(|it| Interned::new(TypeRef::from_ast(&self.ctx(), it))); + param.ty().map(|it| Interned::new(TypeRef::from_ast(&this.ctx(), it))); args.push(pat); arg_types.push(type_ref); } @@ -511,14 +564,14 @@ impl ExprCollector<'_> { let ret_type = e .ret_type() .and_then(|r| r.ty()) - .map(|it| Interned::new(TypeRef::from_ast(&self.ctx(), it))); + .map(|it| Interned::new(TypeRef::from_ast(&this.ctx(), it))); - let prev_is_lowering_generator = self.is_lowering_generator; - self.is_lowering_generator = false; + let prev_is_lowering_generator = this.is_lowering_generator; + this.is_lowering_generator = false; - let body = self.collect_expr_opt(e.body()); + let body = this.collect_expr_opt(e.body()); - let closure_kind = if self.is_lowering_generator { + let closure_kind = if this.is_lowering_generator { let movability = if e.static_token().is_some() { Movability::Static } else { @@ -530,9 +583,9 @@ impl ExprCollector<'_> { } else { ClosureKind::Closure }; - self.is_lowering_generator = prev_is_lowering_generator; + this.is_lowering_generator = prev_is_lowering_generator; - self.alloc_expr( + this.alloc_expr( Expr::Closure { args: args.into(), arg_types: arg_types.into(), @@ -542,7 +595,7 @@ impl ExprCollector<'_> { }, syntax_ptr, ) - } + }), ast::Expr::BinExpr(e) => { let op = e.op_kind(); if let Some(ast::BinaryOp::Assignment { op: None }) = op { @@ -581,7 +634,9 @@ impl ExprCollector<'_> { } ArrayExprKind::Repeat { initializer, repeat } => { let initializer = self.collect_expr_opt(initializer); - let repeat = self.collect_expr_opt(repeat); + let repeat = self.with_label_rib(RibKind::Constant, |this| { + this.collect_expr_opt(repeat) + }); self.alloc_expr( Expr::Array(Array::Repeat { initializer, repeat }), syntax_ptr, @@ -630,20 +685,24 @@ impl ExprCollector<'_> { /// Desugar `try { ; }` into `': { ; ::std::ops::Try::from_output() }`, /// `try { ; }` into `': { ; ::std::ops::Try::from_output(()) }` /// and save the `` to use it as a break target for desugaring of the `?` operator. - fn collect_try_block(&mut self, e: BlockExpr) -> ExprId { + fn desugar_try_block(&mut self, e: BlockExpr) -> ExprId { let Some(try_from_output) = LangItem::TryTraitFromOutput.path(self.db, self.krate) else { - return self.alloc_expr_desugared(Expr::Missing); + return self.collect_block(e); }; - let prev_try_block = self.current_try_block.take(); - self.current_try_block = - Some(self.alloc_label_desugared(Label { name: Name::generate_new_name() })); - let expr_id = self.collect_block(e); + let label = self.alloc_label_desugared(Label { name: Name::generate_new_name() }); + let old_label = self.current_try_block_label.replace(label); + + let (btail, expr_id) = self.with_labeled_rib(label, |this| { + let mut btail = None; + let block = this.collect_block_(e, |id, statements, tail| { + btail = tail; + Expr::Block { id, statements, tail, label: Some(label) } + }); + (btail, block) + }); + let callee = self.alloc_expr_desugared(Expr::Path(try_from_output)); - let Expr::Block { label, tail, .. } = &mut self.body.exprs[expr_id] else { - unreachable!("It is the output of collect block"); - }; - *label = self.current_try_block; - let next_tail = match *tail { + let next_tail = match btail { Some(tail) => self.alloc_expr_desugared(Expr::Call { callee, args: Box::new([tail]), @@ -662,10 +721,10 @@ impl ExprCollector<'_> { } }; let Expr::Block { tail, .. } = &mut self.body.exprs[expr_id] else { - unreachable!("It is the output of collect block"); + unreachable!("block was lowered to non-block"); }; *tail = Some(next_tail); - self.current_try_block = prev_try_block; + self.current_try_block_label = old_label; expr_id } @@ -735,12 +794,13 @@ impl ExprCollector<'_> { Expr::Call { callee, args: Box::new([x]), is_assignee_expr: false }, syntax_ptr.clone(), ); - if let Some(label) = self.current_try_block { - let label = Some(self.body.labels[label].name.clone()); - self.alloc_expr(Expr::Break { expr: Some(result), label }, syntax_ptr.clone()) - } else { - self.alloc_expr(Expr::Return { expr: Some(result) }, syntax_ptr.clone()) - } + self.alloc_expr( + match self.current_try_block_label { + Some(label) => Expr::Break { expr: Some(result), label: Some(label) }, + None => Expr::Return { expr: Some(result) }, + }, + syntax_ptr.clone(), + ) }, }; let arms = Box::new([continue_arm, break_arm]); @@ -966,6 +1026,17 @@ impl ExprCollector<'_> { } } + fn collect_labelled_block_opt( + &mut self, + label: Option, + expr: Option, + ) -> ExprId { + match label { + Some(label) => self.with_labeled_rib(label, |this| this.collect_block_opt(expr)), + None => self.collect_block_opt(expr), + } + } + fn collect_label(&mut self, ast_label: ast::Label) -> LabelId { let label = Label { name: ast_label.lifetime().as_ref().map_or_else(Name::missing, Name::new_lifetime), @@ -1135,8 +1206,9 @@ impl ExprCollector<'_> { Pat::Box { inner } } ast::Pat::ConstBlockPat(const_block_pat) => { - if let Some(expr) = const_block_pat.block_expr() { - let expr_id = self.collect_block(expr); + if let Some(block) = const_block_pat.block_expr() { + let expr_id = + self.with_label_rib(RibKind::Constant, |this| this.collect_block(block)); Pat::ConstBlock(expr_id) } else { Pat::Missing @@ -1213,6 +1285,57 @@ impl ExprCollector<'_> { fn add_definition_to_binding(&mut self, binding_id: BindingId, pat_id: PatId) { self.body.bindings[binding_id].definitions.push(pat_id); } + + fn resolve_label( + &self, + lifetime: Option, + ) -> Result, BodyDiagnostic> { + let Some(lifetime) = lifetime else { + return Ok(None) + }; + let name = Name::new_lifetime(&lifetime); + + for (rib_idx, rib) in self.label_ribs.iter().enumerate().rev() { + if let Some((label_name, id)) = &rib.label { + if *label_name == name { + return if self.is_label_valid_from_rib(rib_idx) { + Ok(Some(*id)) + } else { + Err(BodyDiagnostic::UnreachableLabel { + name, + node: InFile::new( + self.expander.current_file_id, + AstPtr::new(&lifetime), + ), + }) + }; + } + } + } + + Err(BodyDiagnostic::UndeclaredLabel { + name, + node: InFile::new(self.expander.current_file_id, AstPtr::new(&lifetime)), + }) + } + + fn is_label_valid_from_rib(&self, rib_index: usize) -> bool { + !self.label_ribs[rib_index + 1..].iter().any(|rib| rib.kind.is_label_barrier()) + } + + fn with_label_rib(&mut self, kind: RibKind, f: impl FnOnce(&mut Self) -> T) -> T { + self.label_ribs.push(LabelRib::new(kind)); + let res = f(self); + self.label_ribs.pop(); + res + } + + fn with_labeled_rib(&mut self, label: LabelId, f: impl FnOnce(&mut Self) -> T) -> T { + self.label_ribs.push(LabelRib::new_normal((self.body[label].name.clone(), label))); + let res = f(self); + self.label_ribs.pop(); + res + } } impl From for Literal { diff --git a/crates/hir-def/src/body/pretty.rs b/crates/hir-def/src/body/pretty.rs index 8c9d77620e..9ac42c8621 100644 --- a/crates/hir-def/src/body/pretty.rs +++ b/crates/hir-def/src/body/pretty.rs @@ -219,14 +219,14 @@ impl<'a> Printer<'a> { } Expr::Continue { label } => { w!(self, "continue"); - if let Some(label) = label { - w!(self, " {}", label); + if let Some(lbl) = label { + w!(self, " {}", self.body[*lbl].name); } } Expr::Break { expr, label } => { w!(self, "break"); - if let Some(label) = label { - w!(self, " {}", label); + if let Some(lbl) = label { + w!(self, " {}", self.body[*lbl].name); } if let Some(expr) = expr { self.whitespace(); diff --git a/crates/hir-def/src/expr.rs b/crates/hir-def/src/expr.rs index 443594d271..9e83d8ab11 100644 --- a/crates/hir-def/src/expr.rs +++ b/crates/hir-def/src/expr.rs @@ -168,11 +168,11 @@ pub enum Expr { arms: Box<[MatchArm]>, }, Continue { - label: Option, + label: Option, }, Break { expr: Option, - label: Option, + label: Option, }, Return { expr: Option, diff --git a/crates/hir-expand/src/name.rs b/crates/hir-expand/src/name.rs index 8099c20b02..4e688c431a 100644 --- a/crates/hir-expand/src/name.rs +++ b/crates/hir-expand/src/name.rs @@ -120,8 +120,7 @@ impl Name { use std::sync::atomic::{AtomicUsize, Ordering}; static CNT: AtomicUsize = AtomicUsize::new(0); let c = CNT.fetch_add(1, Ordering::Relaxed); - // FIXME: Currently a `__RA_generated_name` in user code will break our analysis - Name::new_text(format!("__RA_geneated_name_{c}").into()) + Name::new_text(format!("{c}").into()) } /// Returns the tuple index this name represents if it is a tuple field. diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index 0c41b92026..ed530408ab 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -18,6 +18,7 @@ use std::{convert::identity, ops::Index}; use chalk_ir::{cast::Cast, DebruijnIndex, Mutability, Safety, Scalar, TypeFlags}; use either::Either; +use hir_def::expr::LabelId; use hir_def::{ body::Body, builtin_type::{BuiltinInt, BuiltinType, BuiltinUint}, @@ -188,12 +189,6 @@ pub enum InferenceDiagnostic { /// Contains the type the field resolves to field_with_same_name: Option, }, - // FIXME: Make this proper - BreakOutsideOfLoop { - expr: ExprId, - is_break: bool, - bad_value_break: bool, - }, MismatchedArgCount { call_expr: ExprId, expected: usize, @@ -468,7 +463,7 @@ struct BreakableContext { /// The coercion target of the context. coerce: Option, /// The optional label of the context. - label: Option, + label: Option, kind: BreakableKind, } @@ -483,28 +478,18 @@ enum BreakableKind { fn find_breakable<'c>( ctxs: &'c mut [BreakableContext], - label: Option<&name::Name>, + label: Option, ) -> Option<&'c mut BreakableContext> { let mut ctxs = ctxs .iter_mut() .rev() .take_while(|it| matches!(it.kind, BreakableKind::Block | BreakableKind::Loop)); match label { - Some(_) => ctxs.find(|ctx| ctx.label.as_ref() == label), + Some(_) => ctxs.find(|ctx| ctx.label == label), None => ctxs.find(|ctx| matches!(ctx.kind, BreakableKind::Loop)), } } -fn find_continuable<'c>( - ctxs: &'c mut [BreakableContext], - label: Option<&name::Name>, -) -> Option<&'c mut BreakableContext> { - match label { - Some(_) => find_breakable(ctxs, label).filter(|it| matches!(it.kind, BreakableKind::Loop)), - None => find_breakable(ctxs, label), - } -} - impl<'a> InferenceContext<'a> { fn new( db: &'a dyn HirDatabase, diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 035f61fc18..4e62e41b58 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -25,9 +25,7 @@ use syntax::ast::RangeOp; use crate::{ autoderef::{builtin_deref, deref_by_trait, Autoderef}, consteval, - infer::{ - coerce::CoerceMany, find_continuable, pat::contains_explicit_ref_binding, BreakableKind, - }, + infer::{coerce::CoerceMany, pat::contains_explicit_ref_binding, BreakableKind}, lang_items::lang_items_for_bin_op, lower::{ const_or_path_to_chalk, generic_arg_to_chalk, lower_to_chalk_mutability, ParamLoweringMode, @@ -459,29 +457,13 @@ impl<'a> InferenceContext<'a> { self.resolver.reset_to_guard(g); ty } - Expr::Continue { label } => { - if let None = find_continuable(&mut self.breakables, label.as_ref()) { - self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop { - expr: tgt_expr, - is_break: false, - bad_value_break: false, - }); - }; - self.result.standard_types.never.clone() - } - Expr::Break { expr, label } => { - let val_ty = if let Some(expr) = *expr { - let opt_coerce_to = match find_breakable(&mut self.breakables, label.as_ref()) { + Expr::Continue { .. } => self.result.standard_types.never.clone(), + &Expr::Break { expr, label } => { + let val_ty = if let Some(expr) = expr { + let opt_coerce_to = match find_breakable(&mut self.breakables, label) { Some(ctxt) => match &ctxt.coerce { Some(coerce) => coerce.expected_ty(), - None => { - self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop { - expr: tgt_expr, - is_break: true, - bad_value_break: true, - }); - self.err_ty() - } + None => self.err_ty(), }, None => self.err_ty(), }; @@ -490,26 +472,20 @@ impl<'a> InferenceContext<'a> { TyBuilder::unit() }; - match find_breakable(&mut self.breakables, label.as_ref()) { + match find_breakable(&mut self.breakables, label) { Some(ctxt) => match ctxt.coerce.take() { Some(mut coerce) => { - coerce.coerce(self, *expr, &val_ty); + coerce.coerce(self, expr, &val_ty); // Avoiding borrowck - let ctxt = find_breakable(&mut self.breakables, label.as_ref()) + let ctxt = find_breakable(&mut self.breakables, label) .expect("breakable stack changed during coercion"); ctxt.may_break = true; ctxt.coerce = Some(coerce); } None => ctxt.may_break = true, }, - None => { - self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop { - expr: tgt_expr, - is_break: true, - bad_value_break: false, - }); - } + None => {} } self.result.standard_types.never.clone() } @@ -1900,7 +1876,6 @@ impl<'a> InferenceContext<'a> { cb: impl FnOnce(&mut Self) -> T, ) -> (Option, T) { self.breakables.push({ - let label = label.map(|label| self.body[label].name.clone()); BreakableContext { kind, may_break: false, coerce: ty.map(CoerceMany::new), label } }); let res = cb(self); diff --git a/crates/hir-ty/src/mir/lower.rs b/crates/hir-ty/src/mir/lower.rs index 1821796be3..0aa5b4dc8d 100644 --- a/crates/hir-ty/src/mir/lower.rs +++ b/crates/hir-ty/src/mir/lower.rs @@ -47,7 +47,7 @@ struct MirLowerCtx<'a> { current_loop_blocks: Option, // FIXME: we should resolve labels in HIR lowering and always work with label id here, not // with raw names. - labeled_loop_blocks: FxHashMap, + labeled_loop_blocks: FxHashMap, discr_temp: Option, db: &'a dyn HirDatabase, body: &'a Body, @@ -579,19 +579,19 @@ impl MirLowerCtx<'_> { Ok(None) } }, - Expr::Break { expr, label } => { + &Expr::Break { expr, label } => { if let Some(expr) = expr { let loop_data = match label { - Some(l) => self.labeled_loop_blocks.get(l).ok_or(MirLowerError::UnresolvedLabel)?, + Some(l) => self.labeled_loop_blocks.get(&l).ok_or(MirLowerError::UnresolvedLabel)?, None => self.current_loop_blocks.as_ref().ok_or(MirLowerError::BreakWithoutLoop)?, }; - let Some(c) = self.lower_expr_to_place(*expr, loop_data.place.clone(), current)? else { + let Some(c) = self.lower_expr_to_place(expr, loop_data.place.clone(), current)? else { return Ok(None); }; current = c; } let end = match label { - Some(l) => self.labeled_loop_blocks.get(l).ok_or(MirLowerError::UnresolvedLabel)?.end.expect("We always generate end for labeled loops"), + Some(l) => self.labeled_loop_blocks.get(&l).ok_or(MirLowerError::UnresolvedLabel)?.end.expect("We always generate end for labeled loops"), None => self.current_loop_end()?, }; self.set_goto(current, end); @@ -1119,10 +1119,8 @@ impl MirLowerCtx<'_> { // bad as we may emit end (unneccessary unreachable block) for unterminating loop, but // it should not affect correctness. self.current_loop_end()?; - self.labeled_loop_blocks.insert( - self.body.labels[label].name.clone(), - self.current_loop_blocks.as_ref().unwrap().clone(), - ) + self.labeled_loop_blocks + .insert(label, self.current_loop_blocks.as_ref().unwrap().clone()) } else { None }; @@ -1131,7 +1129,7 @@ impl MirLowerCtx<'_> { let my = mem::replace(&mut self.current_loop_blocks, prev) .ok_or(MirLowerError::ImplementationError("current_loop_blocks is corrupt"))?; if let Some(prev) = prev_label { - self.labeled_loop_blocks.insert(self.body.labels[label.unwrap()].name.clone(), prev); + self.labeled_loop_blocks.insert(label.unwrap(), prev); } Ok(my.end) } diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 253d62dafc..605bac6157 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -32,7 +32,6 @@ macro_rules! diagnostics { } diagnostics![ - BreakOutsideOfLoop, ExpectedFunction, InactiveCode, IncorrectCase, @@ -50,7 +49,9 @@ diagnostics![ PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch, + UndeclaredLabel, UnimplementedBuiltinMacro, + UnreachableLabel, UnresolvedExternCrate, UnresolvedField, UnresolvedImport, @@ -84,6 +85,17 @@ pub struct UnresolvedMacroCall { pub path: ModPath, pub is_bang: bool, } +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct UnreachableLabel { + pub node: InFile>, + pub name: Name, +} + +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct UndeclaredLabel { + pub node: InFile>, + pub name: Name, +} #[derive(Debug, Clone, Eq, PartialEq)] pub struct InactiveCode { @@ -166,13 +178,6 @@ pub struct PrivateField { pub field: Field, } -#[derive(Debug)] -pub struct BreakOutsideOfLoop { - pub expr: InFile>, - pub is_break: bool, - pub bad_value_break: bool, -} - #[derive(Debug)] pub struct MissingUnsafe { pub expr: InFile>, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 624ee1fd30..cd4d8e17d3 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -85,12 +85,13 @@ use crate::db::{DefDatabase, HirDatabase}; pub use crate::{ attrs::{HasAttrs, Namespace}, diagnostics::{ - AnyDiagnostic, BreakOutsideOfLoop, ExpectedFunction, InactiveCode, IncoherentImpl, - IncorrectCase, InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount, - MissingFields, MissingMatchArms, MissingUnsafe, NeedMut, NoSuchField, PrivateAssocItem, - PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch, UnimplementedBuiltinMacro, - UnresolvedExternCrate, UnresolvedField, UnresolvedImport, UnresolvedMacroCall, - UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro, UnusedMut, + AnyDiagnostic, ExpectedFunction, InactiveCode, IncoherentImpl, IncorrectCase, + InvalidDeriveTarget, MacroError, MalformedDerive, MismatchedArgCount, MissingFields, + MissingMatchArms, MissingUnsafe, NeedMut, NoSuchField, PrivateAssocItem, PrivateField, + ReplaceFilterMapNextWithFindMap, TypeMismatch, UndeclaredLabel, UnimplementedBuiltinMacro, + UnreachableLabel, UnresolvedExternCrate, UnresolvedField, UnresolvedImport, + UnresolvedMacroCall, UnresolvedMethodCall, UnresolvedModule, UnresolvedProcMacro, + UnusedMut, }, has_source::HasSource, semantics::{PathResolution, Semantics, SemanticsScope, TypeInfo, VisibleTraits}, @@ -1393,6 +1394,12 @@ impl DefWithBody { } .into(), ), + BodyDiagnostic::UnreachableLabel { node, name } => { + acc.push(UnreachableLabel { node: node.clone(), name: name.clone() }.into()) + } + BodyDiagnostic::UndeclaredLabel { node, name } => { + acc.push(UndeclaredLabel { node: node.clone(), name: name.clone() }.into()) + } } } @@ -1405,14 +1412,6 @@ impl DefWithBody { let field = source_map.field_syntax(expr); acc.push(NoSuchField { field }.into()) } - &hir_ty::InferenceDiagnostic::BreakOutsideOfLoop { - expr, - is_break, - bad_value_break, - } => { - let expr = expr_syntax(expr); - acc.push(BreakOutsideOfLoop { expr, is_break, bad_value_break }.into()) - } &hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => { acc.push( MismatchedArgCount { call_expr: expr_syntax(call_expr), expected, found } diff --git a/crates/ide-diagnostics/src/handlers/undeclared_label.rs b/crates/ide-diagnostics/src/handlers/undeclared_label.rs new file mode 100644 index 0000000000..9d5cbb31bb --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/undeclared_label.rs @@ -0,0 +1,61 @@ +use crate::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: undeclared-label +pub(crate) fn undeclared_label( + ctx: &DiagnosticsContext<'_>, + d: &hir::UndeclaredLabel, +) -> Diagnostic { + let name = &d.name; + Diagnostic::new( + "undeclared-label", + format!("use of undeclared label `{name}`"), + ctx.sema.diagnostics_display_range(d.node.clone().map(|it| it.into())).range, + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::check_diagnostics; + + #[test] + fn smoke_test() { + check_diagnostics( + r#" +fn foo() { + break 'a; + //^^ error: use of undeclared label `'a` + continue 'a; + //^^ error: use of undeclared label `'a` +} +"#, + ); + } + + #[test] + fn try_operator_desugar_works() { + check_diagnostics( + r#" +//- minicore: option, try +fn foo() { + None?; +} +"#, + ); + check_diagnostics( + r#" +//- minicore: option, try, future +async fn foo() { + None?; +} +"#, + ); + check_diagnostics( + r#" +//- minicore: option, try, future, fn +async fn foo() { + || None?; +} +"#, + ); + } +} diff --git a/crates/ide-diagnostics/src/handlers/unreachable_label.rs b/crates/ide-diagnostics/src/handlers/unreachable_label.rs new file mode 100644 index 0000000000..f09659fa9e --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/unreachable_label.rs @@ -0,0 +1,87 @@ +use crate::{Diagnostic, DiagnosticsContext}; + +// Diagnostic: unreachable-label +pub(crate) fn unreachable_label( + ctx: &DiagnosticsContext<'_>, + d: &hir::UnreachableLabel, +) -> Diagnostic { + let name = &d.name; + Diagnostic::new( + "unreachable-label", + format!("use of unreachable label `{name}`"), + ctx.sema.diagnostics_display_range(d.node.clone().map(|it| it.into())).range, + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::check_diagnostics; + + #[test] + fn async_blocks_are_borders() { + check_diagnostics( + r#" +fn foo() { + 'a: loop { + async { + break 'a; + // ^^ error: use of unreachable label `'a` + continue 'a; + // ^^ error: use of unreachable label `'a` + }; + } +} +"#, + ); + } + + #[test] + fn closures_are_borders() { + check_diagnostics( + r#" +fn foo() { + 'a: loop { + || { + break 'a; + // ^^ error: use of unreachable label `'a` + continue 'a; + // ^^ error: use of unreachable label `'a` + }; + } +} +"#, + ); + } + + #[test] + fn blocks_pass_through() { + check_diagnostics( + r#" +fn foo() { + 'a: loop { + { + break 'a; + continue 'a; + } + } +} +"#, + ); + } + + #[test] + fn try_blocks_pass_through() { + check_diagnostics( + r#" +fn foo() { + 'a: loop { + try { + break 'a; + continue 'a; + }; + } +} +"#, + ); + } +} diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index 0dc5343f94..5cb1d4e1b8 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -26,7 +26,6 @@ #![warn(rust_2018_idioms, unused_lifetimes, semicolon_in_expressions_from_macros)] mod handlers { - pub(crate) mod break_outside_of_loop; pub(crate) mod expected_function; pub(crate) mod inactive_code; pub(crate) mod incoherent_impl; @@ -52,6 +51,8 @@ mod handlers { pub(crate) mod unresolved_macro_call; pub(crate) mod unresolved_module; pub(crate) mod unresolved_proc_macro; + pub(crate) mod undeclared_label; + pub(crate) mod unreachable_label; // The handlers below are unusual, the implement the diagnostics as well. pub(crate) mod field_shorthand; @@ -253,36 +254,38 @@ pub fn diagnostics( for diag in diags { #[rustfmt::skip] let d = match diag { - AnyDiagnostic::BreakOutsideOfLoop(d) => handlers::break_outside_of_loop::break_outside_of_loop(&ctx, &d), AnyDiagnostic::ExpectedFunction(d) => handlers::expected_function::expected_function(&ctx, &d), - AnyDiagnostic::IncorrectCase(d) => handlers::incorrect_case::incorrect_case(&ctx, &d), + AnyDiagnostic::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) { + Some(it) => it, + None => continue, + } AnyDiagnostic::IncoherentImpl(d) => handlers::incoherent_impl::incoherent_impl(&ctx, &d), + AnyDiagnostic::IncorrectCase(d) => handlers::incorrect_case::incorrect_case(&ctx, &d), + AnyDiagnostic::InvalidDeriveTarget(d) => handlers::invalid_derive_target::invalid_derive_target(&ctx, &d), AnyDiagnostic::MacroError(d) => handlers::macro_error::macro_error(&ctx, &d), AnyDiagnostic::MalformedDerive(d) => handlers::malformed_derive::malformed_derive(&ctx, &d), AnyDiagnostic::MismatchedArgCount(d) => handlers::mismatched_arg_count::mismatched_arg_count(&ctx, &d), AnyDiagnostic::MissingFields(d) => handlers::missing_fields::missing_fields(&ctx, &d), AnyDiagnostic::MissingMatchArms(d) => handlers::missing_match_arms::missing_match_arms(&ctx, &d), AnyDiagnostic::MissingUnsafe(d) => handlers::missing_unsafe::missing_unsafe(&ctx, &d), + AnyDiagnostic::NeedMut(d) => handlers::mutability_errors::need_mut(&ctx, &d), AnyDiagnostic::NoSuchField(d) => handlers::no_such_field::no_such_field(&ctx, &d), AnyDiagnostic::PrivateAssocItem(d) => handlers::private_assoc_item::private_assoc_item(&ctx, &d), AnyDiagnostic::PrivateField(d) => handlers::private_field::private_field(&ctx, &d), AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => handlers::replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), AnyDiagnostic::TypeMismatch(d) => handlers::type_mismatch::type_mismatch(&ctx, &d), + AnyDiagnostic::UndeclaredLabel(d) => handlers::undeclared_label::undeclared_label(&ctx, &d), AnyDiagnostic::UnimplementedBuiltinMacro(d) => handlers::unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), + AnyDiagnostic::UnreachableLabel(d) => handlers::unreachable_label:: unreachable_label(&ctx, &d), AnyDiagnostic::UnresolvedExternCrate(d) => handlers::unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), + AnyDiagnostic::UnresolvedField(d) => handlers::unresolved_field::unresolved_field(&ctx, &d), AnyDiagnostic::UnresolvedImport(d) => handlers::unresolved_import::unresolved_import(&ctx, &d), AnyDiagnostic::UnresolvedMacroCall(d) => handlers::unresolved_macro_call::unresolved_macro_call(&ctx, &d), + AnyDiagnostic::UnresolvedMethodCall(d) => handlers::unresolved_method::unresolved_method(&ctx, &d), AnyDiagnostic::UnresolvedModule(d) => handlers::unresolved_module::unresolved_module(&ctx, &d), AnyDiagnostic::UnresolvedProcMacro(d) => handlers::unresolved_proc_macro::unresolved_proc_macro(&ctx, &d, config.proc_macros_enabled, config.proc_attr_macros_enabled), - AnyDiagnostic::InvalidDeriveTarget(d) => handlers::invalid_derive_target::invalid_derive_target(&ctx, &d), - AnyDiagnostic::UnresolvedField(d) => handlers::unresolved_field::unresolved_field(&ctx, &d), - AnyDiagnostic::UnresolvedMethodCall(d) => handlers::unresolved_method::unresolved_method(&ctx, &d), - AnyDiagnostic::NeedMut(d) => handlers::mutability_errors::need_mut(&ctx, &d), AnyDiagnostic::UnusedMut(d) => handlers::mutability_errors::unused_mut(&ctx, &d), - AnyDiagnostic::InactiveCode(d) => match handlers::inactive_code::inactive_code(&ctx, &d) { - Some(it) => it, - None => continue, - } + }; res.push(d) }