Rust: Classify various const generics bugs

Created on 20 Jan 2020  路  17Comments  路  Source: rust-lang/rust

@eddyb identified several bugs in Playground examples on Discord, starting here. We should classify them and make sure we have representative edge cases for each when they've fixed.

A-const-generics C-bug F-const_generics metabug
Source
picture varkor

All 17 comments

Most of the issues are probably https://github.com/rust-lang/rust/issues/68398, but there's also https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=8fb424e1ae41c73a264e7ad1c991686d.

varkor picture varkor on 20 Jan 2020

I'm going to use this issue to make note of existing cost generic issues that I think may not require lazy normalisation to fix. The rest are probably blocked on #67890.

varkor picture varkor on 9 Apr 2020

Please cc me on each fix for anything const-generics-related.
I don't have the time to go through all the issues right now but a "make things work" approach to bugfixing is likely to lead to unsoundness.

A very important milestone is #70107, anything that it creates errors in (without ICE-ing) should probably be considered "fixed as not-a-bug" or rather the bug was in the quality of diagnostics but the code was still wrong.

eddyb picture eddyb on 9 Apr 2020
👍1

69239 seems to be related to #69913, afaict the most useful way to fix this would be #70122 which we decided was not worth it.

70507, #69816, #63695, #61936 something something const args in methods, if I remember correctly these can't be fixed easily rn, @eddyb I can't find where you mentioned this. (maybe https://github.com/rust-lang/rust/issues/70167#issuecomment-602016539)

lcnr picture lcnr on 9 Apr 2020
👍1

Yeah we should just ban const args on TypeRelative segments and method calls.
Unless someone has a clever idea about how to type-check them.

eddyb picture eddyb on 9 Apr 2020

They _should_ work. This would be an unnatural restriction, even a temporary one. That said, I'm not sure how to type-check them yet either.

varkor picture varkor on 9 Apr 2020

Afaik type args work with TypeRelative segments, which means that I am missing a crucial difference between const and type args. :/

Can you explain this to me/guide me to some relevant ressources?

lcnr picture lcnr on 9 Apr 2020

@lcnr Types don't have types (or more correctly, the type of all types is the same, some hypothetical Type that you can't refer to inside the Rust language).

Furthermore, the constant expression is a separate body that has to be fully type-checked before it can be evaluated.

So you have to compute the type before you type-check the constant expression, before you can evaluate it, before you can use it in the original body, which is the only one that knows the type.

You could bypass this with lazy normalization, maybe, but it's too easy to end up with a cyclic dependency between the constant expression and the surrounding body.

Maybe we could type-check the constant expression and get the type from there without having an expected type? And only use it if the type is correct?

But then x.method::<0>() would never work as we can't infer what type that is from just 0.

They should work. This would be an unnatural restriction, even a temporary one. That said, I'm not sure how to type-check them yet either.

Disallowing impossible to implement features is not really unnatural.

We also ban specifying generics when impl Trait is used, or specifying lifetimes when some are late-bound, I think?

eddyb picture eddyb on 9 Apr 2020
❤1

While this makes sense, do we even want to know the type/value of the generic argument before the relevant parameters are known? I still think that I am missing something here, as it seems easily solvable for me. :neutral_face:

Looking at this example:

struct A<const N: usize>;

impl A<7> {
    fn test<const M: bool>() {}
}

impl A<42> {
    fn test<const M: Option<()>>() {}
}

let a = A::<7>;
a.test::<true>();

Due to type inference, we should know that a is A<7>, which means that we know that M = bool.
This allows us to then type check true without even being able to cause cycles.

I would not expect, or want, the following to compile for example

let a = A;
a.test::<true>();
// we could infer from `true` that we require a method
// `test<const _: bool>()` which means that `N` must be `7`.

This seems really fragile and causes the problems mentioned by @eddyb.

I don't yet see how my approach can lead to cycle errors :thinking:

lcnr picture lcnr on 9 Apr 2020

true is in another body. You can't type-check while type-checking the parent function.

It's like this:

const main_ANON0: ??? = true;
fn main() {
    // ...
    a.test::<main_ANON0>();
}

You can infer the type main_ANON0 from the body (true), but I would suggest avoiding strawmans like true which is clearly bool: unsuffixed integer literals are more interesting because 0 infers as i32.

The cycle would be if type_of(main_ANON0) looked up the type in typeck_tables_of(main).
That's theoretically fine, but typeck_tables_of(main) (i.e. type-checking main) might try to evaluate the constant which will call const_eval(main_ANON0) which will call build_mir(main_ANON0) which will call typeck_tables_of(main_ANON0) which will need type_of(main_ANON0) in order to check if the body ("true") matches the type.

eddyb picture eddyb on 9 Apr 2020

Disallowing impossible to implement features is not really unnatural.

From a user's perspective, there's no reason this shouldn't work. It's unnatural from a language POV. This is theoretically possible to implement. If the existing infrastructure in rustc makes that difficult, then something should change there.

varkor picture varkor on 9 Apr 2020

I never want to infer the type of main_ANON0 from its body. I should have probably used Default::default() as an argument in my example.

My expectation is that we never actually need type_of(main_ANON0). I only want to evaluate const args after the corresponding parameter is known(which means that the type of main_ANON0 is also known).

Once we know that main_ANON0 corresponds to const N: bool, type_of(main_ANON0) can be set to bool and checked without relying on main.

lcnr picture lcnr on 9 Apr 2020

If the existing infrastructure in rustc makes that difficult, then something should change there.

The existing infrastructure is sound wrt incremental and query execution order. Most of the things proposed at various times, that don't fit the current system, turn out to not have that property!

Once we know that main_ANON0 corresponds to const N: bool, type_of(main_ANON0) can be set to bool and checked without relying on main.

You have no access to side-effects (see my previous paragraph: they would be unsound), type_of can only compute the type by doing queries, and querying typeck_tables_of(main) (the only query which can possibly know about the fact that main_ANON0 is passed to const N: bool) would be correct, but also could easily cause cycle errors as I've explained.

eddyb picture eddyb on 10 Apr 2020

@eddyb and I discussed this in Discord starting here. The summary is that it might be possible to make method calls work after all, by threading DefIds for const parameters in typeck_tables_of and related queries.

varkor picture varkor on 10 Apr 2020

Note that the only reason we found something that may work is that all of the regular queries still do the things I've described above as almost guaranteeing a query cycle.

But we could have const_arg variants of relevant queries, that also get the DefId of the const parameter "taking" the constant expression, besides the constant expression's DefId.

The normal query would call the const_arg variant, grabbing the const parameter DefId from the parent body (which means this causes a cycle error if used during type-checking).

Inside the const_arg variants of the queries, const_arg variants of dependency queries would be used, bottoming out in const_arg_typeck_tables_of using tcx.type_of(const_param_def_id) as the expected type.

So we would add such query variants, as-needed, every time we want to break a cycle.
Anything that is likely not a cycle can continue to rely on e.g. typeck_tables_of, such as lint passes.

eddyb picture eddyb on 10 Apr 2020

Taken from zulip, the following issues are not blocked at the moment:

Bastian Kauschke: https://github.com/rust-lang/rust/issues/70507#issuecomment-615268893 should be doable at the moment, but I don't know how difficult this will be.

^ done

Bastian Kauschke: this is a spurious warning which is related, I looked into this and at least to me the problem was not directly obvious so it is probably also not that easy to fix https://github.com/rust-lang/rust/issues/70225

Bastian Kauschke: https://github.com/rust-lang/rust/issues/68104 is also unrelated, but is also not that easy

Bastian Kauschke: https://github.com/rust-lang/rust/issues/66451 is both fairly self contained and helpful

This also seems like it requires an unrelated fix: https://github.com/rust-lang/rust/issues/71611

lcnr picture lcnr on 29 Apr 2020

I think the issues raised here specifically have been addressed now that https://github.com/rust-lang/rust/pull/74113 has been merged.

varkor picture varkor on 13 Sep 2020
Was this page helpful?
0 / 5 - 0 ratings

Related issues

overvenus picture overvenus  路  3Comments
eddyb picture eddyb  路  3Comments
tikue picture tikue  路  3Comments
wthrowe picture wthrowe  路  3Comments
dtolnay picture dtolnay  路  3Comments