Skip to content
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

Deref not applying to things that implement Index #16821

Closed
steveklabnik opened this issue Aug 28, 2014 · 14 comments · Fixed by #18486
Closed

Deref not applying to things that implement Index #16821

steveklabnik opened this issue Aug 28, 2014 · 14 comments · Fixed by #18486
Labels
P-medium Medium priority

Comments

@steveklabnik
Copy link
Member

This should compile, but doesn't:

use std::rc::Rc;

fn main() {
    let vec = Rc::new(vec![1i, 2, 3]);
    println!("{}", vec[0]);
}
This was referenced Aug 28, 2014
@bluss
Copy link
Member

bluss commented Aug 28, 2014

Same with box instead of Rc.

@brson
Copy link
Contributor

brson commented Sep 8, 2014

Nominated. @pcwalton What's the intended behavior here?

@nrc
Copy link
Member

nrc commented Sep 8, 2014

cc me

@nrc
Copy link
Member

nrc commented Sep 9, 2014

I would expect this to work. When we check to see if we can index we should search for the Index trait the same way we do when explicitly writing e.index(...), which would include applying implicit derefs, including custom ones.

It is backwards compatible though, so shouldn't need to block 1.0.

@pcwalton
Copy link
Contributor

I think this is not a 1.0 issue.

@steveklabnik
Copy link
Member Author

What's the reason this was removed? It's true that it's not backwards compatible, but it leads to really unfortunate code.

@aturon
Copy link
Member

aturon commented Sep 10, 2014

@steveklabnik The nomination hasn't been removed -- we'll discuss this at tomorrow's triage meeting to determine whether to put it on the 1.0 milestone. At that point we'll remove the nomination tag and replace it with priorities/milestones.

FWIW, I agree that this is really unfortunate as it stands. But the question for the milestone is: would we hold back the 1.0 release until it was fixed? The answer can be "no" even for very high priority items.

I do think that @nikomatsakis's work on method resolution, as part of trait reform, is likely to fix this well before 1.0.

@steveklabnik
Copy link
Member Author

Oh yeah, and honestly, I'm willing to make sacrifices for a 1.0, I guess
what I mean is "I don't know how much work this actually is to fix."

@pnkfelix
Copy link
Member

P-high, not a 1.0 blocker.

@pnkfelix pnkfelix added P-medium Medium priority and removed I-nominated labels Sep 11, 2014
@nikomatsakis
Copy link
Contributor

I think this'll probably wind up being fixed as fallout from various changes.

@steveklabnik
Copy link
Member Author

Oh, @aturon , when I said 'was removed,' I wasn't talking about the nomination, I was asking how the regression in behaviour happened.

@ftxqxd
Copy link
Contributor

ftxqxd commented Sep 27, 2014

Is this a dupe of #15757?

@nikomatsakis
Copy link
Contributor

This would be fixed by #18486

bors added a commit that referenced this issue Nov 5, 2014
This branch cleans up overloaded operator resolution so that it is strictly based on the traits in `ops`, rather than going through the normal method lookup mechanism. It also adds full support for autoderef to overloaded index (whereas before autoderef only worked for non-overloaded index) as well as for the slicing operators.

This is a [breaking-change]: in the past, we were accepting combinations of operands that were not intended to be accepted. For example, it was possible to compare a fixed-length array and a slice, or apply the `!` operator to a `&int`. See the first two commits in this pull-request for examples.

One downside of this change is that comparing fixed-length arrays doesn't always work as smoothly as it did before. Before this, comparisons sometimes worked due to various coercions to slices. I've added impls for `Eq`, `Ord`, etc for fixed-lengths arrays up to and including length 32, but if the array is longer than that you'll need to either newtype the array or convert to slices. Note that this plays better with deriving in any case than the previous scheme.

Fixes #4920.
Fixes #16821.
Fixes #15757.

cc @alexcrichton 
cc @aturon
@chenzhekl
Copy link

chenzhekl commented Apr 19, 2019

It seems that when Deref<Target=B> and Index<(usize, usize)> are implemented on A, and B implements Index<usize>, Index<usize> will not be accessible from A. Is it expected behavior?

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=1a9f6cee41a9d1c9d67b852246e4bc49

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants