Skip to content
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

Don't infer closure signatures with late-bound type vars #108827

Closed
Show file tree
Hide file tree
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
5 changes: 4 additions & 1 deletion compiler/rustc_hir_typeck/src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
}
if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
// FIXME(non_lifetime_binder): Don't infer a signature with late-bound ty/ct vars
if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue()
&& !inferred_sig.has_non_region_late_bound()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this check not enough to prevent ICEs here? also, can you check that !inferred_sig.has_placeholders because inferring a closure signature to contain placeholders is wrong as the closure type is used outside of the binder which created them

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this just makes sure that we just don't eagerly deduce a closure signature from pending obligations.

We can still infer placeholders when we process pending obligations later, so we'd need to do something after typechecking to make sure that the closures don't capture any type placeholders.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that check here along with !inferred_sig.has_placeholders should be enough.
We keep getting ICE because of #109505 and probably because we're not using ty vars in the root universe.

{
expected_sig = inferred_sig;
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2735,6 +2735,7 @@ impl<'tcx> TypeRelation<'tcx> for SameTypeModuloInfer<'_, 'tcx> {
| (ty::Infer(ty::InferTy::TyVar(_)), _)
| (_, ty::Infer(ty::InferTy::TyVar(_))) => Ok(a),
(ty::Infer(_), _) | (_, ty::Infer(_)) => Err(TypeError::Mismatch),
(ty::Bound(..), _) | (_, ty::Bound(..)) => Err(TypeError::Mismatch),
_ => relate::super_relate_tys(self, a, b),
}
}
Expand Down
22 changes: 15 additions & 7 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -700,12 +700,18 @@ pub trait PrettyPrinter<'tcx>:
}
ty::Error(_) => p!("[type error]"),
ty::Param(ref param_ty) => p!(print(param_ty)),
ty::Bound(debruijn, bound_ty) => match bound_ty.kind {
ty::BoundTyKind::Anon(bv) => {
self.pretty_print_bound_var(debruijn, ty::BoundVar::from_u32(bv))?
ty::Bound(debruijn, bound_ty) => {
if self.should_print_verbose() {
p!(write("Bound({:?}, {:?})", debruijn, bound_ty))
} else {
match bound_ty.kind {
ty::BoundTyKind::Anon(bv) => {
self.pretty_print_bound_var(debruijn, ty::BoundVar::from_u32(bv))?
}
ty::BoundTyKind::Param(_, s) => p!(write("{}", s)),
}
}
ty::BoundTyKind::Param(_, s) => p!(write("{}", s)),
},
}
ty::Adt(def, substs) => {
p!(print_def_path(def.did(), substs));
}
Expand Down Expand Up @@ -736,8 +742,10 @@ pub trait PrettyPrinter<'tcx>:
}
}
ty::Placeholder(placeholder) => match placeholder.name {
ty::BoundTyKind::Anon(_) => p!(write("Placeholder({:?})", placeholder)),
ty::BoundTyKind::Param(_, name) => p!(write("{}", name)),
ty::BoundTyKind::Param(_, name) if !self.should_print_verbose() => {
p!(write("{}", name))
}
_ => p!(write("Placeholder({:?})", placeholder)),
},
ty::Alias(ty::Opaque, ty::AliasTy { def_id, substs, .. }) => {
// We use verbose printing in 'NO_QUERIES' mode, to
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ pub trait TypeVisitableExt<'tcx>: TypeVisitable<TyCtxt<'tcx>> {
| TypeFlags::HAS_CT_PLACEHOLDER,
)
}
/// True if there are any non-region placeholders
fn has_non_region_placeholders(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_TY_PLACEHOLDER | TypeFlags::HAS_CT_PLACEHOLDER)
}
fn needs_subst(&self) -> bool {
self.has_type_flags(TypeFlags::NEEDS_SUBST)
}
Expand Down
23 changes: 16 additions & 7 deletions compiler/rustc_trait_selection/src/solve/project_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,22 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
) -> QueryResult<'tcx> {
let tcx = ecx.tcx();
let Some(tupled_inputs_and_output) =
structural_traits::extract_tupled_inputs_and_output_from_callable(
tcx,
goal.predicate.self_ty(),
goal_kind,
)? else {
return ecx.make_canonical_response(Certainty::AMBIGUOUS);
};
structural_traits::extract_tupled_inputs_and_output_from_callable(
tcx,
goal.predicate.self_ty(),
goal_kind,
)? else {
return ecx.make_canonical_response(Certainty::AMBIGUOUS);
};

// FIXME(non_lifetime_binders): Higher-ranked Fn trait candidates are not (yet) supported.
// Make sure that the inputs/output don't capture any placeholder types.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with this. Canonicalization also maps ty::Param to a placeholder.

I generally think that this check should happen somewhere else 🤔

if (goal.predicate.projection_ty.substs[1], goal.predicate.term)
.has_non_region_placeholders()
{
return Err(NoSolution);
}

let output_is_sized_pred = tupled_inputs_and_output
.map_bound(|(_, output)| tcx.at(DUMMY_SP).mk_trait_ref(LangItem::Sized, [output]));

Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,13 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
goal_kind: ty::ClosureKind,
) -> QueryResult<'tcx> {
let tcx = ecx.tcx();

// FIXME(non_lifetime_binders): Higher-ranked Fn trait candidates are not (yet) supported.
// Check that the inputs don't capture any placeholder types.
if goal.predicate.trait_ref.substs[1].has_non_region_placeholders() {
return Err(NoSolution);
}

let Some(tupled_inputs_and_output) =
structural_traits::extract_tupled_inputs_and_output_from_callable(
tcx,
Expand Down
16 changes: 15 additions & 1 deletion compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,21 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
};

let trait_ref = self.closure_trait_ref_unnormalized(obligation, substs);
let mut nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;

// FIXME(non_lifetime_binders): Perform the equivalent of a "leak check" here.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we could let this succeed, then error out during HIR typeck or something, though it would be harder to point to the source of the error.

let mut nested = self.infcx.commit_if_ok(|_| {
let nested = self.confirm_poly_trait_refs(obligation, trait_ref)?;

if self
.infcx
.resolve_vars_if_possible(substs.as_closure().sig())
.has_non_region_placeholders()
{
return Err(SelectionError::Unimplemented);
}

Ok(nested)
})?;

debug!(?closure_def_id, ?trait_ref, ?nested, "confirm closure candidate obligations");

Expand Down
6 changes: 2 additions & 4 deletions tests/ui/nll/user-annotations/dump-fn-method.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Unit test for the "user substitutions" that are annotated on each
// node.

// compile-flags:-Zverbose
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test's output changes with the modification to how bound/placeholder tys print under -Zverbose, though arguably I could revert that change -- just made debug a bit easier for me.


#![feature(rustc_attrs)]

// Note: we reference the names T and U in the comments below.
Expand All @@ -26,7 +24,7 @@ fn main() {
let x = foo::<u32>;
x(22);

let x = foo::<&'static u32>; //~ ERROR [&ReStatic u32]
let x = foo::<&'static u32>; //~ ERROR [&'static u32]
x(&22);

// Here: we only want the `T` to be given, the rest should be variables.
Expand All @@ -41,7 +39,7 @@ fn main() {
x(&22, 44, 66);

// Here: all are given and we have a lifetime.
let x = <u8 as Bazoom<&'static u16>>::method::<u32>; //~ ERROR [u8, &ReStatic u16, u32]
let x = <u8 as Bazoom<&'static u16>>::method::<u32>; //~ ERROR [u8, &'static u16, u32]
x(&22, &44, 66);

// Here: we want in particular that *only* the method `U`
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/nll/user-annotations/dump-fn-method.stderr
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
error: user substs: UserSubsts { substs: [&ReStatic u32], user_self_ty: None }
--> $DIR/dump-fn-method.rs:29:13
error: user substs: UserSubsts { substs: [&'static u32], user_self_ty: None }
--> $DIR/dump-fn-method.rs:27:13
|
LL | let x = foo::<&'static u32>;
| ^^^^^^^^^^^^^^^^^^^

error: user substs: UserSubsts { substs: [^0, u32, ^1], user_self_ty: None }
--> $DIR/dump-fn-method.rs:35:13
--> $DIR/dump-fn-method.rs:33:13
|
LL | let x = <_ as Bazoom<u32>>::method::<_>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: user substs: UserSubsts { substs: [u8, &ReStatic u16, u32], user_self_ty: None }
--> $DIR/dump-fn-method.rs:44:13
error: user substs: UserSubsts { substs: [u8, &'static u16, u32], user_self_ty: None }
--> $DIR/dump-fn-method.rs:42:13
|
LL | let x = <u8 as Bazoom<&'static u16>>::method::<u32>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: user substs: UserSubsts { substs: [^0, ^1, u32], user_self_ty: None }
--> $DIR/dump-fn-method.rs:52:5
--> $DIR/dump-fn-method.rs:50:5
|
LL | y.method::<u32>(44, 66);
| ^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
58 changes: 58 additions & 0 deletions tests/ui/traits/non_lifetime_binders/closure-infer.classic.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
warning: the feature `non_lifetime_binders` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/closure-infer.rs:4:12
|
LL | #![feature(non_lifetime_binders)]
| ^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #108185 <https://github.com/rust-lang/rust/issues/108185> for more information
= note: `#[warn(incomplete_features)]` on by default

error[E0277]: expected a `Fn<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:16:5: 16:8]`
--> $DIR/closure-infer.rs:12:15
|
LL | fn take2() -> impl for<T> Fn(T) -> T {
| ^^^^^^^^^^^^^^^^^^^^^^ expected an `Fn<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:16:5: 16:8]`
|
= help: the trait `Fn<(T,)>` is not implemented for closure `[closure@$DIR/closure-infer.rs:16:5: 16:8]`

error[E0277]: expected a `FnOnce<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:16:5: 16:8]`
--> $DIR/closure-infer.rs:12:15
|
LL | fn take2() -> impl for<T> Fn(T) -> T {
| ^^^^^^^^^^^^^^^^^^^^^^ expected an `FnOnce<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:16:5: 16:8]`
|
= help: the trait `FnOnce<(T,)>` is not implemented for closure `[closure@$DIR/closure-infer.rs:16:5: 16:8]`

error[E0277]: expected a `Fn<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:20:10: 20:13]`
--> $DIR/closure-infer.rs:20:10
|
LL | take(|x| x)
| ---- ^^^^^ expected an `Fn<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:20:10: 20:13]`
| |
| required by a bound introduced by this call
|
= help: the trait `Fn<(T,)>` is not implemented for closure `[closure@$DIR/closure-infer.rs:20:10: 20:13]`
note: required by a bound in `take`
--> $DIR/closure-infer.rs:7:18
|
LL | fn take(id: impl for<T> Fn(T) -> T) {
| ^^^^^^^^^^^^^^^^^ required by this bound in `take`

error[E0277]: expected a `FnOnce<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:20:10: 20:13]`
--> $DIR/closure-infer.rs:20:10
|
LL | take(|x| x)
| ---- ^^^^^ expected an `FnOnce<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:20:10: 20:13]`
| |
| required by a bound introduced by this call
|
= help: the trait `FnOnce<(T,)>` is not implemented for closure `[closure@$DIR/closure-infer.rs:20:10: 20:13]`
note: required by a bound in `take`
--> $DIR/closure-infer.rs:7:34
|
LL | fn take(id: impl for<T> Fn(T) -> T) {
| ^ required by this bound in `take`

error: aborting due to 4 previous errors; 1 warning emitted

For more information about this error, try `rustc --explain E0277`.
56 changes: 56 additions & 0 deletions tests/ui/traits/non_lifetime_binders/closure-infer.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
warning: the feature `non_lifetime_binders` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/closure-infer.rs:4:12
|
LL | #![feature(non_lifetime_binders)]
| ^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #108185 <https://github.com/rust-lang/rust/issues/108185> for more information
= note: `#[warn(incomplete_features)]` on by default

error[E0277]: expected a `Fn<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:16:5: 16:8]`
--> $DIR/closure-infer.rs:12:15
|
LL | fn take2() -> impl for<T> Fn(T) -> T {
| ^^^^^^^^^^^^^^^^^^^^^^ expected an `Fn<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:16:5: 16:8]`
|
= help: the trait `Fn<(T,)>` is not implemented for closure `[closure@$DIR/closure-infer.rs:16:5: 16:8]`

error[E0271]: type mismatch resolving `<[closure@closure-infer.rs:16:5] as FnOnce<(T,)>>::Output == T`
--> $DIR/closure-infer.rs:12:15
|
LL | fn take2() -> impl for<T> Fn(T) -> T {
| ^^^^^^^^^^^^^^^^^^^^^^ types differ

error[E0277]: expected a `Fn<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:20:10: 20:13]`
--> $DIR/closure-infer.rs:20:10
|
LL | take(|x| x)
| ---- ^^^^^ expected an `Fn<(T,)>` closure, found `[closure@$DIR/closure-infer.rs:20:10: 20:13]`
| |
| required by a bound introduced by this call
|
= help: the trait `Fn<(T,)>` is not implemented for closure `[closure@$DIR/closure-infer.rs:20:10: 20:13]`
note: required by a bound in `take`
--> $DIR/closure-infer.rs:7:18
|
LL | fn take(id: impl for<T> Fn(T) -> T) {
| ^^^^^^^^^^^^^^^^^ required by this bound in `take`

error[E0271]: type mismatch resolving `<[closure@closure-infer.rs:20:10] as FnOnce<(T,)>>::Output == T`
--> $DIR/closure-infer.rs:20:10
|
LL | take(|x| x)
| ---- ^^^^^ types differ
| |
| required by a bound introduced by this call
|
note: required by a bound in `take`
--> $DIR/closure-infer.rs:7:34
|
LL | fn take(id: impl for<T> Fn(T) -> T) {
| ^ required by this bound in `take`

error: aborting due to 4 previous errors; 1 warning emitted

Some errors have detailed explanations: E0271, E0277.
For more information about an error, try `rustc --explain E0271`.
24 changes: 24 additions & 0 deletions tests/ui/traits/non_lifetime_binders/closure-infer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// revisions: classic next
//[next] compile-flags: -Ztrait-solver=next

#![feature(non_lifetime_binders)]
//~^ WARNING the feature `non_lifetime_binders` is incomplete

fn take(id: impl for<T> Fn(T) -> T) {
id(0);
id("");
}

fn take2() -> impl for<T> Fn(T) -> T {
//~^ ERROR expected a `Fn<(T,)>` closure, found
//[classic]~| ERROR expected a `FnOnce<(T,)>` closure, found
//[next]~| ERROR type mismatch resolving
|x| x
}

fn main() {
take(|x| x)
//~^ ERROR expected a `Fn<(T,)>` closure, found
//[classic]~| ERROR expected a `FnOnce<(T,)>` closure, found
//[next]~| ERROR type mismatch resolving
}