about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2018-08-09 22:05:18 +0000
committerbors <bors@rust-lang.org>2018-08-09 22:05:18 +0000
commitfb65d7563c93509c343a98e5cccf8e5ab4506924 (patch)
tree2694839888f65ed4f411cd8701def0275c2ca280
parent8958ed672298148841b3b8d6371ce301e1cbbac1 (diff)
parent24abef3689840ec2ad0bb6ccdbc7cbcfb3844a82 (diff)
downloadrust-fb65d7563c93509c343a98e5cccf8e5ab4506924.tar.gz
rust-fb65d7563c93509c343a98e5cccf8e5ab4506924.zip
Auto merge of #52788 - LukasKalbertodt:improve-index-mut-error, r=estebank
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?
-rw-r--r--src/librustc_borrowck/borrowck/mod.rs23
-rw-r--r--src/test/ui/borrowck/index-mut-help.nll.stderr22
-rw-r--r--src/test/ui/borrowck/index-mut-help.rs24
-rw-r--r--src/test/ui/borrowck/index-mut-help.stderr28
-rw-r--r--src/test/ui/issue-41726.stderr2
5 files changed, 99 insertions, 0 deletions
diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs
index aef8b58fa8b..a97fc666e25 100644
--- a/src/librustc_borrowck/borrowck/mod.rs
+++ b/src/librustc_borrowck/borrowck/mod.rs
@@ -898,6 +898,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
                                 }
                             }
                         }
+
                         db
                     }
                     BorrowViolation(euv::ClosureCapture(_)) => {
@@ -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,
diff --git a/src/test/ui/borrowck/index-mut-help.nll.stderr b/src/test/ui/borrowck/index-mut-help.nll.stderr
new file mode 100644
index 00000000000..cc058f1fde5
--- /dev/null
+++ b/src/test/ui/borrowck/index-mut-help.nll.stderr
@@ -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`.
diff --git a/src/test/ui/borrowck/index-mut-help.rs b/src/test/ui/borrowck/index-mut-help.rs
new file mode 100644
index 00000000000..a4df0ced2f2
--- /dev/null
+++ b/src/test/ui/borrowck/index-mut-help.rs
@@ -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
+}
diff --git a/src/test/ui/borrowck/index-mut-help.stderr b/src/test/ui/borrowck/index-mut-help.stderr
new file mode 100644
index 00000000000..b8b35ed3ed3
--- /dev/null
+++ b/src/test/ui/borrowck/index-mut-help.stderr
@@ -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`.
diff --git a/src/test/ui/issue-41726.stderr b/src/test/ui/issue-41726.stderr
index 172309537bf..c79196e0140 100644
--- a/src/test/ui/issue-41726.stderr
+++ b/src/test/ui/issue-41726.stderr
@@ -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