Skip to content

Commit c7ef9c1

Browse files
committed
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.
1 parent 60db57e commit c7ef9c1

File tree

4 files changed

+145
-20
lines changed

4 files changed

+145
-20
lines changed

src/librustc_typeck/check/demand.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ pub fn coerce<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, sp: Span,
6060
debug!("demand::coerce(expected = {}, expr_ty = {})",
6161
expected.repr(fcx.ccx.tcx),
6262
expr_ty.repr(fcx.ccx.tcx));
63-
let expected = fcx.infcx().resolve_type_vars_if_possible(&expected);
63+
let expr_ty = fcx.resolve_type_vars_if_possible(expr_ty);
64+
let expected = fcx.resolve_type_vars_if_possible(expected);
6465
match fcx.mk_assignty(expr, expr_ty, expected) {
6566
Ok(()) => { /* ok */ }
6667
Err(ref err) => {

src/librustc_typeck/check/mod.rs

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,6 +1231,36 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12311231
self.ccx.tcx.sess.err_count() - self.err_count_on_creation
12321232
}
12331233

1234+
/// Resolves type variables in `ty` if possible. Unlike the infcx
1235+
/// version, this version will also select obligations if it seems
1236+
/// useful, in an effort to get more type information.
1237+
fn resolve_type_vars_if_possible(&self, mut ty: Ty<'tcx>) -> Ty<'tcx> {
1238+
// No ty::infer()? Nothing needs doing.
1239+
if !ty::type_has_ty_infer(ty) {
1240+
return ty;
1241+
}
1242+
1243+
// If `ty` is a type variable, see whether we already know what it is.
1244+
ty = self.infcx().resolve_type_vars_if_possible(&ty);
1245+
if !ty::type_has_ty_infer(ty) {
1246+
return ty;
1247+
}
1248+
1249+
// If not, try resolving any new fcx obligations that have cropped up.
1250+
vtable::select_new_fcx_obligations(self);
1251+
ty = self.infcx().resolve_type_vars_if_possible(&ty);
1252+
if !ty::type_has_ty_infer(ty) {
1253+
return ty;
1254+
}
1255+
1256+
// If not, try resolving *all* pending obligations as much as
1257+
// possible. This can help substantially when there are
1258+
// indirect dependencies that don't seem worth tracking
1259+
// precisely.
1260+
vtable::select_fcx_obligations_where_possible(self);
1261+
self.infcx().resolve_type_vars_if_possible(&ty)
1262+
}
1263+
12341264
/// Resolves all type variables in `t` and then, if any were left
12351265
/// unresolved, substitutes an error type. This is used after the
12361266
/// main checking when doing a second pass before writeback. The
@@ -2321,9 +2351,9 @@ fn check_argument_types<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
23212351
let check_blocks = *check_blocks;
23222352
debug!("check_blocks={}", check_blocks);
23232353

2324-
// More awful hacks: before we check the blocks, try to do
2325-
// an "opportunistic" vtable resolution of any trait
2326-
// bounds on the call.
2354+
// More awful hacks: before we check argument types, try to do
2355+
// an "opportunistic" vtable resolution of any trait bounds on
2356+
// the call. This helps coercions.
23272357
if check_blocks {
23282358
vtable::select_new_fcx_obligations(fcx);
23292359
}
@@ -2863,7 +2893,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>,
28632893
// Shift is a special case: rhs must be uint, no matter what lhs is
28642894
check_expr(fcx, &**rhs);
28652895
let rhs_ty = fcx.expr_ty(&**rhs);
2866-
let rhs_ty = fcx.infcx().resolve_type_vars_if_possible(&rhs_ty);
2896+
let rhs_ty = structurally_resolved_type(fcx, rhs.span, rhs_ty);
28672897
if ty::type_is_integral(rhs_ty) {
28682898
fcx.write_ty(expr.id, lhs_t);
28692899
} else {
@@ -5115,21 +5145,12 @@ pub fn instantiate_path<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
51155145

51165146
// Resolves `typ` by a single level if `typ` is a type variable. If no
51175147
// resolution is possible, then an error is reported.
5118-
pub fn structurally_resolved_type<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, sp: Span,
5119-
mut ty: Ty<'tcx>) -> Ty<'tcx> {
5120-
// If `ty` is a type variable, see whether we already know what it is.
5121-
ty = fcx.infcx().shallow_resolve(ty);
5122-
5123-
// If not, try resolve pending fcx obligations. Those can shed light.
5124-
//
5125-
// FIXME(#18391) -- This current strategy can lead to bad performance in
5126-
// extreme cases. We probably ought to smarter in general about
5127-
// only resolving when we need help and only resolving obligations
5128-
// will actually help.
5129-
if ty::type_is_ty_var(ty) {
5130-
vtable::select_fcx_obligations_where_possible(fcx);
5131-
ty = fcx.infcx().shallow_resolve(ty);
5132-
}
5148+
pub fn structurally_resolved_type<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
5149+
sp: Span,
5150+
ty: Ty<'tcx>)
5151+
-> Ty<'tcx>
5152+
{
5153+
let mut ty = fcx.resolve_type_vars_if_possible(ty);
51335154

51345155
// If not, error.
51355156
if ty::type_is_ty_var(ty) {
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Regression test for type inference failure around shifting. In this
12+
// case, the iteration yields an int, but we hadn't run the full type
13+
// propagation yet, and so we just saw a type variable, yielding an
14+
// error.
15+
16+
use std::u8;
17+
18+
trait IntoIterator {
19+
type Iter: Iterator;
20+
21+
fn into_iter(self) -> Self::Iter;
22+
}
23+
24+
impl<I> IntoIterator for I where I: Iterator {
25+
type Iter = I;
26+
27+
fn into_iter(self) -> I {
28+
self
29+
}
30+
}
31+
32+
fn desugared_for_loop_bad(byte: u8) -> u8 {
33+
let mut result = 0;
34+
let mut x = IntoIterator::into_iter(range(0, u8::BITS));
35+
let mut y = Iterator::next(&mut x);
36+
let mut z = y.unwrap();
37+
byte >> z;
38+
1
39+
}
40+
41+
fn main() {}

src/test/run-pass/issue-21245.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Regression test for issue #21245. Check that we are able to infer
12+
// the types in these examples correctly. It used to be that
13+
// insufficient type propagation caused the type of the iterator to be
14+
// incorrectly unified with the `*const` type to which it is coerced.
15+
16+
use std::ptr;
17+
18+
trait IntoIterator {
19+
type Iter: Iterator;
20+
21+
fn into_iter(self) -> Self::Iter;
22+
}
23+
24+
impl<I> IntoIterator for I where I: Iterator {
25+
type Iter = I;
26+
27+
fn into_iter(self) -> I {
28+
self
29+
}
30+
}
31+
32+
fn desugared_for_loop_bad<T>(v: Vec<T>) {
33+
match IntoIterator::into_iter(v.iter()) {
34+
mut iter => {
35+
loop {
36+
match ::std::iter::Iterator::next(&mut iter) {
37+
::std::option::Option::Some(x) => {
38+
unsafe { ptr::read(x); }
39+
},
40+
::std::option::Option::None => break
41+
}
42+
}
43+
}
44+
}
45+
}
46+
47+
fn desugared_for_loop_good<T>(v: Vec<T>) {
48+
match v.iter().into_iter() { // NB method call instead of UFCS
49+
mut iter => {
50+
loop {
51+
match ::std::iter::Iterator::next(&mut iter) {
52+
::std::option::Option::Some(x) => {
53+
unsafe { ptr::read(x); }
54+
},
55+
::std::option::Option::None => break
56+
}
57+
}
58+
}
59+
}
60+
}
61+
62+
fn main() {}

0 commit comments

Comments
 (0)