Note when a mutable trait object is needed

Author: estebank

src/librustc/traits/error_reporting.rs

@@ -453,21 +453,17 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         }
     }
 
-    fn find_similar_impl_candidates(&self,
-                                    trait_ref: ty::PolyTraitRef<'tcx>)
-                                    -> Vec<ty::TraitRef<'tcx>>
-    {
-        let simp = fast_reject::simplify_type(self.tcx,
-                                              trait_ref.skip_binder().self_ty(),
-                                              true);
+    fn find_similar_impl_candidates(
+        &self,
+        trait_ref: ty::PolyTraitRef<'tcx>,
+    ) -> Vec<ty::TraitRef<'tcx>> {
+        let simp = fast_reject::simplify_type(self.tcx, trait_ref.skip_binder().self_ty(), true);
         let all_impls = self.tcx.all_impls(trait_ref.def_id());
 
         match simp {
             Some(simp) => all_impls.iter().filter_map(|&def_id| {
                 let imp = self.tcx.impl_trait_ref(def_id).unwrap();
-                let imp_simp = fast_reject::simplify_type(self.tcx,
-                                                          imp.self_ty(),
-                                                          true);
+                let imp_simp = fast_reject::simplify_type(self.tcx, imp.self_ty(), true);
                 if let Some(imp_simp) = imp_simp {
                     if simp != imp_simp {
                         return None
@@ -482,10 +478,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         }
     }
 
-    fn report_similar_impl_candidates(&self,
-                                      impl_candidates: Vec<ty::TraitRef<'tcx>>,
-                                      err: &mut DiagnosticBuilder<'_>)
-    {
+    fn report_similar_impl_candidates(
+        &self,
+        impl_candidates: Vec<ty::TraitRef<'tcx>>,
+        err: &mut DiagnosticBuilder<'_>,
+    ) {
         if impl_candidates.is_empty() {
             return;
         }
@@ -720,10 +717,18 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                             // which is somewhat confusing.
                             err.help(&format!("consider adding a `where {}` bound",
                                               trait_ref.to_predicate()));
-                        } else if !have_alt_message {
-                            // Can't show anything else useful, try to find similar impls.
-                            let impl_candidates = self.find_similar_impl_candidates(trait_ref);
-                            self.report_similar_impl_candidates(impl_candidates, &mut err);
+                        } else {
+                            if !have_alt_message {
+                                // Can't show anything else useful, try to find similar impls.
+                                let impl_candidates = self.find_similar_impl_candidates(trait_ref);
+                                self.report_similar_impl_candidates(impl_candidates, &mut err);
+                            }
+                            self.suggest_change_mut(
+                                &obligation,
+                                &mut err,
+                                &trait_ref,
+                                points_at_arg,
+                            );
                         }
 
                         // If this error is due to `!: Trait` not implemented but `(): Trait` is
@@ -1081,9 +1086,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
 
                     let substs = self.tcx.mk_substs_trait(trait_type, &[]);
                     let new_trait_ref = ty::TraitRef::new(trait_ref.def_id, substs);
-                    let new_obligation = Obligation::new(ObligationCause::dummy(),
-                                                         obligation.param_env,
-                                                         new_trait_ref.to_predicate());
+                    let new_obligation = Obligation::new(
+                        ObligationCause::dummy(),
+                        obligation.param_env,
+                        new_trait_ref.to_predicate(),
+                    );
 
                     if self.predicate_may_hold(&new_obligation) {
                         let sp = self.tcx.sess.source_map()
@@ -1105,6 +1112,68 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         }
     }
 
+    /// Check if the trait bound is implemented for a different mutability and note it in the
+    /// final error.
+    fn suggest_change_mut(
+        &self,
+        obligation: &PredicateObligation<'tcx>,
+        err: &mut DiagnosticBuilder<'tcx>,
+        trait_ref: &ty::Binder<ty::TraitRef<'tcx>>,
+        points_at_arg: bool,
+    ) {
+        let span = obligation.cause.span;
+        if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
+            let refs_number = snippet.chars()
+                .filter(|c| !c.is_whitespace())
+                .take_while(|c| *c == '&')
+                .count();
+            if let Some('\'') = snippet.chars()
+                .filter(|c| !c.is_whitespace())
+                .skip(refs_number)
+                .next()
+            { // Do not suggest removal of borrow from type arguments.
+                return;
+            }
+
+            if let ty::Ref(region, t_type, mutability) = trait_ref.skip_binder().self_ty().kind {
+                let trait_type = match mutability {
+                    hir::Mutability::MutMutable => self.tcx.mk_imm_ref(region, t_type),
+                    hir::Mutability::MutImmutable => self.tcx.mk_mut_ref(region, t_type),
+                };
+
+                let substs = self.tcx.mk_substs_trait(&trait_type, &[]);
+                let new_trait_ref = ty::TraitRef::new(trait_ref.skip_binder().def_id, substs);
+                let new_obligation = Obligation::new(
+                    ObligationCause::dummy(),
+                    obligation.param_env,
+                    new_trait_ref.to_predicate(),
+                );
+
+                if self.predicate_may_hold(&new_obligation) {
+                    let sp = self.tcx.sess.source_map()
+                        .span_take_while(span, |c| c.is_whitespace() || *c == '&');
+                    if points_at_arg &&
+                        mutability == hir::Mutability::MutImmutable &&
+                        refs_number > 0
+                    {
+                        err.span_suggestion(
+                            sp,
+                            "consider changing this borrow's mutability",
+                            "&mut ".to_string(),
+                            Applicability::MachineApplicable,
+                        );
+                    } else {
+                        err.note(&format!(
+                            "`{}` is implemented for `{:?}`",
+                            trait_ref,
+                            trait_type,
+                        ));
+                    }
+                }
+            }
+        }
+    }
+
     fn suggest_semicolon_removal(
         &self,
         obligation: &PredicateObligation<'tcx>,

src/librustc_typeck/check/method/mod.rs

@@ -58,7 +58,7 @@ pub enum MethodError<'tcx> {
 
     // Found a `Self: Sized` bound where `Self` is a trait object, also the caller may have
     // forgotten to import a trait.
-    IllegalSizedBound(Vec<DefId>),
+    IllegalSizedBound(Vec<DefId>, bool),
 
     // Found a match, but the return type is wrong
     BadReturnType,
@@ -213,33 +213,49 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             segment,
         );
 
+        let mut needs_mut = false;
+        if let ty::Ref(region, t_type, mutability) = self_ty.kind {
+            let trait_type = match mutability {
+                hir::Mutability::MutMutable => self.tcx.mk_imm_ref(region, t_type),
+                hir::Mutability::MutImmutable => self.tcx.mk_mut_ref(region, t_type),
+            };
+            match self.lookup_probe(
+                span,
+                segment.ident,
+                trait_type,
+                call_expr,
+                ProbeScope::TraitsInScope
+            ) {
+                Ok(ref new_pick) if *new_pick != pick => {
+                    needs_mut = true;
+                }
+                _ => {}
+            }
+        }
+
         if result.illegal_sized_bound {
             // We probe again, taking all traits into account (not only those in scope).
-            let candidates =
-                match self.lookup_probe(span,
-                                        segment.ident,
-                                        self_ty,
-                                        call_expr,
-                                        ProbeScope::AllTraits) {
-
-                    // If we find a different result the caller probably forgot to import a trait.
-                    Ok(ref new_pick) if *new_pick != pick => vec![new_pick.item.container.id()],
-                    Err(Ambiguity(ref sources)) => {
-                        sources.iter()
-                               .filter_map(|source| {
-                                   match *source {
-                                       // Note: this cannot come from an inherent impl,
-                                       // because the first probing succeeded.
-                                       ImplSource(def) => self.tcx.trait_id_of_impl(def),
-                                       TraitSource(_) => None,
-                                   }
-                               })
-                               .collect()
+            let candidates = match self.lookup_probe(
+                span,
+                segment.ident,
+                self_ty,
+                call_expr,
+                ProbeScope::AllTraits,
+            ) {
+                // If we find a different result the caller probably forgot to import a trait.
+                Ok(ref new_pick) if *new_pick != pick => vec![new_pick.item.container.id()],
+                Err(Ambiguity(ref sources)) => sources.iter().filter_map(|source| {
+                    match *source {
+                        // Note: this cannot come from an inherent impl,
+                        // because the first probing succeeded.
+                        ImplSource(def) => self.tcx.trait_id_of_impl(def),
+                        TraitSource(_) => None,
                     }
-                    _ => Vec::new(),
-                };
+                }).collect(),
+                _ => Vec::new(),
+            };
 
-            return Err(IllegalSizedBound(candidates));
+            return Err(IllegalSizedBound(candidates, needs_mut));
         }
 
         Ok(result.callee)

src/librustc_typeck/check/method/suggest.rs

@@ -553,7 +553,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 err.emit();
             }
 
-            MethodError::IllegalSizedBound(candidates) => {
+            MethodError::IllegalSizedBound(candidates, needs_mut) => {
                 let msg = format!("the `{}` method cannot be invoked on a trait object", item_name);
                 let mut err = self.sess().struct_span_err(span, &msg);
                 if !candidates.is_empty() {
@@ -569,6 +569,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                     });
                     self.suggest_use_candidates(&mut err, help, candidates);
                 }
+                if let ty::Ref(region, t_type, mutability) = rcvr_ty.kind {
+                    let trait_type = match mutability {
+                        hir::Mutability::MutMutable => self.tcx.mk_imm_ref(region, t_type),
+                        hir::Mutability::MutImmutable => self.tcx.mk_mut_ref(region, t_type),
+                    };
+                    if needs_mut {
+                        err.note(&format!("you need `{}` instead", trait_type));
+                    }
+                }
                 err.emit();
             }
 

src/test/ui/not-panic/not-panic-safe.stderr

@@ -8,6 +8,7 @@ LL |     assert::<&mut i32>();
    |     ^^^^^^^^^^^^^^^^^^ `&mut i32` may not be safely transferred across an unwind boundary
    |
    = help: the trait `std::panic::UnwindSafe` is not implemented for `&mut i32`
+   = note: `std::panic::UnwindSafe` is implemented for `&i32`
 
 error: aborting due to previous error
 

src/test/ui/parser/lex-bad-char-literals-6.stderr

@@ -35,6 +35,7 @@ LL |     if x == y {}
    |          ^^ no implementation for `&str == char`
    |
    = help: the trait `std::cmp::PartialEq<char>` is not implemented for `&str`
+   = note: `std::cmp::PartialEq<char>` is implemented for `&mut str`
 
 error[E0308]: mismatched types
   --> $DIR/lex-bad-char-literals-6.rs:15:20
@@ -52,6 +53,7 @@ LL |     if x == z {}
    |          ^^ no implementation for `&str == char`
    |
    = help: the trait `std::cmp::PartialEq<char>` is not implemented for `&str`
+   = note: `std::cmp::PartialEq<char>` is implemented for `&mut str`
 
 error: aborting due to 6 previous errors
 

src/test/ui/suggestions/imm-ref-trait-object-literal.rs

@@ -0,0 +1,14 @@
+trait Trait {}
+
+struct S;
+
+impl<'a> Trait for &'a mut S {}
+
+fn foo<X: Trait>(_: X) {}
+
+
+fn main() {
+  let s = S;
+  foo(&s); //~ ERROR the trait bound `&S: Trait` is not satisfied
+  foo(s); //~ ERROR the trait bound `S: Trait` is not satisfied
+}

src/test/ui/suggestions/imm-ref-trait-object-literal.stderr

@@ -0,0 +1,30 @@
+error[E0277]: the trait bound `&S: Trait` is not satisfied
+  --> $DIR/imm-ref-trait-object-literal.rs:12:7
+   |
+LL | fn foo<X: Trait>(_: X) {}
+   |    ---    ----- required by this bound in `foo`
+...
+LL |   foo(&s);
+   |       -^
+   |       |
+   |       the trait `Trait` is not implemented for `&S`
+   |       help: consider changing this borrow's mutability: `&mut`
+   |
+   = help: the following implementations were found:
+             <&'a mut S as Trait>
+
+error[E0277]: the trait bound `S: Trait` is not satisfied
+  --> $DIR/imm-ref-trait-object-literal.rs:13:7
+   |
+LL | fn foo<X: Trait>(_: X) {}
+   |    ---    ----- required by this bound in `foo`
+...
+LL |   foo(s);
+   |       ^ the trait `Trait` is not implemented for `S`
+   |
+   = help: the following implementations were found:
+             <&'a mut S as Trait>
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/suggestions/imm-ref-trait-object.rs

@@ -0,0 +1,8 @@
+fn test(t: &dyn Iterator<Item=&u64>) -> u64 {
+     t.min().unwrap() //~ ERROR the `min` method cannot be invoked on a trait object
+}
+
+fn main() {
+     let array = [0u64];
+     test(&mut array.iter());
+}

src/test/ui/suggestions/imm-ref-trait-object.stderr

@@ -0,0 +1,10 @@
+error: the `min` method cannot be invoked on a trait object
+  --> $DIR/imm-ref-trait-object.rs:2:8
+   |
+LL |      t.min().unwrap()
+   |        ^^^
+   |
+   = note: you need `&mut dyn std::iter::Iterator<Item = &u64>` instead
+
+error: aborting due to previous error
+

src/test/ui/suggestions/into-str.stderr

@@ -8,6 +8,7 @@ LL |     foo(String::new());
    |     ^^^ the trait `std::convert::From<std::string::String>` is not implemented for `&str`
    |
    = note: to coerce a `std::string::String` into a `&str`, use `&*` as a prefix
+   = note: `std::convert::From<std::string::String>` is implemented for `&mut str`
    = note: required because of the requirements on the impl of `std::convert::Into<&str>` for `std::string::String`
 
 error: aborting due to previous error

src/test/ui/suggestions/mut-borrow-needed-by-trait.rs

@@ -0,0 +1,23 @@
+use std::env::args;
+use std::fs::File;
+use std::io::{stdout, Write, BufWriter};
+
+fn main() {
+    let mut args = args();
+    let _ = args.next();
+    let dest = args.next();
+
+    let h1; let h2; let h3;
+
+    let fp: &dyn Write = match dest {
+        Some(path) => { h1 = File::create(path).unwrap(); &h1 },
+        None => { h2 = stdout(); h3 = h2.lock(); &h3 }
+    };
+
+    let fp = BufWriter::new(fp);
+    //~^ ERROR the trait bound `&dyn std::io::Write: std::io::Write` is not satisfied
+    //~| ERROR the trait bound `&dyn std::io::Write: std::io::Write` is not satisfied
+    //~| ERROR the trait bound `&dyn std::io::Write: std::io::Write` is not satisfied
+
+    writeln!(fp, "hello world").unwrap(); //~ ERROR no method named `write_fmt` found for type
+}