diff options
| -rw-r--r-- | crates/hir-def/src/body.rs | 5 | ||||
| -rw-r--r-- | crates/hir-def/src/body/lower.rs | 275 | ||||
| -rw-r--r-- | crates/hir-def/src/body/pretty.rs | 8 | ||||
| -rw-r--r-- | crates/hir-def/src/expr.rs | 4 | ||||
| -rw-r--r-- | crates/hir-expand/src/name.rs | 3 | ||||
| -rw-r--r-- | crates/hir-ty/src/infer.rs | 11 | ||||
| -rw-r--r-- | crates/hir-ty/src/infer/expr.rs | 17 | ||||
| -rw-r--r-- | crates/hir-ty/src/mir/lower.rs | 18 | ||||
| -rw-r--r-- | crates/hir/src/diagnostics.rs | 27 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 29 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs | 19 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/undeclared_label.rs | 63 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/handlers/unreachable_label.rs | 91 | ||||
| -rw-r--r-- | crates/ide-diagnostics/src/lib.rs | 24 |
14 files changed, 438 insertions, 156 deletions
diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index bd87eaa2217..0bea905e115 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<AstPtr<ast::MacroCall>>, message: String }, UnresolvedProcMacro { node: InFile<AstPtr<ast::MacroCall>>, krate: CrateId }, UnresolvedMacroCall { node: InFile<AstPtr<ast::MacroCall>>, path: ModPath }, + UnreachableLabel { node: InFile<AstPtr<ast::Lifetime>>, name: Name }, + UndeclaredLabel { node: InFile<AstPtr<ast::Lifetime>>, name: Name }, } impl Body { diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 53627375831..91eeff5b0eb 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<LabelId>, + current_try_block_label: Option<LabelId>, is_lowering_assignee_expr: bool, is_lowering_generator: bool, + label_ribs: Vec<LabelRib>, +} + +#[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 { <stmts>; <expr> }` into `'<new_label>: { <stmts>; ::std::ops::Try::from_output(<expr>) }`, /// `try { <stmts>; }` into `'<new_label>: { <stmts>; ::std::ops::Try::from_output(()) }` /// and save the `<new_label>` 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<LabelId>, + expr: Option<ast::BlockExpr>, + ) -> 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<ast::Lifetime>, + ) -> Result<Option<LabelId>, 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<T>(&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<T>(&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<ast::LiteralKind> for Literal { diff --git a/crates/hir-def/src/body/pretty.rs b/crates/hir-def/src/body/pretty.rs index 8c9d77620e1..9ac42c86214 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 443594d2717..9e83d8ab118 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<Name>, + label: Option<LabelId>, }, Break { expr: Option<ExprId>, - label: Option<Name>, + label: Option<LabelId>, }, Return { expr: Option<ExprId>, diff --git a/crates/hir-expand/src/name.rs b/crates/hir-expand/src/name.rs index 8099c20b027..4e688c431ae 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!("<ra@gennew>{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 0c41b92026a..c1012cc3d9c 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,7 +189,7 @@ pub enum InferenceDiagnostic { /// Contains the type the field resolves to field_with_same_name: Option<Ty>, }, - // FIXME: Make this proper + // FIXME: This should be emitted in body lowering BreakOutsideOfLoop { expr: ExprId, is_break: bool, @@ -468,7 +469,7 @@ struct BreakableContext { /// The coercion target of the context. coerce: Option<CoerceMany>, /// The optional label of the context. - label: Option<name::Name>, + label: Option<LabelId>, kind: BreakableKind, } @@ -483,21 +484,21 @@ enum BreakableKind { fn find_breakable<'c>( ctxs: &'c mut [BreakableContext], - label: Option<&name::Name>, + label: Option<LabelId>, ) -> 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>, + label: Option<LabelId>, ) -> Option<&'c mut BreakableContext> { match label { Some(_) => find_breakable(ctxs, label).filter(|it| matches!(it.kind, BreakableKind::Loop)), diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 035f61fc180..129ff33ae5d 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -459,8 +459,8 @@ 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()) { + &Expr::Continue { label } => { + if let None = find_continuable(&mut self.breakables, label) { self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop { expr: tgt_expr, is_break: false, @@ -469,9 +469,9 @@ impl<'a> InferenceContext<'a> { }; 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::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 => { @@ -490,13 +490,13 @@ 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); @@ -1900,7 +1900,6 @@ impl<'a> InferenceContext<'a> { cb: impl FnOnce(&mut Self) -> T, ) -> (Option<Ty>, 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 1821796be33..0aa5b4dc8d5 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<LoopBlocks>, // FIXME: we should resolve labels in HIR lowering and always work with label id here, not // with raw names. - labeled_loop_blocks: FxHashMap<Name, LoopBlocks>, + labeled_loop_blocks: FxHashMap<LabelId, LoopBlocks>, discr_temp: Option<Place>, 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 253d62dafc6..f756832f0ff 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -50,7 +50,9 @@ diagnostics![ PrivateField, ReplaceFilterMapNextWithFindMap, TypeMismatch, + UndeclaredLabel, UnimplementedBuiltinMacro, + UnreachableLabel, UnresolvedExternCrate, UnresolvedField, UnresolvedImport, @@ -62,6 +64,13 @@ diagnostics![ ]; #[derive(Debug)] +pub struct BreakOutsideOfLoop { + pub expr: InFile<AstPtr<ast::Expr>>, + pub is_break: bool, + pub bad_value_break: bool, +} + +#[derive(Debug)] pub struct UnresolvedModule { pub decl: InFile<AstPtr<ast::Module>>, pub candidates: Box<[String]>, @@ -84,6 +93,17 @@ pub struct UnresolvedMacroCall { pub path: ModPath, pub is_bang: bool, } +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct UnreachableLabel { + pub node: InFile<AstPtr<ast::Lifetime>>, + pub name: Name, +} + +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct UndeclaredLabel { + pub node: InFile<AstPtr<ast::Lifetime>>, + pub name: Name, +} #[derive(Debug, Clone, Eq, PartialEq)] pub struct InactiveCode { @@ -167,13 +187,6 @@ pub struct PrivateField { } #[derive(Debug)] -pub struct BreakOutsideOfLoop { - pub expr: InFile<AstPtr<ast::Expr>>, - pub is_break: bool, - pub bad_value_break: bool, -} - -#[derive(Debug)] pub struct MissingUnsafe { pub expr: InFile<AstPtr<ast::Expr>>, } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 624ee1fd30a..dbb41b1b667 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -88,9 +88,10 @@ pub use crate::{ 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, + 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 } @@ -1484,6 +1483,14 @@ impl DefWithBody { .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()) + } } } for (pat_or_expr, mismatch) in infer.type_mismatches() { diff --git a/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs b/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs index 114face2dca..89aa437d75d 100644 --- a/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs +++ b/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs @@ -31,12 +31,8 @@ mod tests { fn foo() { break; //^^^^^ error: break outside of loop - break 'a; - //^^^^^^^^ error: break outside of loop continue; //^^^^^^^^ error: continue outside of loop - continue 'a; - //^^^^^^^^^^^ error: continue outside of loop } "#, ); @@ -51,12 +47,8 @@ fn foo() { async { break; //^^^^^ error: break outside of loop - break 'a; - //^^^^^^^^ error: break outside of loop continue; //^^^^^^^^ error: continue outside of loop - continue 'a; - //^^^^^^^^^^^ error: continue outside of loop }; } } @@ -73,12 +65,8 @@ fn foo() { || { break; //^^^^^ error: break outside of loop - break 'a; - //^^^^^^^^ error: break outside of loop continue; //^^^^^^^^ error: continue outside of loop - continue 'a; - //^^^^^^^^^^^ error: continue outside of loop }; } } @@ -94,9 +82,7 @@ fn foo() { 'a: loop { { break; - break 'a; continue; - continue 'a; } } } @@ -112,9 +98,7 @@ fn foo() { 'a: loop { try { break; - break 'a; continue; - continue 'a; }; } } @@ -130,11 +114,8 @@ fn foo() { 'a: { break; //^^^^^ error: break outside of loop - break 'a; continue; //^^^^^^^^ error: continue outside of loop - continue 'a; - //^^^^^^^^^^^ error: continue outside of loop } } "#, 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 00000000000..dbedf1e6c1e --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/undeclared_label.rs @@ -0,0 +1,63 @@ +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: break outside of loop + //^^ error: use of undeclared label `'a` + continue 'a; + //^^^^^^^^^^^ error: continue outside of loop + //^^ 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 00000000000..5933a9b694a --- /dev/null +++ b/crates/ide-diagnostics/src/handlers/unreachable_label.rs @@ -0,0 +1,91 @@ +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: break outside of loop + // ^^ error: use of unreachable label `'a` + continue 'a; + //^^^^^^^^^^^ error: continue outside of loop + // ^^ error: use of unreachable label `'a` + }; + } +} +"#, + ); + } + + #[test] + fn closures_are_borders() { + check_diagnostics( + r#" +fn foo() { + 'a: loop { + || { + break 'a; + //^^^^^^^^ error: break outside of loop + // ^^ error: use of unreachable label `'a` + continue 'a; + //^^^^^^^^^^^ error: continue outside of loop + // ^^ 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 0dc5343f942..70116f15a75 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -52,6 +52,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 +255,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, - } + AnyDiagnostic::BreakOutsideOfLoop(d) => handlers::break_outside_of_loop::break_outside_of_loop(&ctx, &d), }; res.push(d) } |