Skip to content

Commit 6929013

Browse files
committed
fix subtle bug in NLL type checker
The bug was revealed by the behavior of the old-lub-glb-hr-noteq1.rs test. The old-lub-glb-hr-noteq2 test shows the current 'order dependent' behavior of coercions around higher-ranked functions, at least when running with `-Zborrowck=mir`. Also, run compare-mode=nll.
1 parent c88a76e commit 6929013

30 files changed

+472
-61
lines changed

src/librustc_infer/infer/nll_relate/mod.rs

+7-1
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,13 @@ where
522522
}
523523

524524
if a == b {
525-
return Ok(a);
525+
// Subtle: if a or b has a bound variable that we are lazilly
526+
// substituting, then even if a == b, it could be that the values we
527+
// will substitute for those bound variables are *not* the same, and
528+
// hence returning `Ok(a)` is incorrect.
529+
if !a.has_escaping_bound_vars() && !b.has_escaping_bound_vars() {
530+
return Ok(a);
531+
}
526532
}
527533

528534
match (&a.kind, &b.kind) {

src/librustc_mir/borrow_check/type_check/relate_tys.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub(super) fn relate_types<'tcx>(
2525
category: ConstraintCategory,
2626
borrowck_context: Option<&mut BorrowCheckContext<'_, 'tcx>>,
2727
) -> Fallible<()> {
28-
debug!("eq_types(a={:?}, b={:?}, locations={:?})", a, b, locations);
28+
debug!("relate_types(a={:?}, v={:?}, b={:?}, locations={:?})", a, v, b, locations);
2929
TypeRelating::new(
3030
infcx,
3131
NllTypeRelatingDelegate::new(infcx, borrowck_context, locations, category),

src/librustc_typeck/check/coercion.rs

+19-1
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
895895
{
896896
let prev_ty = self.resolve_vars_with_obligations(prev_ty);
897897
let new_ty = self.resolve_vars_with_obligations(new_ty);
898-
debug!("coercion::try_find_coercion_lub({:?}, {:?})", prev_ty, new_ty);
898+
debug!(
899+
"coercion::try_find_coercion_lub({:?}, {:?}, exprs={:?} exprs)",
900+
prev_ty,
901+
new_ty,
902+
exprs.len()
903+
);
899904

900905
// Special-case that coercion alone cannot handle:
901906
// Function items or non-capturing closures of differing IDs or InternalSubsts.
@@ -1001,6 +1006,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10011006
Ok(ok) => {
10021007
let (adjustments, target) = self.register_infer_ok_obligations(ok);
10031008
self.apply_adjustments(new, adjustments);
1009+
debug!(
1010+
"coercion::try_find_coercion_lub: was able to coerce from previous type {:?} to new type {:?}",
1011+
prev_ty, new_ty,
1012+
);
10041013
return Ok(target);
10051014
}
10061015
Err(e) => first_error = Some(e),
@@ -1031,6 +1040,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10311040
};
10321041

10331042
if !noop {
1043+
debug!(
1044+
"coercion::try_find_coercion_lub: older expression {:?} had adjustments, requiring LUB",
1045+
expr,
1046+
);
1047+
10341048
return self
10351049
.commit_if_ok(|_| self.at(cause, self.param_env).lub(prev_ty, new_ty))
10361050
.map(|ok| self.register_infer_ok_obligations(ok));
@@ -1048,6 +1062,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10481062
}
10491063
}
10501064
Ok(ok) => {
1065+
debug!(
1066+
"coercion::try_find_coercion_lub: was able to coerce previous type {:?} to new type {:?}",
1067+
prev_ty, new_ty,
1068+
);
10511069
let (adjustments, target) = self.register_infer_ok_obligations(ok);
10521070
for expr in exprs {
10531071
let expr = expr.as_coercion_site();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
error[E0271]: type mismatch resolving `for<'x> <UintStruct as TheTrait<&'x isize>>::A == &'x isize`
2+
--> $DIR/associated-types-eq-hr.rs:87:5
3+
|
4+
LL | fn foo<T>()
5+
| --- required by a bound in this
6+
LL | where
7+
LL | T: for<'x> TheTrait<&'x isize, A = &'x isize>,
8+
| ------------- required by this bound in `foo`
9+
...
10+
LL | foo::<UintStruct>();
11+
| ^^^^^^^^^^^^^^^^^ expected `isize`, found `usize`
12+
|
13+
= note: expected reference `&isize`
14+
found reference `&usize`
15+
16+
error[E0271]: type mismatch resolving `for<'x> <IntStruct as TheTrait<&'x isize>>::A == &'x usize`
17+
--> $DIR/associated-types-eq-hr.rs:91:5
18+
|
19+
LL | fn bar<T>()
20+
| --- required by a bound in this
21+
LL | where
22+
LL | T: for<'x> TheTrait<&'x isize, A = &'x usize>,
23+
| ------------- required by this bound in `bar`
24+
...
25+
LL | bar::<IntStruct>();
26+
| ^^^^^^^^^^^^^^^^ expected `usize`, found `isize`
27+
|
28+
= note: expected reference `&usize`
29+
found reference `&isize`
30+
31+
error: aborting due to 2 previous errors
32+
33+
For more information about this error, try `rustc --explain E0271`.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/higher-ranked-projection.rs:25:5
3+
|
4+
LL | foo(());
5+
| ^^^^^^^
6+
7+
error: aborting due to previous error
8+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/resume-arg-late-bound.rs:15:5
3+
|
4+
LL | test(gen);
5+
| ^^^^^^^^^
6+
7+
error: aborting due to previous error
8+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/hr-subtype.rs:45:13
3+
|
4+
LL | gimme::<$t1>(None::<$t2>);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
6+
...
7+
LL | / check! { bound_a_b_ret_a_vs_bound_a_ret_a: (for<'a,'b> fn(&'a u32, &'b u32) -> &'a u32,
8+
LL | | for<'a> fn(&'a u32, &'a u32) -> &'a u32) }
9+
| |_____________________________________________- in this macro invocation
10+
|
11+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
12+
13+
error: aborting due to previous error
14+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/hr-subtype.rs:45:13
3+
|
4+
LL | gimme::<$t1>(None::<$t2>);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
6+
...
7+
LL | / check! { bound_a_vs_free_x: (for<'a> fn(&'a u32),
8+
LL | | fn(&'x u32)) }
9+
| |______________- in this macro invocation
10+
|
11+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
12+
13+
error: aborting due to previous error
14+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/hr-subtype.rs:45:13
3+
|
4+
LL | gimme::<$t1>(None::<$t2>);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
6+
...
7+
LL | / check! { bound_inv_a_b_vs_bound_inv_a: (for<'a,'b> fn(Inv<'a>, Inv<'b>),
8+
LL | | for<'a> fn(Inv<'a>, Inv<'a>)) }
9+
| |__________________________________- in this macro invocation
10+
|
11+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
12+
13+
error: higher-ranked subtype error
14+
--> $DIR/hr-subtype.rs:45:13
15+
|
16+
LL | gimme::<$t1>(None::<$t2>);
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
18+
...
19+
LL | / check! { bound_inv_a_b_vs_bound_inv_a: (for<'a,'b> fn(Inv<'a>, Inv<'b>),
20+
LL | | for<'a> fn(Inv<'a>, Inv<'a>)) }
21+
| |__________________________________- in this macro invocation
22+
|
23+
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
24+
25+
error: aborting due to 2 previous errors
26+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/hrtb-conflate-regions.rs:27:10
3+
|
4+
LL | fn b() { want_foo2::<SomeStruct>(); }
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
6+
7+
error: higher-ranked subtype error
8+
--> $DIR/hrtb-conflate-regions.rs:27:10
9+
|
10+
LL | fn b() { want_foo2::<SomeStruct>(); }
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
12+
13+
error: aborting due to 2 previous errors
14+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/hrtb-exists-forall-fn.rs:17:12
3+
|
4+
LL | let _: for<'b> fn(&'b u32) = foo();
5+
| ^^^^^^^^^^^^^^^^^^^
6+
7+
error: aborting due to previous error
8+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/hrtb-exists-forall-trait-contravariant.rs:34:5
3+
|
4+
LL | foo::<()>();
5+
| ^^^^^^^^^^^
6+
7+
error: aborting due to previous error
8+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/hrtb-exists-forall-trait-invariant.rs:28:5
3+
|
4+
LL | foo::<()>();
5+
| ^^^^^^^^^^^
6+
7+
error: aborting due to previous error
8+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/hrtb-just-for-static.rs:24:5
3+
|
4+
LL | want_hrtb::<StaticInt>()
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^
6+
7+
error: lifetime may not live long enough
8+
--> $DIR/hrtb-just-for-static.rs:30:5
9+
|
10+
LL | fn give_some<'a>() {
11+
| -- lifetime `'a` defined here
12+
LL | want_hrtb::<&'a u32>()
13+
| ^^^^^^^^^^^^^^^^^^^^ requires that `'a` must outlive `'static`
14+
|
15+
= help: consider replacing `'a` with `'static`
16+
17+
error: higher-ranked subtype error
18+
--> $DIR/hrtb-just-for-static.rs:30:5
19+
|
20+
LL | want_hrtb::<&'a u32>()
21+
| ^^^^^^^^^^^^^^^^^^^^
22+
23+
error: aborting due to 3 previous errors
24+
+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/issue-46989.rs:38:5
3+
|
4+
LL | assert_foo::<fn(&i32)>();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^
6+
7+
error: aborting due to previous error
8+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/issue-40000.rs:6:9
3+
|
4+
LL | foo(bar);
5+
| ^^^
6+
7+
error: aborting due to previous error
8+
+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Test that we give a note when the old LUB/GLB algorithm would have
2+
// succeeded but the new code (which requires equality) gives an
3+
// error. However, now that we handle subtyping correctly, we no
4+
// longer get an error, because we recognize these two types as
5+
// equivalent!
6+
//
7+
// check-pass
8+
9+
fn foo(x: fn(&u8, &u8), y: for<'a> fn(&'a u8, &'a u8)) {
10+
// The two types above are actually equivalent. With the older
11+
// leak check, though, we didn't consider them as equivalent, and
12+
// hence we gave errors. But now we've fixed that.
13+
let z = match 22 {
14+
0 => x,
15+
_ => y,
16+
};
17+
}
18+
19+
fn foo_cast(x: fn(&u8, &u8), y: for<'a> fn(&'a u8, &'a u8)) {
20+
let z = match 22 {
21+
// No error with an explicit cast:
22+
0 => x as for<'a> fn(&'a u8, &'a u8),
23+
_ => y,
24+
};
25+
}
26+
27+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
error: higher-ranked subtype error
2+
--> $DIR/old-lub-glb-hr-noteq1.rs:11:14
3+
|
4+
LL | _ => y,
5+
| ^
6+
7+
error: aborting due to previous error
8+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Test taking the LUB of two function types that are not equatable but where one is more
2+
// general than the other. Test the case where the more general type (`x`) is the first
3+
// match arm specifically.
4+
5+
fn foo(x: for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8, y: for<'a> fn(&'a u8, &'a u8) -> &'a u8) {
6+
// The two types above are not equivalent. With the older LUB/GLB
7+
// algorithm, this may have worked (I don't remember), but now it
8+
// doesn't because we require equality.
9+
let z = match 22 {
10+
0 => x,
11+
_ => y, //~ ERROR `match` arms have incompatible types
12+
};
13+
}
14+
15+
fn foo_cast(x: for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8, y: for<'a> fn(&'a u8, &'a u8) -> &'a u8) {
16+
// But we can *upcast* explicitly the type of `x` and figure
17+
// things out:
18+
let z = match 22 {
19+
0 => x as for<'a> fn(&'a u8, &'a u8) -> &'a u8,
20+
_ => y,
21+
};
22+
}
23+
24+
fn main() {}

src/test/ui/lub-glb/old-lub-glb-hr.stderr renamed to src/test/ui/lub-glb/old-lub-glb-hr-noteq1.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0308]: `match` arms have incompatible types
2-
--> $DIR/old-lub-glb-hr.rs:40:14
2+
--> $DIR/old-lub-glb-hr-noteq1.rs:11:14
33
|
44
LL | let z = match 22 {
55
| _____________-
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Test taking the LUB of two function types that are not equatable but where
2+
// one is more general than the other. Test the case where the more general type
3+
// (`x`) is the second match arm specifically.
4+
//
5+
// Skip for compare-mode because the pure NLL checker accepts this test. (Note
6+
// that it still errors in old-lub-glb-hr-noteq1.rs). What happens is that, due
7+
// to the ordering of the match arms, we pick the correct "more general" fn
8+
// type, and we ignore the errors from the non-NLL type checker that requires
9+
// equality. The NLL type checker only requires a subtyping relationship, and
10+
// that holds.
11+
//
12+
// ignore-compare-mode-nll
13+
14+
fn foo(x: for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8, y: for<'a> fn(&'a u8, &'a u8) -> &'a u8) {
15+
// The two types above are not equivalent. With the older LUB/GLB
16+
// algorithm, this may have worked (I don't remember), but now it
17+
// doesn't because we require equality.
18+
let z = match 22 {
19+
0 => y,
20+
_ => x, //~ ERROR `match` arms have incompatible types
21+
};
22+
}
23+
24+
fn foo_cast(x: for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8, y: for<'a> fn(&'a u8, &'a u8) -> &'a u8) {
25+
// But we can *upcast* explicitly the type of `x` and figure
26+
// things out:
27+
let z = match 22 {
28+
0 => x as for<'a> fn(&'a u8, &'a u8) -> &'a u8,
29+
_ => y,
30+
};
31+
}
32+
33+
fn main() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
error[E0308]: `match` arms have incompatible types
2+
--> $DIR/old-lub-glb-hr-noteq2.rs:20:14
3+
|
4+
LL | let z = match 22 {
5+
| _____________-
6+
LL | | 0 => y,
7+
| | - this is found to be of type `for<'a> fn(&'a u8, &'a u8) -> &'a u8`
8+
LL | | _ => x,
9+
| | ^ one type is more general than the other
10+
LL | | };
11+
| |_____- `match` arms have incompatible types
12+
|
13+
= note: expected fn pointer `for<'a> fn(&'a u8, &'a u8) -> &'a u8`
14+
found fn pointer `for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8`
15+
16+
error: aborting due to previous error
17+
18+
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)