Skip to content

Propagate coercions through match expressions. #19769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 18, 2014
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/librustc_typeck/check/_match.rs
Original file line number Diff line number Diff line change
@@ -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());
64 changes: 35 additions & 29 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
@@ -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);
4 changes: 2 additions & 2 deletions src/test/compile-fail/lub-match.rs
Original file line number Diff line number Diff line change
@@ -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)",
}
21 changes: 21 additions & 0 deletions src/test/run-pass/coerce-match.rs
Original file line number Diff line number Diff line change
@@ -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;
}