From 0fe876925e59aad4765b415d9caaf262a6d43c4c Mon Sep 17 00:00:00 2001 From: Roland Ruckerbauer Date: Mon, 18 May 2020 23:39:10 +0200 Subject: Infer return type of loops with value breaks. --- crates/ra_hir_ty/src/infer.rs | 1 + crates/ra_hir_ty/src/infer/expr.rs | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/ra_hir_ty/src/infer.rs b/crates/ra_hir_ty/src/infer.rs index 2876cb141..957d6e0b5 100644 --- a/crates/ra_hir_ty/src/infer.rs +++ b/crates/ra_hir_ty/src/infer.rs @@ -218,6 +218,7 @@ struct InferenceContext<'a> { #[derive(Clone, Debug)] struct BreakableContext { pub may_break: bool, + pub break_ty: Ty, } impl<'a> InferenceContext<'a> { diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs index 0b67d216a..c7aa67fbe 100644 --- a/crates/ra_hir_ty/src/infer/expr.rs +++ b/crates/ra_hir_ty/src/infer/expr.rs @@ -93,7 +93,7 @@ impl<'a> InferenceContext<'a> { Ty::Unknown } Expr::Loop { body } => { - self.breakables.push(BreakableContext { may_break: false }); + self.breakables.push(BreakableContext { may_break: false, break_ty: Ty::Unknown }); self.infer_expr(*body, &Expectation::has_type(Ty::unit())); let ctxt = self.breakables.pop().expect("breakable stack broken"); @@ -102,13 +102,13 @@ impl<'a> InferenceContext<'a> { } // FIXME handle break with value if ctxt.may_break { - Ty::unit() + ctxt.break_ty } else { Ty::simple(TypeCtor::Never) } } Expr::While { condition, body } => { - self.breakables.push(BreakableContext { may_break: false }); + self.breakables.push(BreakableContext { may_break: false, break_ty: Ty::Unknown }); // while let is desugared to a match loop, so this is always simple while self.infer_expr(*condition, &Expectation::has_type(Ty::simple(TypeCtor::Bool))); self.infer_expr(*body, &Expectation::has_type(Ty::unit())); @@ -120,7 +120,7 @@ impl<'a> InferenceContext<'a> { Expr::For { iterable, body, pat } => { let iterable_ty = self.infer_expr(*iterable, &Expectation::none()); - self.breakables.push(BreakableContext { may_break: false }); + self.breakables.push(BreakableContext { may_break: false, break_ty: Ty::Unknown }); let pat_ty = self.resolve_associated_type(iterable_ty, self.resolve_into_iter_item()); @@ -229,12 +229,21 @@ impl<'a> InferenceContext<'a> { } Expr::Continue => Ty::simple(TypeCtor::Never), Expr::Break { expr } => { + let mut has_val_ty = None; + if let Some(expr) = expr { - // FIXME handle break with value - self.infer_expr(*expr, &Expectation::none()); + has_val_ty = Some(self.infer_expr(*expr, &Expectation::none())); } + if let Some(ctxt) = self.breakables.last_mut() { ctxt.may_break = true; + if let Some(val_ty) = has_val_ty { + if ctxt.break_ty == Ty::Unknown { + ctxt.break_ty = val_ty; + } else if ctxt.break_ty != val_ty { + // TODO: Unify partially matching type information (Option<{unknown}> + Option => Option) + } + } } else { self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop { expr: tgt_expr, -- cgit v1.2.3 From 6eaa669da0c7b3730a309db5e320126653b88997 Mon Sep 17 00:00:00 2001 From: Roland Ruckerbauer Date: Tue, 19 May 2020 21:03:59 +0200 Subject: loop return value inference: coerce_merge branches --- crates/ra_hir_ty/src/infer/expr.rs | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs index c7aa67fbe..83702ada0 100644 --- a/crates/ra_hir_ty/src/infer/expr.rs +++ b/crates/ra_hir_ty/src/infer/expr.rs @@ -93,14 +93,17 @@ impl<'a> InferenceContext<'a> { Ty::Unknown } Expr::Loop { body } => { - self.breakables.push(BreakableContext { may_break: false, break_ty: Ty::Unknown }); + self.breakables.push(BreakableContext { + may_break: false, + break_ty: self.table.new_type_var(), + }); self.infer_expr(*body, &Expectation::has_type(Ty::unit())); let ctxt = self.breakables.pop().expect("breakable stack broken"); if ctxt.may_break { self.diverges = Diverges::Maybe; } - // FIXME handle break with value + if ctxt.may_break { ctxt.break_ty } else { @@ -229,26 +232,31 @@ impl<'a> InferenceContext<'a> { } Expr::Continue => Ty::simple(TypeCtor::Never), Expr::Break { expr } => { - let mut has_val_ty = None; + let val_ty = if let Some(expr) = expr { + self.infer_expr(*expr, &Expectation::none()) + } else { + Ty::unit() + }; - if let Some(expr) = expr { - has_val_ty = Some(self.infer_expr(*expr, &Expectation::none())); - } + let mut has_brkctx = false; - if let Some(ctxt) = self.breakables.last_mut() { - ctxt.may_break = true; - if let Some(val_ty) = has_val_ty { - if ctxt.break_ty == Ty::Unknown { - ctxt.break_ty = val_ty; - } else if ctxt.break_ty != val_ty { - // TODO: Unify partially matching type information (Option<{unknown}> + Option => Option) - } - } + if self.breakables.last().is_some() { + has_brkctx = true; } else { self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop { expr: tgt_expr, }); } + + if has_brkctx { + let last_ty = self.breakables.last().expect("This is a bug").break_ty.clone(); + let merged_type = self.coerce_merge_branch(&last_ty, &val_ty); + + let ctxt = self.breakables.last_mut().expect("This is a bug"); + ctxt.may_break = true; + ctxt.break_ty = merged_type; + } + Ty::simple(TypeCtor::Never) } Expr::Return { expr } => { -- cgit v1.2.3 From 6e36ad3d910ebec5af5f4f208b0f98c613687c41 Mon Sep 17 00:00:00 2001 From: Roland Ruckerbauer Date: Tue, 19 May 2020 21:18:43 +0200 Subject: Move false negative expr_diverges_missing_arm() to working tests --- crates/ra_hir_ty/src/_match.rs | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/crates/ra_hir_ty/src/_match.rs b/crates/ra_hir_ty/src/_match.rs index 149f65042..3e6e1e333 100644 --- a/crates/ra_hir_ty/src/_match.rs +++ b/crates/ra_hir_ty/src/_match.rs @@ -1946,6 +1946,23 @@ mod tests { check_no_diagnostic(content); } + + #[test] + fn expr_diverges_missing_arm() { + let content = r" + enum Either { + A, + B, + } + fn test_fn() { + match loop {} { + Either::A => (), + } + } + "; + + check_no_diagnostic(content); + } } #[cfg(test)] @@ -1997,26 +2014,6 @@ mod false_negatives { check_no_diagnostic(content); } - #[test] - fn expr_diverges_missing_arm() { - let content = r" - enum Either { - A, - B, - } - fn test_fn() { - match loop {} { - Either::A => (), - } - } - "; - - // This is a false negative. - // Even though the match expression diverges, rustc fails - // to compile here since `Either::B` is missing. - check_no_diagnostic(content); - } - #[test] fn expr_loop_missing_arm() { let content = r" @@ -2035,7 +2032,7 @@ mod false_negatives { // We currently infer the type of `loop { break Foo::A }` to `!`, which // causes us to skip the diagnostic since `Either::A` doesn't type check // with `!`. - check_no_diagnostic(content); + check_diagnostic(content); } #[test] -- cgit v1.2.3 From da09f967469127576d9a87a7c143f754777a4f6b Mon Sep 17 00:00:00 2001 From: Roland Ruckerbauer Date: Tue, 19 May 2020 21:49:45 +0200 Subject: loop return value inference: add tests --- crates/ra_hir_ty/src/tests/simple.rs | 63 ++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/crates/ra_hir_ty/src/tests/simple.rs b/crates/ra_hir_ty/src/tests/simple.rs index 72122c070..fd2208af2 100644 --- a/crates/ra_hir_ty/src/tests/simple.rs +++ b/crates/ra_hir_ty/src/tests/simple.rs @@ -1860,3 +1860,66 @@ fn test() { "### ); } + +#[test] +fn infer_loop_break_with_val() { + assert_snapshot!( + infer(r#" +enum Option { Some(T), None } +use Option::*; + +fn test() { + let x = loop { + if false { + break None; + } + + break Some(true); + }; +} +"#), + @r###" + 60..169 '{ ... }; }': () + 70..71 'x': Option + 74..166 'loop {... }': Option + 79..166 '{ ... }': () + 89..133 'if fal... }': () + 92..97 'false': bool + 98..133 '{ ... }': () + 112..122 'break None': ! + 118..122 'None': Option + 143..159 'break ...(true)': ! + 149..153 'Some': Some(bool) -> Option + 149..159 'Some(true)': Option + 154..158 'true': bool + "### + ); +} + +#[test] +fn infer_loop_break_without_val() { + assert_snapshot!( + infer(r#" +enum Option { Some(T), None } +use Option::*; + +fn test() { + let x = loop { + if false { + break; + } + }; +} +"#), + @r###" + 60..137 '{ ... }; }': () + 70..71 'x': () + 74..134 'loop {... }': () + 79..134 '{ ... }': () + 89..128 'if fal... }': () + 92..97 'false': bool + 98..128 '{ ... }': () + 112..117 'break': ! + "### + ); +} -- cgit v1.2.3 From 45021cae551826727c32c7499c68ca48d046890f Mon Sep 17 00:00:00 2001 From: Roland Ruckerbauer Date: Tue, 19 May 2020 22:52:15 +0200 Subject: Apply suggestion of @flodiebold: Get rid of multiple unwraps --- crates/ra_hir_ty/src/infer/expr.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/ra_hir_ty/src/infer/expr.rs b/crates/ra_hir_ty/src/infer/expr.rs index 83702ada0..b28724f0e 100644 --- a/crates/ra_hir_ty/src/infer/expr.rs +++ b/crates/ra_hir_ty/src/infer/expr.rs @@ -238,25 +238,23 @@ impl<'a> InferenceContext<'a> { Ty::unit() }; - let mut has_brkctx = false; + let last_ty = if let Some(ctxt) = self.breakables.last() { + ctxt.break_ty.clone() + } else { + Ty::Unknown + }; - if self.breakables.last().is_some() { - has_brkctx = true; + let merged_type = self.coerce_merge_branch(&last_ty, &val_ty); + + if let Some(ctxt) = self.breakables.last_mut() { + ctxt.break_ty = merged_type; + ctxt.may_break = true; } else { self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop { expr: tgt_expr, }); } - if has_brkctx { - let last_ty = self.breakables.last().expect("This is a bug").break_ty.clone(); - let merged_type = self.coerce_merge_branch(&last_ty, &val_ty); - - let ctxt = self.breakables.last_mut().expect("This is a bug"); - ctxt.may_break = true; - ctxt.break_ty = merged_type; - } - Ty::simple(TypeCtor::Never) } Expr::Return { expr } => { -- cgit v1.2.3