Stream: t-compiler/help

Topic: lazy normalization


Bastian Kauschke (May 08 2020 at 17:33, on Zulip):

https://github.com/rust-lang/rust/pull/71973

I am currently trying to remove lazy_normalization_consts by checking the const_generics flag instead.

Bastian Kauschke (May 08 2020 at 17:33, on Zulip):

Doing this lead to a stack overflow for the following test:

#![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 T: usize> {}
impl<const T: usize> Foo<T>
where
    [u8; T]: Bar<[(); T]>,
{
    fn foo() {}
}

fn main() {
    Foo::foo();
}
Bastian Kauschke (May 08 2020 at 17:34, on Zulip):

With the following cycle:

[DEBUG rustc_infer::infer::resolve] OpportunisticVarResolver::fold_const(Const { ty: usize, val: Unevaluated(DefId(0:7 ~ issue_69654[317d]::{{impl}}[0]::{{constant}}[0]), [_], None) })
[DEBUG rustc_infer::infer::resolve] OpportunisticVarResolver::fold_ty(usize)
[DEBUG rustc_infer::infer::resolve] OpportunisticVarResolver::fold_ty(_)
[DEBUG rustc_infer::infer::resolve] OpportunisticVarResolver::fold_ty(())
Bastian Kauschke (May 08 2020 at 17:38, on Zulip):

cc @varkor @nikomatsakis do you have an idea where this might come from?

Bastian Kauschke (May 08 2020 at 17:38, on Zulip):

Or more generally, how to debug this further

Bastian Kauschke (May 08 2020 at 17:43, on Zulip):

This somehow creates an unevaluated const which has itself as a subst afaict

Bastian Kauschke (May 08 2020 at 17:45, on Zulip):

https://github.com/rust-lang/rust/blob/7b805396bf46dce972692a6846ce2ad8481c5f85/src/librustc_infer/infer/resolve.rs#L40
Should we short circuit here due to lazy normalization?

Bastian Kauschke (May 08 2020 at 17:49, on Zulip):

^ That doesn't work :disappointed:

Bastian Kauschke (May 08 2020 at 17:55, on Zulip):

Will look at Const::Unevaluated creation for now

Bastian Kauschke (May 08 2020 at 19:57, on Zulip):

oh no, type_of is probably at fault here :distraught: I was actually the one who introduced this incorrect behavior :/ fu

nikomatsakis (May 08 2020 at 20:24, on Zulip):

type_of?

nikomatsakis (May 08 2020 at 20:24, on Zulip):

that is, the test is not using type_of

nikomatsakis (May 08 2020 at 20:25, on Zulip):

(right?)

Bastian Kauschke (May 08 2020 at 20:25, on Zulip):

This type of :laughing: https://github.com/rust-lang/rust/blob/7b805396bf46dce972692a6846ce2ad8481c5f85/src/librustc_typeck/collect/type_of.rs#L273

Bastian Kauschke (May 08 2020 at 20:27, on Zulip):

Some Node::Ty's have Some(Res::Err) without having an actual error, which is why I added this filter here

Matthew Jasper (May 08 2020 at 20:29, on Zulip):

Res::Err without an actual error sounds like a bug in resolve/hir construction...

nikomatsakis (May 08 2020 at 20:30, on Zulip):

agreed

Bastian Kauschke (May 08 2020 at 20:31, on Zulip):

Hm, we currently create a lot of Res::Err without an associated delay_span_bug :sweat_smile: So while I agree,
I wasn't able to fix it

Bastian Kauschke (May 08 2020 at 20:42, on Zulip):

Bad news, Node::Ty aren't the only problem.

Bastian Kauschke (May 08 2020 at 20:43, on Zulip):

for example

// run-pass

#![feature(const_generics)]
//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash

trait T<const A: usize> {
    fn l<const N: bool>() -> usize;
    fn r<const N: bool>() -> bool;
}

struct S;

impl<const N: usize> T<N> for S {
    fn l<const M: bool>() -> usize { N }
    fn r<const M: bool>() -> bool { M }
}

fn main() {
   assert_eq!(<S as T<123>>::l::<true>(), 123);
   assert!(<S as T<123>>::r::<true>());
}

Has an incorrect Res as well:

error: internal compiler error: src/librustc_typeck/collect/type_of.rs:274: unexpected anon const resultion for parent: Expr(Expr { hir_id: HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 31 }, kind: Struct(Resolved(None, Path { span: /home/programming/rust/src/test/ui/const-generics/types-mismatch-const-args.rs:13:41: 13:69, res: Def(Struct, DefId(0:4 ~ types_mismatch_const_args[317d]::A[0])), segments: [PathSegment { ident: A#0, hir_id: Some(HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 27 }), res: Some(Err), args: Some(GenericArgs { args: [Lifetime(lifetime(HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 16 }: 'a)), Type(Ty { hir_id: HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 17 }, kind: Path(Resolved(None, Path { span: /home/programming/rust/src/test/ui/const-generics/types-mismatch-const-args.rs:13:49: 13:52, res: PrimTy(Uint(U32)), segments: [PathSegment { ident: u32#0, hir_id: Some(HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 18 }), res: Some(Err), args: None, infer_args: false }] })), span: /home/programming/rust/src/test/ui/const-generics/types-mismatch-const-args.rs:13:49: 13:52 }), Const(ConstArg { value: AnonConst { hir_id: HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 19 }, body: BodyId { hir_id: HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 22 } } }, span: /home/programming/rust/src/test/ui/const-generics/types-mismatch-const-args.rs:13:54: 13:60 }), Const(ConstArg { value: AnonConst { hir_id: HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 23 }, body: BodyId { hir_id: HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 26 } } }, span: /home/programming/rust/src/test/ui/const-generics/types-mismatch-const-args.rs:13:62: 13:68 })], bindings: [], parenthesized: false }), infer_args: false }] }), [Field { hir_id: HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 28 }, ident: data#0, expr: Expr { hir_id: HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 30 }, kind: Path(Resolved(None, Path { span: /home/programming/rust/src/test/ui/const-generics/types-mismatch-const-args.rs:13:78: 13:89, res: Def(Ctor(Struct, Const), DefId(2:1915 ~ core[99f1]::marker[0]::PhantomData[0]::{{constructor}}[0])), segments: [PathSegment { ident: PhantomData#0, hir_id: Some(HirId { owner: DefId(0:10 ~ types_mismatch_const_args[317d]::a[0]), local_id: 29 }), res: Some(Err), args: None, infer_args: true }] })), attrs: ThinVec(None), span: /home/programming/rust/src/test/ui/const-generics/types-mismatch-const-args.rs:13:78: 13:89 }, span: /home/programming/rust/src/test/ui/const-generics/types-mismatch-const-args.rs:13:72: 13:89, is_shorthand: false }], None), attrs: ThinVec(None), span: /home/programming/rust/src/test/ui/const-generics/types-mismatch-const-args.rs:13:41: 13:91 })
Bastian Kauschke (May 08 2020 at 20:54, on Zulip):

Would it be a good idea to add a token with a private constructor to all these error variants?
Similar to what I am doing here https://github.com/lcnr/computer/blob/8b8681dc83ca2df24e7d5bc5e1c96c9673ebd6a8/boulder/diagnostics/src/lib.rs#L20

nikomatsakis (May 08 2020 at 21:03, on Zulip):

I have to admit I'm kind of confused as to what is going on here

nikomatsakis (May 08 2020 at 21:03, on Zulip):

I'd have to read more into the code to understand what I think is the right solution

Bastian Kauschke (May 08 2020 at 21:09, on Zulip):

This is something I should be able to either fix myself, or at least get far enough to be certain where this failure comes from.

If my current thinking is correct this will be both hard to fix and somewhat orthogonal to lazy normalization.
IMO it may be best to disable this test for now and solve this problem afterwards in this case

Bastian Kauschke (May 08 2020 at 23:01, on Zulip):

Res::Err is not actually the culprit here.
Which is both good, because we can fix this bug as part of this PR, and bad, because I still haven't done so :laughing:
I believe it's a wrong interaction here https://github.com/rust-lang/rust/blob/7b805396bf46dce972692a6846ce2ad8481c5f85/src/librustc_middle/ty/sty.rs#L2280

While creates a ConstKind::Unevaluated with a subst which indirectly references itself.
Will look into this further tomorrow

mark-i-m (May 08 2020 at 23:01, on Zulip):

Would it be a good idea to add a token with a private constructor to all these error variants?

(aside: I'm actually doing this for TyKind::Error in #70551)

Bastian Kauschke (May 09 2020 at 10:16, on Zulip):

I believe it's a wrong interaction here https://github.com/rust-lang/rust/blob/7b805396bf46dce972692a6846ce2ad8481c5f85/src/librustc_middle/ty/sty.rs#L2280

tbh I don't know where this problem comes from :distraught:

Somewhat minimized (this has a stack overflow):

#![feature(const_generics)]
#![allow(incomplete_features)]

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();
}
Bastian Kauschke (May 09 2020 at 10:16, on Zulip):

This does not:

// run-pass
#![feature(const_generics)]
#![allow(incomplete_features)]

trait Bar<T> {}
impl<T> Bar<T> for [u8; 7] {}
// ^ removing the braces around 7

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

fn main() {
    Foo::foo();
}
Bastian Kauschke (May 09 2020 at 10:20, on Zulip):

And this also works:

// run-pass
#![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::<{7}>::foo();
    // ^ turbofish to the rescue
}
Bastian Kauschke (May 09 2020 at 12:45, on Zulip):

I believe that the lowering hir -> Ty/Const is not at fault here but don't really know how to progress further here.

@nikomatsakis @varkor can you help me out here or are you fine with disabling this test for now?

lcnr (May 09 2020 at 20:16, on Zulip):

might be related to https://github.com/rust-lang/rust/issues/68104

varkor (May 10 2020 at 12:51, on Zulip):

I'm happy to disable tests involving specialisation for const parameters for now: there are unanswered questions about how exactly it should work, and we definitely haven't put any effort into deliberately making it work yet.

lcnr (May 10 2020 at 13:53, on Zulip):

What's the best way to do this? Should I just remove the test in my PR and open an issue for it?

lcnr (May 17 2020 at 08:47, on Zulip):

cursed

Last update: Sep 28 2020 at 16:00UTC