auto merge of #19769 : nick29581/rust/coerce-if, r=nikomatsakis

r? @nikomatsakis 

We discussed coercions for `if` and `match` expressions. `if` seems to work already, was there some specific behaviour which wasn't working?

4 files changed, 81 insertions, 38 deletions

diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs
index 7dcf0aa3e21..b88da5d9387 100644
--- a/src/librustc_typeck/check/_match.rs
+++ b/src/librustc_typeck/check/_match.rs
@@ -13,7 +13,8 @@ use middle::infer::{mod, resolve};
 use middle::pat_util::{PatIdMap, pat_id_map, pat_is_binding, pat_is_const};
 use middle::subst::{Subst, Substs};
 use middle::ty::{mod, Ty};
-use check::{check_expr, check_expr_has_type, demand, FnCtxt};
+use check::{check_expr, check_expr_has_type, check_expr_with_expectation};
+use check::{check_expr_coercable_to_type, demand, FnCtxt, Expectation};
 use check::{instantiate_path, structurally_resolved_type, valid_range_bounds};
 use require_same_types;
 use util::nodemap::FnvHashMap;
@@ -233,10 +234,11 @@ pub fn check_dereferencable<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
     }
 }
 
-pub fn check_match(fcx: &FnCtxt,
-                   expr: &ast::Expr,
-                   discrim: &ast::Expr,
-                   arms: &[ast::Arm]) {
+pub fn check_match<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
+                             expr: &ast::Expr,
+                             discrim: &ast::Expr,
+                             arms: &[ast::Arm],
+                             expected: Expectation<'tcx>) {
     let tcx = fcx.ccx.tcx;
 
     let discrim_ty = fcx.infcx().next_ty_var();
@@ -263,9 +265,23 @@ pub fn check_match(fcx: &FnCtxt,
     // on any empty type and is therefore unreachable; should the flow
     // of execution reach it, we will panic, so bottom is an appropriate
     // type in that case)
+    let expected = expected.adjust_for_branches(fcx);
     let result_ty = arms.iter().fold(fcx.infcx().next_diverging_ty_var(), |result_ty, arm| {
-        check_expr(fcx, &*arm.body);
-        let bty = fcx.node_ty(arm.body.id);
+        let bty = match expected {
+            // We don't coerce to `()` so that if the match expression is a
+            // statement it's branches can have any consistent type. That allows
+            // us to give better error messages (pointing to a usually better
+            // arm for inconsistent arms or to the whole match when a `()` type
+            // is required).
+            Expectation::ExpectHasType(ety) if ety != ty::mk_nil(fcx.tcx()) => {
+                check_expr_coercable_to_type(fcx, &*arm.body, ety);
+                ety
+            }
+            _ => {
+                check_expr_with_expectation(fcx, &*arm.body, expected);
+                fcx.node_ty(arm.body.id)
+            }
+        };
 
         if let Some(ref e) = arm.guard {
             check_expr_has_type(fcx, &**e, ty::mk_bool());
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
index f3b9e98ad48..ad63806df66 100644
--- a/src/librustc_typeck/check/mod.rs
+++ b/src/librustc_typeck/check/mod.rs
@@ -179,6 +179,38 @@ enum Expectation<'tcx> {
 
 impl<'tcx> Copy for Expectation<'tcx> {}
 
+impl<'tcx> Expectation<'tcx> {
+    // Disregard "castable to" expectations because they
+    // can lead us astray. Consider for example `if cond
+    // {22} else {c} as u8` -- if we propagate the
+    // "castable to u8" constraint to 22, it will pick the
+    // type 22u8, which is overly constrained (c might not
+    // be a u8). In effect, the problem is that the
+    // "castable to" expectation is not the tightest thing
+    // we can say, so we want to drop it in this case.
+    // The tightest thing we can say is "must unify with
+    // else branch". Note that in the case of a "has type"
+    // constraint, this limitation does not hold.
+
+    // If the expected type is just a type variable, then don't use
+    // an expected type. Otherwise, we might write parts of the type
+    // when checking the 'then' block which are incompatible with the
+    // 'else' branch.
+    fn adjust_for_branches<'a>(&self, fcx: &FnCtxt<'a, 'tcx>) -> Expectation<'tcx> {
+        match self.only_has_type() {
+            ExpectHasType(ety) => {
+                let ety = fcx.infcx().shallow_resolve(ety);
+                if !ty::type_is_ty_var(ety) {
+                    ExpectHasType(ety)
+                } else {
+                    NoExpectation
+                }
+            }
+            _ => NoExpectation
+        }
+    }
+}
+
 #[deriving(Copy, Clone)]
 pub struct UnsafetyState {
     pub def: ast::NodeId,
@@ -3047,7 +3079,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>,
     }
 
     // A generic function for checking the then and else in an if
-    // or if-check
+    // or if-else.
     fn check_then_else<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
                                  cond_expr: &ast::Expr,
                                  then_blk: &ast::Block,
@@ -3057,33 +3089,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>,
                                  expected: Expectation<'tcx>) {
         check_expr_has_type(fcx, cond_expr, ty::mk_bool());
 
-        // Disregard "castable to" expectations because they
-        // can lead us astray. Consider for example `if cond
-        // {22} else {c} as u8` -- if we propagate the
-        // "castable to u8" constraint to 22, it will pick the
-        // type 22u8, which is overly constrained (c might not
-        // be a u8). In effect, the problem is that the
-        // "castable to" expectation is not the tightest thing
-        // we can say, so we want to drop it in this case.
-        // The tightest thing we can say is "must unify with
-        // else branch". Note that in the case of a "has type"
-        // constraint, this limitation does not hold.
-
-        // If the expected type is just a type variable, then don't use
-        // an expected type. Otherwise, we might write parts of the type
-        // when checking the 'then' block which are incompatible with the
-        // 'else' branch.
-        let expected = match expected.only_has_type() {
-            ExpectHasType(ety) => {
-                let ety = fcx.infcx().shallow_resolve(ety);
-                if !ty::type_is_ty_var(ety) {
-                    ExpectHasType(ety)
-                } else {
-                    NoExpectation
-                }
-            }
-            _ => NoExpectation
-        };
+        let expected = expected.adjust_for_branches(fcx);
         check_block_with_expected(fcx, then_blk, expected);
         let then_ty = fcx.node_ty(then_blk.id);
 
@@ -3989,7 +3995,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>,
         }
       }
       ast::ExprMatch(ref discrim, ref arms, _) => {
-        _match::check_match(fcx, expr, &**discrim, arms.as_slice());
+        _match::check_match(fcx, expr, &**discrim, arms.as_slice(), expected);
       }
       ast::ExprClosure(_, opt_kind, ref decl, ref body) => {
           closure::check_expr_closure(fcx, expr, opt_kind, &**decl, &**body, expected);
diff --git a/src/test/compile-fail/lub-match.rs b/src/test/compile-fail/lub-match.rs
index df39e9e894e..1939df2877b 100644
--- a/src/test/compile-fail/lub-match.rs
+++ b/src/test/compile-fail/lub-match.rs
@@ -34,21 +34,21 @@ pub fn opt_str1<'a>(maybestr: &'a Option<String>) -> &'a str {
 
 pub fn opt_str2<'a>(maybestr: &'a Option<String>) -> &'static str {
     match *maybestr {
-    //~^ ERROR cannot infer an appropriate lifetime due to conflicting requirements
         None => "(none)",
         Some(ref s) => {
             let s: &'a str = s.as_slice();
             s
+            //~^ ERROR cannot infer an appropriate lifetime
         }
     }
 }
 
 pub fn opt_str3<'a>(maybestr: &'a Option<String>) -> &'static str {
     match *maybestr {
-    //~^ ERROR cannot infer an appropriate lifetime due to conflicting requirements
         Some(ref s) => {
             let s: &'a str = s.as_slice();
             s
+            //~^ ERROR cannot infer an appropriate lifetime
         }
         None => "(none)",
     }
diff --git a/src/test/run-pass/coerce-match.rs b/src/test/run-pass/coerce-match.rs
new file mode 100644
index 00000000000..d67664bb1be
--- /dev/null
+++ b/src/test/run-pass/coerce-match.rs
@@ -0,0 +1,21 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Check that coercions are propagated through match and if expressions.
+
+pub fn main() {
+    let _: Box<[int]> = if true { box [1i, 2, 3] } else { box [1i] };
+
+    let _: Box<[int]> = match true { true => box [1i, 2, 3], false => box [1i] };
+
+    // Check we don't get over-keen at propagating coercions in the case of casts.
+    let x = if true { 42 } else { 42u8 } as u16;
+    let x = match true { true => 42, false => 42u8 } as u16;
+}