From 02eb2d758e88ab1afb7b04ea0e8dbf0310c4e5a6 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 3 Mar 2023 16:44:25 +0100 Subject: [PATCH 1/2] Distinguish between expected and final type in CoerceMany --- crates/hir-ty/src/infer.rs | 23 +++- crates/hir-ty/src/infer/coerce.rs | 58 ++++++--- crates/hir-ty/src/infer/expr.rs | 194 +++++++++++++++++++----------- 3 files changed, 187 insertions(+), 88 deletions(-) diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index f229bf2f644..d3e7f6e7dca 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -66,8 +66,10 @@ pub(crate) fn infer_query(db: &dyn HirDatabase, def: DefWithBodyId) -> Arc { + ctx.collect_fn(f); + } DefWithBodyId::ConstId(c) => ctx.collect_const(&db.const_data(c)), - DefWithBodyId::FunctionId(f) => ctx.collect_fn(f), DefWithBodyId::StaticId(s) => ctx.collect_static(&db.static_data(s)), DefWithBodyId::VariantId(v) => { ctx.return_ty = TyBuilder::builtin(match db.enum_data(v.parent).variant_body_type() { @@ -392,9 +394,12 @@ pub(crate) struct InferenceContext<'a> { /// currently within one. /// /// We might consider using a nested inference context for checking - /// closures, but currently this is the only field that will change there, - /// so it doesn't make sense. + /// closures so we can swap all shared things out at once. return_ty: Ty, + /// If `Some`, this stores coercion information for returned + /// expressions. If `None`, this is in a context where return is + /// inappropriate, such as a const expression. + return_coercion: Option, /// The resume type and the yield type, respectively, of the generator being inferred. resume_yield_tys: Option<(Ty, Ty)>, diverges: Diverges, @@ -462,6 +467,7 @@ fn new( trait_env, return_ty: TyKind::Error.intern(Interner), // set in collect_* calls resume_yield_tys: None, + return_coercion: None, db, owner, body, @@ -595,10 +601,19 @@ fn collect_fn(&mut self, func: FunctionId) { }; self.return_ty = self.normalize_associated_types_in(return_ty); + self.return_coercion = Some(CoerceMany::new(self.return_ty.clone())); } fn infer_body(&mut self) { - self.infer_expr_coerce(self.body.body_expr, &Expectation::has_type(self.return_ty.clone())); + match self.return_coercion { + Some(_) => self.infer_return(self.body.body_expr), + None => { + _ = self.infer_expr_coerce( + self.body.body_expr, + &Expectation::has_type(self.return_ty.clone()), + ) + } + } } fn write_expr_ty(&mut self, expr: ExprId, ty: Ty) { diff --git a/crates/hir-ty/src/infer/coerce.rs b/crates/hir-ty/src/infer/coerce.rs index 3293534a068..8bce47d71cb 100644 --- a/crates/hir-ty/src/infer/coerce.rs +++ b/crates/hir-ty/src/infer/coerce.rs @@ -50,11 +50,44 @@ fn success( #[derive(Clone, Debug)] pub(super) struct CoerceMany { expected_ty: Ty, + final_ty: Option, } impl CoerceMany { pub(super) fn new(expected: Ty) -> Self { - CoerceMany { expected_ty: expected } + CoerceMany { expected_ty: expected, final_ty: None } + } + + /// Returns the "expected type" with which this coercion was + /// constructed. This represents the "downward propagated" type + /// that was given to us at the start of typing whatever construct + /// we are typing (e.g., the match expression). + /// + /// Typically, this is used as the expected type when + /// type-checking each of the alternative expressions whose types + /// we are trying to merge. + pub(super) fn expected_ty(&self) -> Ty { + self.expected_ty.clone() + } + + /// Returns the current "merged type", representing our best-guess + /// at the LUB of the expressions we've seen so far (if any). This + /// isn't *final* until you call `self.complete()`, which will return + /// the merged type. + pub(super) fn merged_ty(&self) -> Ty { + self.final_ty.clone().unwrap_or_else(|| self.expected_ty.clone()) + } + + pub(super) fn complete(self, ctx: &mut InferenceContext<'_>) -> Ty { + if let Some(final_ty) = self.final_ty { + final_ty + } else { + ctx.result.standard_types.never.clone() + } + } + + pub(super) fn coerce_forced_unit(&mut self, ctx: &mut InferenceContext<'_>) { + self.coerce(ctx, None, &ctx.result.standard_types.unit.clone()) } /// Merge two types from different branches, with possible coercion. @@ -76,25 +109,25 @@ pub(super) fn coerce( // Special case: two function types. Try to coerce both to // pointers to have a chance at getting a match. See // https://github.com/rust-lang/rust/blob/7b805396bf46dce972692a6846ce2ad8481c5f85/src/librustc_typeck/check/coercion.rs#L877-L916 - let sig = match (self.expected_ty.kind(Interner), expr_ty.kind(Interner)) { + let sig = match (self.merged_ty().kind(Interner), expr_ty.kind(Interner)) { (TyKind::FnDef(..) | TyKind::Closure(..), TyKind::FnDef(..) | TyKind::Closure(..)) => { // FIXME: we're ignoring safety here. To be more correct, if we have one FnDef and one Closure, // we should be coercing the closure to a fn pointer of the safety of the FnDef cov_mark::hit!(coerce_fn_reification); let sig = - self.expected_ty.callable_sig(ctx.db).expect("FnDef without callable sig"); + self.merged_ty().callable_sig(ctx.db).expect("FnDef without callable sig"); Some(sig) } _ => None, }; if let Some(sig) = sig { let target_ty = TyKind::Function(sig.to_fn_ptr()).intern(Interner); - let result1 = ctx.table.coerce_inner(self.expected_ty.clone(), &target_ty); + let result1 = ctx.table.coerce_inner(self.merged_ty(), &target_ty); let result2 = ctx.table.coerce_inner(expr_ty.clone(), &target_ty); if let (Ok(result1), Ok(result2)) = (result1, result2) { ctx.table.register_infer_ok(result1); ctx.table.register_infer_ok(result2); - return self.expected_ty = target_ty; + return self.final_ty = Some(target_ty); } } @@ -102,25 +135,20 @@ pub(super) fn coerce( // type is a type variable and the new one is `!`, trying it the other // way around first would mean we make the type variable `!`, instead of // just marking it as possibly diverging. - if ctx.coerce(expr, &expr_ty, &self.expected_ty).is_ok() { - /* self.expected_ty is already correct */ - } else if ctx.coerce(expr, &self.expected_ty, &expr_ty).is_ok() { - self.expected_ty = expr_ty; + if let Ok(res) = ctx.coerce(expr, &expr_ty, &self.merged_ty()) { + self.final_ty = Some(res); + } else if let Ok(res) = ctx.coerce(expr, &self.merged_ty(), &expr_ty) { + self.final_ty = Some(res); } else { if let Some(id) = expr { ctx.result.type_mismatches.insert( id.into(), - TypeMismatch { expected: self.expected_ty.clone(), actual: expr_ty }, + TypeMismatch { expected: self.merged_ty().clone(), actual: expr_ty.clone() }, ); } cov_mark::hit!(coerce_merge_fail_fallback); - /* self.expected_ty is already correct */ } } - - pub(super) fn complete(self) -> Ty { - self.expected_ty - } } pub fn could_coerce( diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index 6f20f0dc893..b650afe9963 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -60,6 +60,10 @@ pub(crate) fn infer_expr(&mut self, tgt_expr: ExprId, expected: &Expectation) -> ty } + pub(crate) fn infer_expr_no_expect(&mut self, tgt_expr: ExprId) -> Ty { + self.infer_expr_inner(tgt_expr, &Expectation::None) + } + /// Infer type of expression with possibly implicit coerce to the expected type. /// Return the type after possible coercion. pub(super) fn infer_expr_coerce(&mut self, expr: ExprId, expected: &Expectation) -> Ty { @@ -99,17 +103,20 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { both_arms_diverge &= mem::replace(&mut self.diverges, Diverges::Maybe); let mut coerce = CoerceMany::new(expected.coercion_target_type(&mut self.table)); coerce.coerce(self, Some(then_branch), &then_ty); - let else_ty = match else_branch { - Some(else_branch) => self.infer_expr_inner(else_branch, expected), - None => TyBuilder::unit(), - }; + match else_branch { + Some(else_branch) => { + let else_ty = self.infer_expr_inner(else_branch, expected); + coerce.coerce(self, Some(else_branch), &else_ty); + } + None => { + coerce.coerce_forced_unit(self); + } + } both_arms_diverge &= self.diverges; - // FIXME: create a synthetic `else {}` so we have something to refer to here instead of None? - coerce.coerce(self, else_branch, &else_ty); self.diverges = condition_diverges | both_arms_diverge; - coerce.complete() + coerce.complete(self) } &Expr::Let { pat, expr } => { let input_ty = self.infer_expr(expr, &Expectation::none()); @@ -172,6 +179,8 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { let ret_ty = self.table.new_type_var(); let prev_diverges = mem::replace(&mut self.diverges, Diverges::Maybe); let prev_ret_ty = mem::replace(&mut self.return_ty, ret_ty.clone()); + let prev_ret_coercion = + mem::replace(&mut self.return_coercion, Some(CoerceMany::new(ret_ty.clone()))); let (_, inner_ty) = self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| { @@ -180,6 +189,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { self.diverges = prev_diverges; self.return_ty = prev_ret_ty; + self.return_coercion = prev_ret_coercion; // Use the first type parameter as the output type of future. // existential type AsyncBlockImplTrait: Future @@ -303,17 +313,21 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { self.infer_top_pat(*arg_pat, &arg_ty); } + // FIXME: lift these out into a struct let prev_diverges = mem::replace(&mut self.diverges, Diverges::Maybe); let prev_ret_ty = mem::replace(&mut self.return_ty, ret_ty.clone()); + let prev_ret_coercion = + mem::replace(&mut self.return_coercion, Some(CoerceMany::new(ret_ty.clone()))); let prev_resume_yield_tys = mem::replace(&mut self.resume_yield_tys, resume_yield_tys); self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| { - this.infer_expr_coerce(*body, &Expectation::has_type(ret_ty)); + this.infer_return(*body); }); self.diverges = prev_diverges; self.return_ty = prev_ret_ty; + self.return_coercion = prev_ret_coercion; self.resume_yield_tys = prev_resume_yield_tys; ty @@ -413,7 +427,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { self.diverges = matchee_diverges | all_arms_diverge; - coerce.complete() + coerce.complete(self) } Expr::Path(p) => { // FIXME this could be more efficient... @@ -431,7 +445,12 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { } Expr::Break { expr, label } => { let val_ty = if let Some(expr) = *expr { - self.infer_expr(expr, &Expectation::none()) + let opt_coerce_to = find_breakable(&mut self.breakables, label.as_ref()) + .map(|ctxt| ctxt.coerce.expected_ty()); + self.infer_expr_inner( + expr, + &Expectation::HasType(opt_coerce_to.unwrap_or_else(|| self.err_ty())), + ) } else { TyBuilder::unit() }; @@ -461,15 +480,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { } self.result.standard_types.never.clone() } - Expr::Return { expr } => { - if let Some(expr) = expr { - self.infer_expr_coerce(*expr, &Expectation::has_type(self.return_ty.clone())); - } else { - let unit = TyBuilder::unit(); - let _ = self.coerce(Some(tgt_expr), &unit, &self.return_ty.clone()); - } - self.result.standard_types.never.clone() - } + &Expr::Return { expr } => self.infer_expr_return(expr), Expr::Yield { expr } => { if let Some((resume_ty, yield_ty)) = self.resume_yield_tys.clone() { if let Some(expr) = expr { @@ -486,7 +497,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { } Expr::Yeet { expr } => { if let &Some(expr) = expr { - self.infer_expr_inner(expr, &Expectation::None); + self.infer_expr_no_expect(expr); } self.result.standard_types.never.clone() } @@ -614,7 +625,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { Expr::Cast { expr, type_ref } => { let cast_ty = self.make_ty(type_ref); // FIXME: propagate the "castable to" expectation - let _inner_ty = self.infer_expr_inner(*expr, &Expectation::None); + let _inner_ty = self.infer_expr_no_expect(*expr); // FIXME check the cast... cast_ty } @@ -810,53 +821,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { TyKind::Tuple(tys.len(), Substitution::from_iter(Interner, tys)).intern(Interner) } - Expr::Array(array) => { - let elem_ty = - match expected.to_option(&mut self.table).as_ref().map(|t| t.kind(Interner)) { - Some(TyKind::Array(st, _) | TyKind::Slice(st)) => st.clone(), - _ => self.table.new_type_var(), - }; - let mut coerce = CoerceMany::new(elem_ty.clone()); - - let expected = Expectation::has_type(elem_ty.clone()); - let len = match array { - Array::ElementList { elements, .. } => { - for &expr in elements.iter() { - let cur_elem_ty = self.infer_expr_inner(expr, &expected); - coerce.coerce(self, Some(expr), &cur_elem_ty); - } - consteval::usize_const( - self.db, - Some(elements.len() as u128), - self.resolver.krate(), - ) - } - &Array::Repeat { initializer, repeat } => { - self.infer_expr_coerce(initializer, &Expectation::has_type(elem_ty)); - self.infer_expr( - repeat, - &Expectation::HasType( - TyKind::Scalar(Scalar::Uint(UintTy::Usize)).intern(Interner), - ), - ); - - if let Some(g_def) = self.owner.as_generic_def_id() { - let generics = generics(self.db.upcast(), g_def); - consteval::eval_to_const( - repeat, - ParamLoweringMode::Placeholder, - self, - || generics, - DebruijnIndex::INNERMOST, - ) - } else { - consteval::usize_const(self.db, None, self.resolver.krate()) - } - } - }; - - TyKind::Array(coerce.complete(), len).intern(Interner) - } + Expr::Array(array) => self.infer_expr_array(array, expected), Expr::Literal(lit) => match lit { Literal::Bool(..) => self.result.standard_types.bool_.clone(), Literal::String(..) => { @@ -915,6 +880,97 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { ty } + fn infer_expr_array( + &mut self, + array: &Array, + expected: &Expectation, + ) -> chalk_ir::Ty { + let elem_ty = match expected.to_option(&mut self.table).as_ref().map(|t| t.kind(Interner)) { + Some(TyKind::Array(st, _) | TyKind::Slice(st)) => st.clone(), + _ => self.table.new_type_var(), + }; + + let krate = self.resolver.krate(); + + let expected = Expectation::has_type(elem_ty.clone()); + let (elem_ty, len) = match array { + Array::ElementList { elements, .. } if elements.is_empty() => { + (elem_ty, consteval::usize_const(self.db, Some(0), krate)) + } + Array::ElementList { elements, .. } => { + let mut coerce = CoerceMany::new(elem_ty.clone()); + for &expr in elements.iter() { + let cur_elem_ty = self.infer_expr_inner(expr, &expected); + coerce.coerce(self, Some(expr), &cur_elem_ty); + } + ( + coerce.complete(self), + consteval::usize_const(self.db, Some(elements.len() as u128), krate), + ) + } + &Array::Repeat { initializer, repeat } => { + self.infer_expr_coerce(initializer, &Expectation::has_type(elem_ty.clone())); + self.infer_expr( + repeat, + &Expectation::HasType( + TyKind::Scalar(Scalar::Uint(UintTy::Usize)).intern(Interner), + ), + ); + + ( + elem_ty, + if let Some(g_def) = self.owner.as_generic_def_id() { + let generics = generics(self.db.upcast(), g_def); + consteval::eval_to_const( + repeat, + ParamLoweringMode::Placeholder, + self, + || generics, + DebruijnIndex::INNERMOST, + ) + } else { + consteval::usize_const(self.db, None, krate) + }, + ) + } + }; + + TyKind::Array(elem_ty, len).intern(Interner) + } + + pub(super) fn infer_return(&mut self, expr: ExprId) { + let ret_ty = self + .return_coercion + .as_mut() + .expect("infer_return called outside function body") + .expected_ty(); + let return_expr_ty = self.infer_expr_inner(expr, &Expectation::HasType(ret_ty)); + let mut coerce_many = self.return_coercion.take().unwrap(); + coerce_many.coerce(self, Some(expr), &return_expr_ty); + self.return_coercion = Some(coerce_many); + } + + fn infer_expr_return(&mut self, expr: Option) -> Ty { + match self.return_coercion { + Some(_) => { + if let Some(expr) = expr { + self.infer_return(expr); + } else { + let mut coerce = self.return_coercion.take().unwrap(); + coerce.coerce_forced_unit(self); + self.return_coercion = Some(coerce); + } + } + None => { + // FIXME: diagnose return outside of function + if let Some(expr) = expr { + self.infer_expr_no_expect(expr); + } + } + } + self.result.standard_types.never.clone() + } + fn infer_expr_box(&mut self, inner_expr: ExprId, expected: &Expectation) -> Ty { if let Some(box_id) = self.resolve_boxed_box() { let table = &mut self.table; @@ -1666,6 +1722,6 @@ fn with_breakable_ctx( }); let res = cb(self); let ctx = self.breakables.pop().expect("breakable stack broken"); - (ctx.may_break.then(|| ctx.coerce.complete()), res) + (ctx.may_break.then(|| ctx.coerce.complete(self)), res) } } From 41f234df09440dcd9420cc752649c68135fc09ed Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 3 Mar 2023 17:28:57 +0100 Subject: [PATCH 2/2] Diagnose value breaks in incorrect breakables --- crates/hir-ty/src/infer.rs | 5 +- crates/hir-ty/src/infer/expr.rs | 70 +++++++++++-------- crates/hir/src/diagnostics.rs | 1 + crates/hir/src/lib.rs | 8 ++- .../src/handlers/break_outside_of_loop.rs | 23 +++++- 5 files changed, 70 insertions(+), 37 deletions(-) diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index d3e7f6e7dca..cca787befee 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -167,7 +167,8 @@ pub enum InferenceDiagnostic { NoSuchField { expr: ExprId }, PrivateField { expr: ExprId, field: FieldId }, PrivateAssocItem { id: ExprOrPatId, item: AssocItemId }, - BreakOutsideOfLoop { expr: ExprId, is_break: bool }, + // FIXME: Make this proper + BreakOutsideOfLoop { expr: ExprId, is_break: bool, bad_value_break: bool }, MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize }, } @@ -411,7 +412,7 @@ struct BreakableContext { /// Whether this context contains at least one break expression. may_break: bool, /// The coercion target of the context. - coerce: CoerceMany, + coerce: Option, /// The optional label of the context. label: Option, kind: BreakableKind, diff --git a/crates/hir-ty/src/infer/expr.rs b/crates/hir-ty/src/infer/expr.rs index b650afe9963..e64b020c7fb 100644 --- a/crates/hir-ty/src/infer/expr.rs +++ b/crates/hir-ty/src/infer/expr.rs @@ -133,7 +133,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { let break_ty = self.table.new_type_var(); let (breaks, ty) = self.with_breakable_ctx( BreakableKind::Block, - break_ty.clone(), + Some(break_ty.clone()), *label, |this| { this.infer_block( @@ -153,7 +153,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { } Expr::Unsafe { body } => self.infer_expr(*body, expected), Expr::Const { body } => { - self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| { + self.with_breakable_ctx(BreakableKind::Border, None, None, |this| { this.infer_expr(*body, expected) }) .1 @@ -169,7 +169,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { let ok_ty = self.resolve_associated_type(try_ty.clone(), self.resolve_ops_try_output()); - self.with_breakable_ctx(BreakableKind::Block, ok_ty.clone(), None, |this| { + self.with_breakable_ctx(BreakableKind::Block, Some(ok_ty.clone()), None, |this| { this.infer_expr(*body, &Expectation::has_type(ok_ty)); }); @@ -183,7 +183,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { mem::replace(&mut self.return_coercion, Some(CoerceMany::new(ret_ty.clone()))); let (_, inner_ty) = - self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| { + self.with_breakable_ctx(BreakableKind::Border, None, None, |this| { this.infer_expr_coerce(*body, &Expectation::has_type(ret_ty)) }); @@ -203,7 +203,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { // let ty = expected.coercion_target_type(&mut self.table); let ty = self.table.new_type_var(); let (breaks, ()) = - self.with_breakable_ctx(BreakableKind::Loop, ty, label, |this| { + self.with_breakable_ctx(BreakableKind::Loop, Some(ty), label, |this| { this.infer_expr(body, &Expectation::HasType(TyBuilder::unit())); }); @@ -216,7 +216,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { } } &Expr::While { condition, body, label } => { - self.with_breakable_ctx(BreakableKind::Loop, self.err_ty(), label, |this| { + self.with_breakable_ctx(BreakableKind::Loop, None, label, |this| { this.infer_expr( condition, &Expectation::HasType(this.result.standard_types.bool_.clone()), @@ -236,7 +236,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { self.resolve_associated_type(into_iter_ty, self.resolve_iterator_item()); self.infer_top_pat(pat, &pat_ty); - self.with_breakable_ctx(BreakableKind::Loop, self.err_ty(), label, |this| { + self.with_breakable_ctx(BreakableKind::Loop, None, label, |this| { this.infer_expr(body, &Expectation::HasType(TyBuilder::unit())); }); @@ -321,7 +321,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { let prev_resume_yield_tys = mem::replace(&mut self.resume_yield_tys, resume_yield_tys); - self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| { + self.with_breakable_ctx(BreakableKind::Border, None, None, |this| { this.infer_return(*body); }); @@ -439,42 +439,50 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { 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 = find_breakable(&mut self.breakables, label.as_ref()) - .map(|ctxt| ctxt.coerce.expected_ty()); - self.infer_expr_inner( - expr, - &Expectation::HasType(opt_coerce_to.unwrap_or_else(|| self.err_ty())), - ) + let opt_coerce_to = match find_breakable(&mut self.breakables, label.as_ref()) { + 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(), + }; + self.infer_expr_inner(expr, &Expectation::HasType(opt_coerce_to)) } else { TyBuilder::unit() }; match find_breakable(&mut self.breakables, label.as_ref()) { - Some(ctxt) => { - // avoiding the borrowck - let mut coerce = mem::replace( - &mut ctxt.coerce, - CoerceMany::new(expected.coercion_target_type(&mut self.table)), - ); + Some(ctxt) => match ctxt.coerce.take() { + Some(mut coerce) => { + coerce.coerce(self, *expr, &val_ty); - // FIXME: create a synthetic `()` during lowering so we have something to refer to here? - coerce.coerce(self, *expr, &val_ty); - - let ctxt = find_breakable(&mut self.breakables, label.as_ref()) - .expect("breakable stack changed during coercion"); - ctxt.coerce = coerce; - ctxt.may_break = true; - } + // Avoiding borrowck + let ctxt = find_breakable(&mut self.breakables, label.as_ref()) + .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, }); } } @@ -1712,16 +1720,16 @@ fn is_builtin_binop(&mut self, lhs: &Ty, rhs: &Ty, op: BinaryOp) -> bool { fn with_breakable_ctx( &mut self, kind: BreakableKind, - ty: Ty, + ty: Option, label: Option, 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: CoerceMany::new(ty), label } + BreakableContext { kind, may_break: false, coerce: ty.map(CoerceMany::new), label } }); let res = cb(self); let ctx = self.breakables.pop().expect("breakable stack broken"); - (ctx.may_break.then(|| ctx.coerce.complete(self)), res) + (if ctx.may_break { ctx.coerce.map(|ctx| ctx.complete(self)) } else { None }, res) } } diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 3b2591e8a10..bb7468d4660 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -140,6 +140,7 @@ pub struct PrivateField { pub struct BreakOutsideOfLoop { pub expr: InFile>, pub is_break: bool, + pub bad_value_break: bool, } #[derive(Debug)] diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 846ad4007ce..c9ae12d8d33 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1373,11 +1373,15 @@ pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec) { let field = source_map.field_syntax(*expr); acc.push(NoSuchField { field }.into()) } - &hir_ty::InferenceDiagnostic::BreakOutsideOfLoop { expr, is_break } => { + &hir_ty::InferenceDiagnostic::BreakOutsideOfLoop { + expr, + is_break, + bad_value_break, + } => { let expr = source_map .expr_syntax(expr) .expect("break outside of loop in synthetic syntax"); - acc.push(BreakOutsideOfLoop { expr, is_break }.into()) + acc.push(BreakOutsideOfLoop { expr, is_break, bad_value_break }.into()) } hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => { match source_map.expr_syntax(*call_expr) { 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 10e637979f2..114face2dca 100644 --- a/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs +++ b/crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs @@ -7,10 +7,15 @@ pub(crate) fn break_outside_of_loop( ctx: &DiagnosticsContext<'_>, d: &hir::BreakOutsideOfLoop, ) -> Diagnostic { - let construct = if d.is_break { "break" } else { "continue" }; + let message = if d.bad_value_break { + "can't break with a value in this position".to_owned() + } else { + let construct = if d.is_break { "break" } else { "continue" }; + format!("{construct} outside of loop") + }; Diagnostic::new( "break-outside-of-loop", - format!("{construct} outside of loop"), + message, ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, ) } @@ -132,6 +137,20 @@ fn foo() { //^^^^^^^^^^^ error: continue outside of loop } } +"#, + ); + } + + #[test] + fn value_break_in_for_loop() { + check_diagnostics( + r#" +fn test() { + for _ in [()] { + break 3; + // ^^^^^^^ error: can't break with a value in this position + } +} "#, ); }