Skip to content

Check substs of ConstKind::Unevaluated in generalize #72351

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

Closed
wants to merge 4 commits into from
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
38 changes: 29 additions & 9 deletions src/librustc_infer/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ impl<'infcx, 'tcx> CombineFields<'infcx, 'tcx> {
for_vid: ty::TyVid,
dir: RelationDir,
) -> RelateResult<'tcx, Generalization<'tcx>> {
debug!("generalize(ty={:?}, for_vid={:?}, dir={:?}", ty, for_vid, dir);
debug!("generalize(ty={:?}, for_vid={:?}, dir={:?})", ty, for_vid, dir);
// Determine the ambient variance within which `ty` appears.
// The surrounding equation is:
//
Expand Down Expand Up @@ -649,27 +649,47 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
) -> RelateResult<'tcx, &'tcx ty::Const<'tcx>> {
assert_eq!(c, c2); // we are abusing TypeRelation here; both LHS and RHS ought to be ==

debug!("generalize: consts c={:?}", c);
match c.val {
ty::ConstKind::Infer(InferConst::Var(vid)) => {
let mut inner = self.infcx.inner.borrow_mut();
let variable_table = &mut inner.const_unification_table();
let var_value = variable_table.probe_value(vid);
let var_value =
self.infcx.inner.borrow_mut().const_unification_table().probe_value(vid);
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, so, actually, what does subtyping even mean with respect to const kind parameters? I assume that T<C1> <: T<C2> only if C1 == C2, right? In other words, if we have something like ?X <: Foo<?Y, ?C>, we probably want to instantiate Foo with Foo<?Z, ?C> -- i.e., the constant still has to be equal..?

But I guess the point is that it only has to be equal after evaluation, and hence we do want a fresh variable ?C2 and the requirement that ?C1 = ?C2, which really means that the evaluate to the same value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't quite get why subtyping is relevant here 🤔

Currently looking into getting a potentially more helpful example, the minimized const generics example is

#![feature(const_generics)]
trait Bar<const M: usize> {}
impl<const N: usize> Bar<N> for A<{ 6 + 1 }> {}

struct A<const N: usize>
where
    A<N>: Bar<N>;

fn main() {
    let _ = A;
}

Copy link
Contributor Author

@lcnr lcnr Jul 2, 2020

Choose a reason for hiding this comment

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

And the same problem for ty vars

#![feature(const_generics)]

// The goal is is to get an unevaluated const `ct` with a `Ty::Infer(TyVar(_#1t)` subst.
//
// If we are then able to infer `ty::Infer(TyVar(_#1t) := Ty<ct>`, we should be able
// to get a stack overflow.

struct Foo<const N: usize>;

trait Bind<T> {
    fn bind() -> (T, Self);
}

// `N` has to be `ConstKind::Unevaluated`.
impl<T> Bind<T> for Foo<{ 6 + 1 }> {
    fn bind() -> (T, Self) {
        (panic!(), Foo)
    }
}

fn main() {
    let (mut t, foo) = Foo::bind();
    // `t` is `ty::Infer(TyVar(_#1t))`,
    // `foo` contains `ty::Infer(TyVar(_#1t))` in its substs
    t = foo; // ! MISSED OCCURS CHECK => STACKOVERFLOW !
}

Copy link
Contributor Author

@lcnr lcnr Jul 2, 2020

Choose a reason for hiding this comment

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

Further minimized, we don't even need explicit const params

#![feature(const_generics)]

// The goal is is to get an unevaluated const `ct` with a `Ty::Infer(TyVar(_#1t)` subst.
//
// If we are then able to infer `ty::Infer(TyVar(_#1t) := Ty<ct>`, we should be able
// to get a stack overflow.
fn bind<T>() -> (T, [u8; 6 + 1]) {
    todo!()
} 

fn main() {
    let (mut t, foo) = bind();
    // `t` is `ty::Infer(TyVar(_#1t))`,
    // `foo` contains `ty::Infer(TyVar(_#1t))` in its substs
    t = foo; // ! MISSED OCCURS CHECK => STACKOVERFLOW !
}

match var_value.val {
ConstVariableValue::Known { value: u } => self.relate(&u, &u),
ConstVariableValue::Unknown { universe } => {
if self.for_universe.can_name(universe) {
Ok(c)
} else {
let new_var_id = variable_table.new_key(ConstVarValue {
origin: var_value.origin,
val: ConstVariableValue::Unknown { universe: self.for_universe },
});
let new_var_id =
self.infcx.inner.borrow_mut().const_unification_table().new_key(
ConstVarValue {
origin: var_value.origin,
val: ConstVariableValue::Unknown {
universe: self.for_universe,
},
},
);
Ok(self.tcx().mk_const_var(new_var_id, c.ty))
}
}
}
}
ty::ConstKind::Unevaluated(..) if self.tcx().lazy_normalization() => Ok(c),
ty::ConstKind::Unevaluated(did, substs, promoted)
if self.tcx().lazy_normalization() =>
{
// We have to generalize inference variables used in the generic substitutions,
// as unevaluated consts may otherwise contain invalid inference variables.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is kind of uninformative, yeah. It'd be nice to have an example. But I guess it comes down to the point I was raising earlier, right? That two 'unevaluated' constants can be equal so long as they are equal after evaluation, and we don't know yet which of the substitutions will be important?

let new_substs =
self.relate_with_variance(ty::Variance::Invariant, &substs, &substs)?;
Copy link
Member

Choose a reason for hiding this comment

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

Is this an existing pattern? It feels like a hack, because relating something to itself should be a no-op. Why does this work?

Copy link
Contributor Author

@lcnr lcnr Jun 30, 2020

Choose a reason for hiding this comment

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

This is an existing pattern, we already (ab)use type relations in a lot of places where want to traverse a given type/const/whatever:

assert_eq!(t, t2); // we are abusing TypeRelation here; both LHS and RHS ought to be ==

or

impl<D> TypeRelation<'tcx> for TypeGeneralizer<'me, 'tcx, D>

Copy link
Member

@varkor varkor Jun 30, 2020

Choose a reason for hiding this comment

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

Would you be able to give a small example of what's going wrong as part of a comment in the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

later this week 😆 I have to think of a good example where this check is required and not just
a stopgap because we have unused substs in ConstKind::Unevaluated.

if new_substs != substs {
Ok(self.tcx().mk_const(ty::Const {
ty: c.ty,
val: ty::ConstKind::Unevaluated(did, new_substs, promoted),
}))
} else {
Ok(c)
}
}
_ => relate::super_relate_consts(self, c, c),
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_middle/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,7 @@ impl<'tcx> TypeFoldable<'tcx> for interpret::GlobalId<'tcx> {

impl<'tcx> TypeFoldable<'tcx> for Ty<'tcx> {
fn super_fold_with<F: TypeFolder<'tcx>>(&self, folder: &mut F) -> Self {
debug!("Ty::super_fold_with({:?})", self);
let kind = match self.kind {
ty::RawPtr(tm) => ty::RawPtr(tm.fold_with(folder)),
ty::Array(typ, sz) => ty::Array(typ.fold_with(folder), sz.fold_with(folder)),
Expand Down Expand Up @@ -1040,6 +1041,7 @@ impl<'tcx, T: TypeFoldable<'tcx>, I: Idx> TypeFoldable<'tcx> for IndexVec<I, T>

impl<'tcx> TypeFoldable<'tcx> for &'tcx ty::Const<'tcx> {
fn super_fold_with<F: TypeFolder<'tcx>>(&self, folder: &mut F) -> Self {
debug!("Const::super_fold_with({:?})", self);
let ty = self.ty.fold_with(folder);
let val = self.val.fold_with(folder);
if ty != self.ty || val != self.val {
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/const-generics/issues/issue-69654-run-pass.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![feature(const_generics)]
#![allow(incomplete_features, unused_braces)]

trait Bar<T> {}
impl<T> Bar<T> for [u8; {7}] {}

struct Foo<const N: usize> {}
impl<const N: usize> Foo<N>
where
[u8; N]: Bar<[(); N]>,
{
fn foo() {}
}

fn main() {
Foo::foo();
//~^ ERROR no function or associated item named `foo`
// FIXME(const_generics): The above should not error
}
15 changes: 15 additions & 0 deletions src/test/ui/const-generics/issues/issue-69654-run-pass.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0599]: no function or associated item named `foo` found for struct `Foo<{_: usize}>` in the current scope
--> $DIR/issue-69654-run-pass.rs:16:10
|
LL | struct Foo<const N: usize> {}
| -------------------------- function or associated item `foo` not found for this
...
LL | Foo::foo();
| ^^^ function or associated item not found in `Foo<{_: usize}>`
|
= note: the method `foo` exists but the following trait bounds were not satisfied:
`[u8; _]: Bar<[(); _]>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0599`.
19 changes: 19 additions & 0 deletions src/test/ui/const-generics/issues/issue-69654.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![feature(const_generics)]
#![allow(incomplete_features)]

trait Bar<T> {}
impl<T> Bar<T> for [u8; T] {}
//~^ ERROR expected value, found type parameter `T`

struct Foo<const N: usize> {}
impl<const N: usize> Foo<N>
where
[u8; N]: Bar<[(); N]>,
{
fn foo() {}
}

fn main() {
Foo::foo();
//~^ ERROR no function or associated item named `foo`
}
22 changes: 22 additions & 0 deletions src/test/ui/const-generics/issues/issue-69654.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0423]: expected value, found type parameter `T`
--> $DIR/issue-69654.rs:5:25
|
LL | impl<T> Bar<T> for [u8; T] {}
| ^ not a value

error[E0599]: no function or associated item named `foo` found for struct `Foo<{_: usize}>` in the current scope
--> $DIR/issue-69654.rs:17:10
|
LL | struct Foo<const N: usize> {}
| -------------------------- function or associated item `foo` not found for this
...
LL | Foo::foo();
| ^^^ function or associated item not found in `Foo<{_: usize}>`
|
= note: the method `foo` exists but the following trait bounds were not satisfied:
`[u8; _]: Bar<[(); _]>`

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0423, E0599.
For more information about an error, try `rustc --explain E0423`.
12 changes: 12 additions & 0 deletions src/test/ui/const-generics/unused-substs/unused-substs-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![feature(const_generics)] //~ WARN the feature `const_generics` is incomplete
trait Bar<const M: usize> {}
impl<const N: usize> Bar<N> for A<{ 6 + 1 }> {}

struct A<const N: usize>
where
A<N>: Bar<N>;

fn main() {
let _ = A;
//~^ ERROR mismatched types
}
26 changes: 26 additions & 0 deletions src/test/ui/const-generics/unused-substs/unused-substs-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#![feature(const_generics)] //~ WARN the feature `const_generics` is incomplete

// The goal is is to get an unevaluated const `ct` with a `Ty::Infer(TyVar(_#1t)` subst.
//
// If we are then able to infer `ty::Infer(TyVar(_#1t) := Ty<ct>` we introduced an
// artificial inference cycle.
struct Foo<const N: usize>;

trait Bind<T> {
fn bind() -> (T, Self);
}

// `N` has to be `ConstKind::Unevaluated`.
impl<T> Bind<T> for Foo<{ 6 + 1 }> {
fn bind() -> (T, Self) {
(panic!(), Foo)
}
}

fn main() {
let (mut t, foo) = Foo::bind();
// `t` is `ty::Infer(TyVar(_#1t))`
// `foo` contains `ty::Infer(TyVar(_#1t))` in its substs
t = foo;
//~^ ERROR mismatched types
}
18 changes: 18 additions & 0 deletions src/test/ui/const-generics/unused-substs/unused-substs-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
warning: the feature `const_generics` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/unused-substs-2.rs:1:12
|
LL | #![feature(const_generics)]
| ^^^^^^^^^^^^^^
|
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #44580 <https://github.com/rust-lang/rust/issues/44580> for more information

error[E0308]: mismatched types
--> $DIR/unused-substs-2.rs:24:9
|
LL | t = foo;
| ^^^ cyclic type of infinite size

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0308`.
18 changes: 18 additions & 0 deletions src/test/ui/const-generics/unused-substs/unused-substs-3.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@

#![feature(const_generics)] //~ WARN the feature `const_generics` is incomplete

// The goal is is to get an unevaluated const `ct` with a `Ty::Infer(TyVar(_#1t)` subst.
//
// If we are then able to infer `ty::Infer(TyVar(_#1t) := Ty<ct>` we introduced an
// artificial inference cycle.
fn bind<T>() -> (T, [u8; 6 + 1]) {
todo!()
}

fn main() {
let (mut t, foo) = bind();
// `t` is `ty::Infer(TyVar(_#1t))`
// `foo` contains `ty::Infer(TyVar(_#1t))` in its substs
t = foo;
//~^ ERROR mismatched types
}
21 changes: 21 additions & 0 deletions src/test/ui/const-generics/unused-substs/unused-substs-3.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
warning: the feature `const_generics` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/unused-substs-3.rs:2:12
|
LL | #![feature(const_generics)]
| ^^^^^^^^^^^^^^
|
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #44580 <https://github.com/rust-lang/rust/issues/44580> for more information

error[E0308]: mismatched types
--> $DIR/unused-substs-3.rs:16:9
|
LL | t = foo;
| ^^^
| |
| cyclic type of infinite size
| help: try using a conversion method: `foo.to_vec()`

error: aborting due to previous error; 1 warning emitted

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