const generics support

[basil-cow]

No description provided.

[basil-cow]

@nikomatsakis I guess this is ready for review, I can't really think of something to make it nicer. As for the parser, I want #394 to land before I add proper name resolution to lowering.

[detrumi]

Left some comments, but skipped most of chalk-solve in my review since I don't understand half of it 😅

[nikomatsakis]

left misc nits but this all looked basically correct to me

[nikomatsakis]

I think we're still waiting on

work list extracted

Anything else? I haven't done a look, so some of those may be fixed, I mostly wanted to change it so that, in the list of PRs, this one was listed as "changes requested".

[basil-cow]

We have two things we haven't decided on what to do with:
a) this
b) ConcreteConst vs ConstValue

[nikomatsakis]

Work list:

• adding ty back to definition of const
• adding tests listed here
• various nits
• nit: https://github.com/rust-lang/chalk/pull/393/files#r410943055
• ConcreteConst vs ConstValue, naming choice

[basil-cow]

So, I implemented const types by adding Ty to ParameterKinds::Const and to ConstData, moving the old contents into ConstValue. This design has some drawbacks: mainly, it has a lot of duplication.

a) ParameterData aka ParameterKind<Ty, Lifetime, Const> which has type of constant both in ParameterKinds::Const and in Const.
b) all bound vars, inference vars and placeholder vars store their types in both vars themselves and in binders and universes and inference tables.

b) is not really "fixable" I think;
in a) ParameterData should really be ParameterKind::<Ty, Lifetime, ConstValue> from the type standpoint, but that requires(?) to intern ConstValue and that is too many internings imo.

If you have ideas on how to design this better, I would like to hear them.

@jackh726 @detrumi I would appreciate if you took a second look at the pr if you have time

[basil-cow]

One more of my concerns is const type unification: it can push lifetime constraints if types are not equal-equal, which is not a big deal maybe, worst case scenario it's kicking the can down the road. I decided to defer dealing with it until subtyping is implemented

[nikomatsakis]

@Areredify I'm kind of confused -- I would expect to have a type like

pub struct ConstData<I> {
    ty: Ty<I>,
    kind: ConstKind<I>,
}

pub enum ConstKind<I> {
    Variable, Bound, Value, etc
}

and then type Parameter<I> = ParameterKind<Ty<I>, Lifetime<I>, Const<I>>.

Was there something with this approach that didn't work? (Modulo having to find a good name for ConstKind...) I feel like that's the approach you had early on in the PR history.

[basil-cow]

@nikomatsakis That's the current setup + ParameterKind is defined as

pub enum ParameterKind<I, T, L, C> {
    Ty(T),
    Lifetime(L),
    Const {
        ty: Ty<I>,
        value: C
    } 
}

Right now, everything works, my concern is more of "maybe is there a way to design this better"

[nikomatsakis]

@Areredify I'm not sure what you mean by "current", I guess, but it seems like the setup I laid out is not what you described. In particular, I don't understand why the ParameterKind::Const variant should be "different" than the others, in that it contains both a type and a constant. I think the ty field should live in the "constant", as I described.

EDIT: I guess I didn't make it totally clear, but I expect

pub enum ParameterKind<T, L, C> {
    Ty(T),
    Lifetime(L),
    Const(C),
}

in addition to

pub struct ConstData<I> {
    ty: Ty<I>,
    kind: ConstKind<I>,
}

pub enum ConstKind<I> {
    Variable, Bound, Value, etc
}

type Parameter<I> = ParameterKind<Ty<I>, Lifetime<I>, Const<I>>;

[basil-cow]

@nikomatsakis Because we use ParameterKind<T> in a lot of places (ParameterKind<()> in binders, ParameterKind<EnaVariable> in unification) we need it to carry type of the constant with it, that's why my current setup is:

pub enum ParameterKind<I, T, L, C> {
    Ty(T),
    Lifetime(L),
    Const { ty: Ty<I>, value: C },
}

pub struct ConstData<I> {
    ty: Ty<I>,
    kind: ConstKind<I>,
}

pub enum ConstKind<I> {
    Variable, Bound, Value, etc
}

type Parameter<I> = ParameterKind<Ty<I>, Lifetime<I>, Const<I>>;

[nikomatsakis]

@Areredify Why do we need ParameterKind<()> to carry the type of the constant along with it? (Similarly ParameterKind<EnaVariable>)

[basil-cow]

To instantiate universal binders correctly, for example, it's a strange situation when your constants (in unverse sense) are not typed

[basil-cow]

rebased against the GenericArg pr

[nikomatsakis]

Gave this a quick read. Thanks for factoring out the other PR -- this PR now reads fairly cleanly, I think. At least based on my current reading, everything seems like a fairly straightforward extension. Did anything surprise you or catch your eye while you were rebasing?

[basil-cow]

@nikomatsakis locked and loaded; I think there is a suspicious place that we probably should save for later: specialization rules for consts say that literals are more specific than anything else, but in this pr I ignore that. Other than that, I can't think of anything