From c7ef9c1edf5d659f3d44cc19509f644125ea03a1 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 10 Jan 2015 21:44:14 -0500 Subject: [PATCH] Fix two type inference failures uncovered by japaric corresponding to UFCS form. In both cases the problems came about because we were failing to process pending trait obligations. So change code to process pending trait obligations before coercions to ensure maximum type information is available (and also adjust shift to do something similar). Fixes #21245. --- src/librustc_typeck/check/demand.rs | 3 +- src/librustc_typeck/check/mod.rs | 59 ++++++++++++------ .../into-iterator-type-inference-shift.rs | 41 ++++++++++++ src/test/run-pass/issue-21245.rs | 62 +++++++++++++++++++ 4 files changed, 145 insertions(+), 20 deletions(-) create mode 100644 src/test/run-pass/into-iterator-type-inference-shift.rs create mode 100644 src/test/run-pass/issue-21245.rs diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index a51e89c1669d..8188835718cd 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -60,7 +60,8 @@ pub fn coerce<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, sp: Span, debug!("demand::coerce(expected = {}, expr_ty = {})", expected.repr(fcx.ccx.tcx), expr_ty.repr(fcx.ccx.tcx)); - let expected = fcx.infcx().resolve_type_vars_if_possible(&expected); + let expr_ty = fcx.resolve_type_vars_if_possible(expr_ty); + let expected = fcx.resolve_type_vars_if_possible(expected); match fcx.mk_assignty(expr, expr_ty, expected) { Ok(()) => { /* ok */ } Err(ref err) => { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 5f8ae09b5bd6..4e23106c1c50 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1231,6 +1231,36 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.ccx.tcx.sess.err_count() - self.err_count_on_creation } + /// Resolves type variables in `ty` if possible. Unlike the infcx + /// version, this version will also select obligations if it seems + /// useful, in an effort to get more type information. + fn resolve_type_vars_if_possible(&self, mut ty: Ty<'tcx>) -> Ty<'tcx> { + // No ty::infer()? Nothing needs doing. + if !ty::type_has_ty_infer(ty) { + return ty; + } + + // If `ty` is a type variable, see whether we already know what it is. + ty = self.infcx().resolve_type_vars_if_possible(&ty); + if !ty::type_has_ty_infer(ty) { + return ty; + } + + // If not, try resolving any new fcx obligations that have cropped up. + vtable::select_new_fcx_obligations(self); + ty = self.infcx().resolve_type_vars_if_possible(&ty); + if !ty::type_has_ty_infer(ty) { + return ty; + } + + // If not, try resolving *all* pending obligations as much as + // possible. This can help substantially when there are + // indirect dependencies that don't seem worth tracking + // precisely. + vtable::select_fcx_obligations_where_possible(self); + self.infcx().resolve_type_vars_if_possible(&ty) + } + /// Resolves all type variables in `t` and then, if any were left /// unresolved, substitutes an error type. This is used after the /// main checking when doing a second pass before writeback. The @@ -2321,9 +2351,9 @@ fn check_argument_types<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, let check_blocks = *check_blocks; debug!("check_blocks={}", check_blocks); - // More awful hacks: before we check the blocks, try to do - // an "opportunistic" vtable resolution of any trait - // bounds on the call. + // More awful hacks: before we check argument types, try to do + // an "opportunistic" vtable resolution of any trait bounds on + // the call. This helps coercions. if check_blocks { vtable::select_new_fcx_obligations(fcx); } @@ -2863,7 +2893,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, // Shift is a special case: rhs must be uint, no matter what lhs is check_expr(fcx, &**rhs); let rhs_ty = fcx.expr_ty(&**rhs); - let rhs_ty = fcx.infcx().resolve_type_vars_if_possible(&rhs_ty); + let rhs_ty = structurally_resolved_type(fcx, rhs.span, rhs_ty); if ty::type_is_integral(rhs_ty) { fcx.write_ty(expr.id, lhs_t); } else { @@ -5115,21 +5145,12 @@ pub fn instantiate_path<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, // Resolves `typ` by a single level if `typ` is a type variable. If no // resolution is possible, then an error is reported. -pub fn structurally_resolved_type<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, sp: Span, - mut ty: Ty<'tcx>) -> Ty<'tcx> { - // If `ty` is a type variable, see whether we already know what it is. - ty = fcx.infcx().shallow_resolve(ty); - - // If not, try resolve pending fcx obligations. Those can shed light. - // - // FIXME(#18391) -- This current strategy can lead to bad performance in - // extreme cases. We probably ought to smarter in general about - // only resolving when we need help and only resolving obligations - // will actually help. - if ty::type_is_ty_var(ty) { - vtable::select_fcx_obligations_where_possible(fcx); - ty = fcx.infcx().shallow_resolve(ty); - } +pub fn structurally_resolved_type<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, + sp: Span, + ty: Ty<'tcx>) + -> Ty<'tcx> +{ + let mut ty = fcx.resolve_type_vars_if_possible(ty); // If not, error. if ty::type_is_ty_var(ty) { diff --git a/src/test/run-pass/into-iterator-type-inference-shift.rs b/src/test/run-pass/into-iterator-type-inference-shift.rs new file mode 100644 index 000000000000..26a0abc76aee --- /dev/null +++ b/src/test/run-pass/into-iterator-type-inference-shift.rs @@ -0,0 +1,41 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Regression test for type inference failure around shifting. In this +// case, the iteration yields an int, but we hadn't run the full type +// propagation yet, and so we just saw a type variable, yielding an +// error. + +use std::u8; + +trait IntoIterator { + type Iter: Iterator; + + fn into_iter(self) -> Self::Iter; +} + +impl IntoIterator for I where I: Iterator { + type Iter = I; + + fn into_iter(self) -> I { + self + } +} + +fn desugared_for_loop_bad(byte: u8) -> u8 { + let mut result = 0; + let mut x = IntoIterator::into_iter(range(0, u8::BITS)); + let mut y = Iterator::next(&mut x); + let mut z = y.unwrap(); + byte >> z; + 1 +} + +fn main() {} diff --git a/src/test/run-pass/issue-21245.rs b/src/test/run-pass/issue-21245.rs new file mode 100644 index 000000000000..1ed939cbacaf --- /dev/null +++ b/src/test/run-pass/issue-21245.rs @@ -0,0 +1,62 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Regression test for issue #21245. Check that we are able to infer +// the types in these examples correctly. It used to be that +// insufficient type propagation caused the type of the iterator to be +// incorrectly unified with the `*const` type to which it is coerced. + +use std::ptr; + +trait IntoIterator { + type Iter: Iterator; + + fn into_iter(self) -> Self::Iter; +} + +impl IntoIterator for I where I: Iterator { + type Iter = I; + + fn into_iter(self) -> I { + self + } +} + +fn desugared_for_loop_bad(v: Vec) { + match IntoIterator::into_iter(v.iter()) { + mut iter => { + loop { + match ::std::iter::Iterator::next(&mut iter) { + ::std::option::Option::Some(x) => { + unsafe { ptr::read(x); } + }, + ::std::option::Option::None => break + } + } + } + } +} + +fn desugared_for_loop_good(v: Vec) { + match v.iter().into_iter() { // NB method call instead of UFCS + mut iter => { + loop { + match ::std::iter::Iterator::next(&mut iter) { + ::std::option::Option::Some(x) => { + unsafe { ptr::read(x); } + }, + ::std::option::Option::None => break + } + } + } + } +} + +fn main() {}