Add std::ops::Index support for infering

[edwin0cheng]

see also #2534

Seem like this can't fix #2534 for this case:

fn foo3(bar: [usize; 2]) {
    let baz = bar[1];   // <--- baz is still unknown ?
    println!("{}", baz);
}

[edwin0cheng]

cc @flodiebold

[edwin0cheng]

Seem like this PR itself is not enough to prove all trait associated types for [T] impl for std::ops::Index ?

impl<T, I> ops::Index<I> for [T]
    where I: SliceIndex<[T]>
{
    type Output = I::Output;

    #[inline]
    fn index(&self, index: I) -> &I::Output {
        index.index(self)
    }
}

[kiljacken]

I have some input on the issue of the example and a little on the new helper func, but currently stuck in public transit so it will be an hour or so :)

…

On Thu, 19 Dec 2019, 06:47 Edwin Cheng, ***@***.***> wrote: Seem like this PR is not enough to prove all trait associated types for [T] impl for std::ops::Index ? impl<T, I> ops::Index<I> for [T] where I: SliceIndex<[T]> { type Output = I::Output; #[inline] fn index(&self, index: I) -> &I::Output { index.index(self) } } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2592>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABTDKPQVZRGNMHQWQVHCUTQZMDGNANCNFSM4J427GCA> .

[kiljacken]

As for the issue with the example above, the problem is two-fold:

1. Some amount of autoderef is needed on the type being indexed, but I never quite managed to figure out how that works. @matklad / @flodiebold are one of you able to give a short intro to how our autoderef stuff works?
2. Index is only implemented for slices, but arrays with size coerces to slices in rust, so we need to handle that coercion.

[kiljacken]

Also just thought of the following: Once this is in place it can be trivially copied and adapted to handle custom BinOp implementations!
Maybe it would make sense to handle indexing as a binary op in hir? They seems to need the same kind of autoderef and inference handling.

[matklad]

r? @flodiebold

Maybe it would make sense to handle indexing as a binary op in hir?

See #1694 (comment). I think index is different from other binops in this case, but I might be mistaken.

[kiljacken]

See #1694 (comment). I think index is different from other binops in this case, but I might be mistaken.

Ahh you've had the same thought already :) Right, that conclusion does make some sense, keeping it seperate seems like it will remove a layer of enum matching when handling &mut on index expressions at some point.

[edwin0cheng]

As for the issue with the example above, the problem is two-fold:

1. Some amount of autoderef is needed on the type being indexed, but I never quite managed to figure out how that works. @matklad / @flodiebold are one of you able to give a short intro to how our autoderef stuff works?

2. Index is only implemented for slices, but arrays with size coerces to slices in rust, so we need to handle that coercion.

@kiljacken And I found that the following case do not work too and I don't know how to add that IndexWithOutput associate type information to the solver...

struct Bar;
struct Foo;

trait IndexWithOutput {
    type Output;
}

impl IndexWithOutput for u32 {
    type Output = Foo;
}

impl<T:IndexWithOutput> std::ops::Index<T> for Bar {
    type Output = T::Output;
}

fn test() {
    let a = Bar;
    let b = a[1];
    b<|>;
}

[flodiebold]

And I found that the following case do not work too and I don't know how to add that IndexWithOutput associate type information to the solver...

Without investigating in detail, I'd suspect rust-lang/chalk#234 for this. As I mentioned in #2454, that bug sadly prevents associated type normalization from working in a lot of non-trivial cases. Of course it could very well be something else, but I'm currently a bit reluctant to investigate associated type issues deeply because of it...

Some amount of autoderef is needed on the type being indexed, but I never quite managed to figure out how that works. @matklad / @flodiebold are one of you able to give a short intro to how our autoderef stuff works?

Hmm, actually this probably also does autoref, right? Should it be handled like method resolution? (It took quite a bit of care to get the order in which candidates are considered right there...) I haven't actually checked how rustc handles this... (does it lower to a method call, or just share method resolution code, or do something completely separate?)

As for autoderef, basically you just call autoderef and get an iterator of successively derefed types. The only complication is that you have to canonicalize the type (which basically means getting rid of inference variables so we can properly cache the trait queries) and pass the correct environment (i.e. set of where clauses). It should work exactly the same as e.g. for field accesses though.

[flodiebold]

LGTM

[flodiebold]

@kiljacken And I found that the following case do not work too and I don't know how to add that IndexWithOutput associate type information to the solver...

Actually, this might be a really complicated case... The 1 is an unresolved integer type variable when we're trying to infer the slice operation, so we'd need to somehow infer it's usize, but I don't really know how either Chalk or we would 🤔

[kiljacken]

Actually, this might be a really complicated case... The 1 is an unresolved integer type variable when we're trying to infer the slice operation, so we'd need to somehow infer it's usize, but I don't really know how either Chalk or we would thinking

I just added it as an expectation on the infer of the inner expr in my initial attempt at this, that seemed to do the trick, although I'm not quite sure if it was "correct".

[flodiebold]

I just added it as an expectation on the infer of the inner expr in my initial attempt at this, that seemed to do the trick, although I'm not quite sure if it was "correct".

If you mean adding an expectation of usize, I don't think that works -- e.g. in @edwin0cheng's example rustc infers u32: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=42a900cdc14d79df4e51c4b2033c3441

[kiljacken]

If you mean adding an expectation of usize, I don't think that works -- e.g. in @edwin0cheng's example rustc infers u32: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=42a900cdc14d79df4e51c4b2033c3441

Oh right, hadn't considered that. That does indeed seem like a great can of worms

[edwin0cheng]

I just checked, it do not works even without usize:

struct Bar;
struct Foo;
struct Baz;

trait IndexWithOutput {
    type Output;
}

impl IndexWithOutput for Baz {
    type Output = Foo;
}

impl<T:IndexWithOutput> std::ops::Index<T> for Bar {
    type Output = T::Output;
    
    fn index(&self, _index: T) -> &Self::Output { unimplemented!() }    
}

fn main() {
    let a = Bar;
    let c = Baz;
    let b = a[c];  // b is unknown
}

[flodiebold]

Yeah, I don't think we'll get this working right now, but we should still merge this. Although I guess we could detect when the parameter is an int variable and try using usize in that case... 🤔

[edwin0cheng]

@flodiebold, indeed. I just opened another issue to tracking part of it.

[kiljacken]

The librustc_typeck source seems to point some light on it being a bit different from method handling, and quite an ordeal!

[flodiebold]

@edwin0cheng I'd be fine with r+ing this, unless you first want to try getting autoderef working?

[edwin0cheng]

@edwin0cheng I'd be fine with r+ing this, unless you first want to try getting autoderef working?

@flodiebold Um.. I want to wait for rust-lang/chalk#234 to be fixed before implementing autodref working.

[edwin0cheng]

bors r+

[bors]

Build succeeded

• Rust
• TypeScript