Navigation Menu

Skip to content

Commit

Permalink
fix subtle bug in NLL type checker
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nikomatsakis committed Jun 22, 2020
1 parent c88a76e commit 6929013
Show file tree
Hide file tree
Showing 30 changed files with 472 additions and 61 deletions.
8 changes: 7 additions & 1 deletion src/librustc_infer/infer/nll_relate/mod.rs
Expand Up @@ -522,7 +522,13 @@ where
}

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

match (&a.kind, &b.kind) {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/type_check/relate_tys.rs
Expand Up @@ -25,7 +25,7 @@ pub(super) fn relate_types<'tcx>(
category: ConstraintCategory,
borrowck_context: Option<&mut BorrowCheckContext<'_, 'tcx>>,
) -> Fallible<()> {
debug!("eq_types(a={:?}, b={:?}, locations={:?})", a, b, locations);
debug!("relate_types(a={:?}, v={:?}, b={:?}, locations={:?})", a, v, b, locations);
TypeRelating::new(
infcx,
NllTypeRelatingDelegate::new(infcx, borrowck_context, locations, category),
Expand Down
20 changes: 19 additions & 1 deletion src/librustc_typeck/check/coercion.rs
Expand Up @@ -895,7 +895,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
{
let prev_ty = self.resolve_vars_with_obligations(prev_ty);
let new_ty = self.resolve_vars_with_obligations(new_ty);
debug!("coercion::try_find_coercion_lub({:?}, {:?})", prev_ty, new_ty);
debug!(
"coercion::try_find_coercion_lub({:?}, {:?}, exprs={:?} exprs)",
prev_ty,
new_ty,
exprs.len()
);

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

if !noop {
debug!(
"coercion::try_find_coercion_lub: older expression {:?} had adjustments, requiring LUB",
expr,
);

return self
.commit_if_ok(|_| self.at(cause, self.param_env).lub(prev_ty, new_ty))
.map(|ok| self.register_infer_ok_obligations(ok));
Expand All @@ -1048,6 +1062,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
Ok(ok) => {
debug!(
"coercion::try_find_coercion_lub: was able to coerce previous type {:?} to new type {:?}",
prev_ty, new_ty,
);
let (adjustments, target) = self.register_infer_ok_obligations(ok);
for expr in exprs {
let expr = expr.as_coercion_site();
Expand Down
33 changes: 33 additions & 0 deletions src/test/ui/associated-types/associated-types-eq-hr.nll.stderr
@@ -0,0 +1,33 @@
error[E0271]: type mismatch resolving `for<'x> <UintStruct as TheTrait<&'x isize>>::A == &'x isize`
--> $DIR/associated-types-eq-hr.rs:87:5
|
LL | fn foo<T>()
| --- required by a bound in this
LL | where
LL | T: for<'x> TheTrait<&'x isize, A = &'x isize>,
| ------------- required by this bound in `foo`
...
LL | foo::<UintStruct>();
| ^^^^^^^^^^^^^^^^^ expected `isize`, found `usize`
|
= note: expected reference `&isize`
found reference `&usize`

error[E0271]: type mismatch resolving `for<'x> <IntStruct as TheTrait<&'x isize>>::A == &'x usize`
--> $DIR/associated-types-eq-hr.rs:91:5
|
LL | fn bar<T>()
| --- required by a bound in this
LL | where
LL | T: for<'x> TheTrait<&'x isize, A = &'x usize>,
| ------------- required by this bound in `bar`
...
LL | bar::<IntStruct>();
| ^^^^^^^^^^^^^^^^ expected `usize`, found `isize`
|
= note: expected reference `&usize`
found reference `&isize`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0271`.
@@ -0,0 +1,8 @@
error: higher-ranked subtype error
--> $DIR/higher-ranked-projection.rs:25:5
|
LL | foo(());
| ^^^^^^^

error: aborting due to previous error

8 changes: 8 additions & 0 deletions src/test/ui/generator/resume-arg-late-bound.nll.stderr
@@ -0,0 +1,8 @@
error: higher-ranked subtype error
--> $DIR/resume-arg-late-bound.rs:15:5
|
LL | test(gen);
| ^^^^^^^^^

error: aborting due to previous error

@@ -0,0 +1,14 @@
error: higher-ranked subtype error
--> $DIR/hr-subtype.rs:45:13
|
LL | gimme::<$t1>(None::<$t2>);
| ^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | / check! { bound_a_b_ret_a_vs_bound_a_ret_a: (for<'a,'b> fn(&'a u32, &'b u32) -> &'a u32,
LL | | for<'a> fn(&'a u32, &'a u32) -> &'a u32) }
| |_____________________________________________- in this macro invocation
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

14 changes: 14 additions & 0 deletions src/test/ui/hr-subtype/hr-subtype.bound_a_vs_free_x.nll.stderr
@@ -0,0 +1,14 @@
error: higher-ranked subtype error
--> $DIR/hr-subtype.rs:45:13
|
LL | gimme::<$t1>(None::<$t2>);
| ^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | / check! { bound_a_vs_free_x: (for<'a> fn(&'a u32),
LL | | fn(&'x u32)) }
| |______________- in this macro invocation
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

@@ -0,0 +1,26 @@
error: higher-ranked subtype error
--> $DIR/hr-subtype.rs:45:13
|
LL | gimme::<$t1>(None::<$t2>);
| ^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | / check! { bound_inv_a_b_vs_bound_inv_a: (for<'a,'b> fn(Inv<'a>, Inv<'b>),
LL | | for<'a> fn(Inv<'a>, Inv<'a>)) }
| |__________________________________- in this macro invocation
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: higher-ranked subtype error
--> $DIR/hr-subtype.rs:45:13
|
LL | gimme::<$t1>(None::<$t2>);
| ^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | / check! { bound_inv_a_b_vs_bound_inv_a: (for<'a,'b> fn(Inv<'a>, Inv<'b>),
LL | | for<'a> fn(Inv<'a>, Inv<'a>)) }
| |__________________________________- in this macro invocation
|
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors

14 changes: 14 additions & 0 deletions src/test/ui/hrtb/hrtb-conflate-regions.nll.stderr
@@ -0,0 +1,14 @@
error: higher-ranked subtype error
--> $DIR/hrtb-conflate-regions.rs:27:10
|
LL | fn b() { want_foo2::<SomeStruct>(); }
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: higher-ranked subtype error
--> $DIR/hrtb-conflate-regions.rs:27:10
|
LL | fn b() { want_foo2::<SomeStruct>(); }
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors

8 changes: 8 additions & 0 deletions src/test/ui/hrtb/hrtb-exists-forall-fn.nll.stderr
@@ -0,0 +1,8 @@
error: higher-ranked subtype error
--> $DIR/hrtb-exists-forall-fn.rs:17:12
|
LL | let _: for<'b> fn(&'b u32) = foo();
| ^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

@@ -0,0 +1,8 @@
error: higher-ranked subtype error
--> $DIR/hrtb-exists-forall-trait-contravariant.rs:34:5
|
LL | foo::<()>();
| ^^^^^^^^^^^

error: aborting due to previous error

@@ -0,0 +1,8 @@
error: higher-ranked subtype error
--> $DIR/hrtb-exists-forall-trait-invariant.rs:28:5
|
LL | foo::<()>();
| ^^^^^^^^^^^

error: aborting due to previous error

24 changes: 24 additions & 0 deletions src/test/ui/hrtb/hrtb-just-for-static.nll.stderr
@@ -0,0 +1,24 @@
error: higher-ranked subtype error
--> $DIR/hrtb-just-for-static.rs:24:5
|
LL | want_hrtb::<StaticInt>()
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: lifetime may not live long enough
--> $DIR/hrtb-just-for-static.rs:30:5
|
LL | fn give_some<'a>() {
| -- lifetime `'a` defined here
LL | want_hrtb::<&'a u32>()
| ^^^^^^^^^^^^^^^^^^^^ requires that `'a` must outlive `'static`
|
= help: consider replacing `'a` with `'static`

error: higher-ranked subtype error
--> $DIR/hrtb-just-for-static.rs:30:5
|
LL | want_hrtb::<&'a u32>()
| ^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors

8 changes: 8 additions & 0 deletions src/test/ui/hrtb/issue-46989.nll.stderr
@@ -0,0 +1,8 @@
error: higher-ranked subtype error
--> $DIR/issue-46989.rs:38:5
|
LL | assert_foo::<fn(&i32)>();
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

8 changes: 8 additions & 0 deletions src/test/ui/issues/issue-40000.nll.stderr
@@ -0,0 +1,8 @@
error: higher-ranked subtype error
--> $DIR/issue-40000.rs:6:9
|
LL | foo(bar);
| ^^^

error: aborting due to previous error

27 changes: 27 additions & 0 deletions src/test/ui/lub-glb/old-lub-glb-hr-eq.rs
@@ -0,0 +1,27 @@
// Test that we give a note when the old LUB/GLB algorithm would have
// succeeded but the new code (which requires equality) gives an
// error. However, now that we handle subtyping correctly, we no
// longer get an error, because we recognize these two types as
// equivalent!
//
// check-pass

fn foo(x: fn(&u8, &u8), y: for<'a> fn(&'a u8, &'a u8)) {
// The two types above are actually equivalent. With the older
// leak check, though, we didn't consider them as equivalent, and
// hence we gave errors. But now we've fixed that.
let z = match 22 {
0 => x,
_ => y,
};
}

fn foo_cast(x: fn(&u8, &u8), y: for<'a> fn(&'a u8, &'a u8)) {
let z = match 22 {
// No error with an explicit cast:
0 => x as for<'a> fn(&'a u8, &'a u8),
_ => y,
};
}

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/lub-glb/old-lub-glb-hr-noteq1.nll.stderr
@@ -0,0 +1,8 @@
error: higher-ranked subtype error
--> $DIR/old-lub-glb-hr-noteq1.rs:11:14
|
LL | _ => y,
| ^

error: aborting due to previous error

24 changes: 24 additions & 0 deletions src/test/ui/lub-glb/old-lub-glb-hr-noteq1.rs
@@ -0,0 +1,24 @@
// Test taking the LUB of two function types that are not equatable but where one is more
// general than the other. Test the case where the more general type (`x`) is the first
// match arm specifically.

fn foo(x: for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8, y: for<'a> fn(&'a u8, &'a u8) -> &'a u8) {
// The two types above are not equivalent. With the older LUB/GLB
// algorithm, this may have worked (I don't remember), but now it
// doesn't because we require equality.
let z = match 22 {
0 => x,
_ => y, //~ ERROR `match` arms have incompatible types
};
}

fn foo_cast(x: for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8, y: for<'a> fn(&'a u8, &'a u8) -> &'a u8) {
// But we can *upcast* explicitly the type of `x` and figure
// things out:
let z = match 22 {
0 => x as for<'a> fn(&'a u8, &'a u8) -> &'a u8,
_ => y,
};
}

fn main() {}
@@ -1,5 +1,5 @@
error[E0308]: `match` arms have incompatible types
--> $DIR/old-lub-glb-hr.rs:40:14
--> $DIR/old-lub-glb-hr-noteq1.rs:11:14
|
LL | let z = match 22 {
| _____________-
Expand Down
33 changes: 33 additions & 0 deletions src/test/ui/lub-glb/old-lub-glb-hr-noteq2.rs
@@ -0,0 +1,33 @@
// Test taking the LUB of two function types that are not equatable but where
// one is more general than the other. Test the case where the more general type
// (`x`) is the second match arm specifically.
//
// Skip for compare-mode because the pure NLL checker accepts this test. (Note
// that it still errors in old-lub-glb-hr-noteq1.rs). What happens is that, due
// to the ordering of the match arms, we pick the correct "more general" fn
// type, and we ignore the errors from the non-NLL type checker that requires
// equality. The NLL type checker only requires a subtyping relationship, and
// that holds.
//
// ignore-compare-mode-nll

fn foo(x: for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8, y: for<'a> fn(&'a u8, &'a u8) -> &'a u8) {
// The two types above are not equivalent. With the older LUB/GLB
// algorithm, this may have worked (I don't remember), but now it
// doesn't because we require equality.
let z = match 22 {
0 => y,
_ => x, //~ ERROR `match` arms have incompatible types
};
}

fn foo_cast(x: for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8, y: for<'a> fn(&'a u8, &'a u8) -> &'a u8) {
// But we can *upcast* explicitly the type of `x` and figure
// things out:
let z = match 22 {
0 => x as for<'a> fn(&'a u8, &'a u8) -> &'a u8,
_ => y,
};
}

fn main() {}
18 changes: 18 additions & 0 deletions src/test/ui/lub-glb/old-lub-glb-hr-noteq2.stderr
@@ -0,0 +1,18 @@
error[E0308]: `match` arms have incompatible types
--> $DIR/old-lub-glb-hr-noteq2.rs:20:14
|
LL | let z = match 22 {
| _____________-
LL | | 0 => y,
| | - this is found to be of type `for<'a> fn(&'a u8, &'a u8) -> &'a u8`
LL | | _ => x,
| | ^ one type is more general than the other
LL | | };
| |_____- `match` arms have incompatible types
|
= note: expected fn pointer `for<'a> fn(&'a u8, &'a u8) -> &'a u8`
found fn pointer `for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

0 comments on commit 6929013

Please sign in to comment.