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

Index chosen over IndexMut when coercing index type #72002

Closed
ramosbugs opened this issue May 8, 2020 · 3 comments · Fixed by #72068
Closed

Index chosen over IndexMut when coercing index type #72002

ramosbugs opened this issue May 8, 2020 · 3 comments · Fixed by #72068
Labels
A-inference Area: Type inference A-trait-system Area: Trait system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ramosbugs
Copy link

We've run into an issue where we have a type that implements both Index<&str> and IndexMut<&str>. Passing an index value of &String causes rustc to choose Index instead of IndexMut, even if a mutable reference is required. If we first call String::as_str and use that as the index, IndexMut is chosen, as expected.

Example repro by @idubrov (see https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=9f0e5d703465d4b87218cac8b1eea608):

struct Indexable;

impl Indexable {
    fn boo(&mut self) {
    }
}

impl std::ops::Index<&str> for Indexable {
    type Output = Indexable;
    
    fn index(&self, field: &str) -> &Indexable {
        self
    }
}

impl std::ops::IndexMut<&str> for Indexable {
    fn index_mut(&mut self, field: &str) -> &mut Indexable {
        self
    }
}

fn main() {
    let mut v = Indexable;
    let field = "hello".to_string();

    // Works    
    v[field.as_str()].boo();
    
    // Doesn't work    
    v[&field].boo();
}

I expected to see this happen: v[&field] should invoke IndexMut on v since boo() expects a mutable reference.

Instead, this happened: v[&field] invoked Index and complained that IndexMut isn't implemented even though it is:

error[E0596]: cannot borrow data in an index of `Indexable` as mutable
  --> src/main.rs:30:5
   |
30 |     v[&field].boo();
   |     ^^^^^^^^^ cannot borrow as mutable
   |
   = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `Indexable`

Meta

This is reproducible on stable Rust 1.41.0 and 1.43.0 and 1.45.0-nightly (2020-05-06 1836e3b42a5b2f37fd79).

cc: @estebank

@ramosbugs ramosbugs added the C-bug Category: This is a bug. label May 8, 2020
@estebank estebank added A-inference Area: Type inference T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 8, 2020
@jonas-schievink jonas-schievink added the A-trait-system Area: Trait system label May 8, 2020
@estebank
Copy link
Contributor

I have a hacky solution to this in #72068. Looking this code up in godbolt I can reproduce it all the way to 1.0, earlier versions giving a type error (&str vs &String), then an ICE in the 1.2x (that I was causing during my experiments to fix this: error: internal compiler error: re-trying op failed) and finally the current error. The problem is caused (somehow) in convert_place_op_to_mutable slightly differing from try_index_step, but haven't figured how to fix that underlying difference.

@timotree3
Copy link
Contributor

In case this helps to find the root problem, #65682 gives a similar error: "IndexMut is not implemented" even though it really is.

@estebank
Copy link
Contributor

It seems closely related but #72068 sadly doesn't change the behavior on that ticket. I'll take a look at it though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-inference Area: Type inference A-trait-system Area: Trait system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants