Merge #4505

4505: Infer return type of loops with value breaks r=flodiebold a=ruabmbua

Creates a type variable to represent the return value of the loop.
Uses `coerce_merge_branch` on each break with the previous value, to determine the actual return value of the loop.

Resolves: https://github.com/rust-analyzer/rust-analyzer/issues/4492 , https://github.com/rust-analyzer/rust-analyzer/issues/4512

Co-authored-by: Roland Ruckerbauer <[email protected]>

4 files changed, 106 insertions, 30 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)]
@@ -1998,26 +2015,6 @@ mod false_negatives {
     }
 
     #[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"
             enum Either {
@@ -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]
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..b28724f0e 100644
--- a/crates/ra_hir_ty/src/infer/expr.rs
+++ b/crates/ra_hir_ty/src/infer/expr.rs
@@ -93,22 +93,25 @@ 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: 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 {
-                    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 +123,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,17 +232,29 @@ impl<'a> InferenceContext<'a> {
             }
             Expr::Continue => Ty::simple(TypeCtor::Never),
             Expr::Break { expr } => {
-                if let Some(expr) = expr {
-                    // FIXME handle break with value
-                    self.infer_expr(*expr, &Expectation::none());
-                }
+                let val_ty = if let Some(expr) = expr {
+                    self.infer_expr(*expr, &Expectation::none())
+                } else {
+                    Ty::unit()
+                };
+
+                let last_ty = if let Some(ctxt) = self.breakables.last() {
+                    ctxt.break_ty.clone()
+                } else {
+                    Ty::Unknown
+                };
+
+                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,
                     });
                 }
+
                 Ty::simple(TypeCtor::Never)
             }
             Expr::Return { expr } => {
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<T> { Some(T), None }
+use Option::*;
+
+fn test() {
+    let x = loop {
+        if false {
+            break None;
+        }
+
+        break Some(true);
+    };
+}
+"#),
+        @r###"
+    60..169 '{     ...  }; }': ()
+    70..71 'x': Option<bool>
+    74..166 'loop {...     }': Option<bool>
+    79..166 '{     ...     }': ()
+    89..133 'if fal...     }': ()
+    92..97 'false': bool
+    98..133 '{     ...     }': ()
+    112..122 'break None': !
+    118..122 'None': Option<bool>
+    143..159 'break ...(true)': !
+    149..153 'Some': Some<bool>(bool) -> Option<bool>
+    149..159 'Some(true)': Option<bool>
+    154..158 'true': bool
+    "###
+    );
+}
+
+#[test]
+fn infer_loop_break_without_val() {
+    assert_snapshot!(
+        infer(r#"
+enum Option<T> { 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': !
+    "###
+    );
+}