Skip to content

Commit

Permalink
Auto merge of #52788 - LukasKalbertodt:improve-index-mut-error, r=est…
Browse files Browse the repository at this point in the history
…ebank

Add help message for missing `IndexMut` impl

Code:
```rust
let mut map = HashMap::new();
map.insert("peter", 23);
map["peter"] = 27;
```

Before:
```
error[E0594]: cannot assign to immutable indexed content
 --> src/main.rs:7:5
  |
7 |     map["peter"] = 27;
  |     ^^^^^^^^^^^^^^^^^ cannot borrow as mutable
```

With this change (just the `help` was added):
```
error[E0594]: cannot assign to immutable indexed content
 --> index-error.rs:7:5
  |
7 |     map["peter"] = 27;
  |     ^^^^^^^^^^^^^^^^^ cannot borrow as mutable
  |
  = help: trait `IndexMut` is required to modify indexed content, but it is not implemented for std::collections::HashMap<&str, i32>
```

---

Yesterday I did some pair programming with a Rust-beginner. We created a type and implemented `Index` for it. Trying to modify the value returned by the index operation returns in a rather vague error that was not very clear for the Rust beginner. So I tried to improve the situation.

## Notes/questions for reviewers:
- Is the formulation OK like that? I'm fine with changing it.
- Can we be absolutely sure that `IndexMut` is actually not implemented in the case my `help` message is added? I'm fairly sure myself, but there could be some cases I didn't think of. Also, I don't know the compiler very well, so I don't know what exactly certain enum variants are used for.
  - It would be nice to test if `IndexMut` is in fact not implemented for the type, but I couldn't figure out how to check that. If you think that additional check would be beneficial, could you tell me how to check if a trait is implemented?
- Do you think I should change the error message instead of only adding an additional help message?
  • Loading branch information
bors committed Aug 9, 2018
2 parents 8958ed6 + 24abef3 commit fb65d75
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 0 deletions.
23 changes: 23 additions & 0 deletions src/librustc_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
}
}
}

db
}
BorrowViolation(euv::ClosureCapture(_)) => {
Expand All @@ -918,6 +919,28 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
}
};

// We add a special note about `IndexMut`, if the source of this error
// is the fact that `Index` is implemented, but `IndexMut` is not. Needing
// to implement two traits for "one operator" is not very intuitive for
// many programmers.
if err.cmt.note == mc::NoteIndex {
let node_id = self.tcx.hir.hir_to_node_id(err.cmt.hir_id);
let node = self.tcx.hir.get(node_id);

// This pattern probably always matches.
if let hir_map::NodeExpr(
hir::Expr { node: hir::ExprKind::Index(lhs, _), ..}
) = node {
let ty = self.tables.expr_ty(lhs);

db.help(&format!(
"trait `IndexMut` is required to modify indexed content, but \
it is not implemented for `{}`",
ty
));
}
}

self.note_and_explain_mutbl_error(&mut db, &err, &error_span);
self.note_immutability_blame(
&mut db,
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/borrowck/index-mut-help.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0596]: cannot borrow data in a `&` reference as mutable
--> $DIR/index-mut-help.rs:21:5
|
LL | map["peter"].clear(); //~ ERROR
| ^^^^^^^^^^^^ cannot borrow as mutable

error[E0594]: cannot assign to data in a `&` reference
--> $DIR/index-mut-help.rs:22:5
|
LL | map["peter"] = "0".to_string(); //~ ERROR
| ^^^^^^^^^^^^ cannot assign

error[E0596]: cannot borrow data in a `&` reference as mutable
--> $DIR/index-mut-help.rs:23:13
|
LL | let _ = &mut map["peter"]; //~ ERROR
| ^^^^^^^^^^^^^^^^^ cannot borrow as mutable

error: aborting due to 3 previous errors

Some errors occurred: E0594, E0596.
For more information about an error, try `rustc --explain E0594`.
24 changes: 24 additions & 0 deletions src/test/ui/borrowck/index-mut-help.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// When mutably indexing a type that implements `Index` but not `IndexMut`, a
// special 'help' message is added to the output.


fn main() {
use std::collections::HashMap;

let mut map = HashMap::new();
map.insert("peter", "23".to_string());

map["peter"].clear(); //~ ERROR
map["peter"] = "0".to_string(); //~ ERROR
let _ = &mut map["peter"]; //~ ERROR
}
28 changes: 28 additions & 0 deletions src/test/ui/borrowck/index-mut-help.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
error[E0596]: cannot borrow immutable indexed content as mutable
--> $DIR/index-mut-help.rs:21:5
|
LL | map["peter"].clear(); //~ ERROR
| ^^^^^^^^^^^^ cannot borrow as mutable
|
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `std::collections::HashMap<&str, std::string::String>`

error[E0594]: cannot assign to immutable indexed content
--> $DIR/index-mut-help.rs:22:5
|
LL | map["peter"] = "0".to_string(); //~ ERROR
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
|
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `std::collections::HashMap<&str, std::string::String>`

error[E0596]: cannot borrow immutable indexed content as mutable
--> $DIR/index-mut-help.rs:23:18
|
LL | let _ = &mut map["peter"]; //~ ERROR
| ^^^^^^^^^^^^ cannot borrow as mutable
|
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `std::collections::HashMap<&str, std::string::String>`

error: aborting due to 3 previous errors

Some errors occurred: E0594, E0596.
For more information about an error, try `rustc --explain E0594`.
2 changes: 2 additions & 0 deletions src/test/ui/issue-41726.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0596]: cannot borrow immutable indexed content as mutable
|
LL | things[src.as_str()].sort(); //~ ERROR cannot borrow immutable
| ^^^^^^^^^^^^^^^^^^^^ cannot borrow as mutable
|
= help: trait `IndexMut` is required to modify indexed content, but it is not implemented for `std::collections::HashMap<std::string::String, std::vec::Vec<std::string::String>>`

error: aborting due to previous error

Expand Down

0 comments on commit fb65d75

Please sign in to comment.