about summary refs log tree commit diff
diff options
context:
space:
mode:
authorDavid Wood <david@davidtw.co>2019-04-22 10:16:47 +0100
committerDavid Wood <david@davidtw.co>2019-04-22 19:26:24 +0100
commit7ab1bfd692c2679278c5dc978b04dfb16dfae470 (patch)
tree813d67fa414a65a40f2acb39da5e458d04b1d425
parentfd95ba357465e39386924e40e05efef715a4ad46 (diff)
downloadrust-7ab1bfd692c2679278c5dc978b04dfb16dfae470.tar.gz
rust-7ab1bfd692c2679278c5dc978b04dfb16dfae470.zip
Only make suggestion when type is `Copy`.
This commit makes the suggestion to dereference when a type implements
`Deref` only apply if the dereference would succeed (ie. the type is
`Copy`, otherwise a borrow check error would occur).
-rw-r--r--src/librustc_typeck/check/demand.rs105
-rw-r--r--src/test/ui/infinite/infinite-autoderef.stderr2
-rw-r--r--src/test/ui/occurs-check-2.stderr2
-rw-r--r--src/test/ui/occurs-check.stderr2
-rw-r--r--src/test/ui/span/coerce-suggestions.stderr2
-rw-r--r--src/test/ui/suggestions/issue-59819.fixed15
-rw-r--r--src/test/ui/suggestions/issue-59819.rs15
-rw-r--r--src/test/ui/suggestions/issue-59819.stderr18
-rw-r--r--src/test/ui/terr-sorts.stderr5
9 files changed, 91 insertions, 75 deletions
diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs
index 648a48d8907..6ce03025bf2 100644
--- a/src/librustc_typeck/check/demand.rs
+++ b/src/librustc_typeck/check/demand.rs
@@ -324,8 +324,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             sp,
         );
 
-        match (&expected.sty, &checked_ty.sty) {
-            (&ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.sty, &check.sty) {
+        // Check the `expn_info()` to see if this is a macro; if so, it's hard to
+        // extract the text and make a good suggestion, so don't bother.
+        let is_macro = sp.ctxt().outer().expn_info().is_some();
+
+        match (&expr.node, &expected.sty, &checked_ty.sty) {
+            (_, &ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.sty, &check.sty) {
                 (&ty::Str, &ty::Array(arr, _)) |
                 (&ty::Str, &ty::Slice(arr)) if arr == self.tcx.types.u8 => {
                     if let hir::ExprKind::Lit(_) = expr.node {
@@ -352,7 +356,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 }
                 _ => {}
             },
-            (&ty::Ref(_, _, mutability), _) => {
+            (_, &ty::Ref(_, _, mutability), _) => {
                 // Check if it can work when put into a ref. For example:
                 //
                 // ```
@@ -407,65 +411,31 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                         });
                     }
                 }
-            }
-            (_, &ty::Ref(_, checked, _)) => {
+            },
+            (hir::ExprKind::AddrOf(_, ref expr), _, &ty::Ref(_, checked, _)) if {
+                self.infcx.can_sub(self.param_env, checked, &expected).is_ok() && !is_macro
+            } => {
                 // We have `&T`, check if what was expected was `T`. If so,
-                // we may want to suggest adding a `*`, or removing
-                // a `&`.
-                //
-                // (But, also check the `expn_info()` to see if this is
-                // a macro; if so, it's hard to extract the text and make a good
-                // suggestion, so don't bother.)
-                if self.infcx.can_sub(self.param_env, checked, &expected).is_ok() &&
-                   sp.ctxt().outer().expn_info().is_none() {
-                    match expr.node {
-                        // Maybe remove `&`?
-                        hir::ExprKind::AddrOf(_, ref expr) => {
-                            if !cm.span_to_filename(expr.span).is_real() {
-                                if let Ok(code) = cm.span_to_snippet(sp) {
-                                    if code.chars().next() == Some('&') {
-                                        return Some((
-                                            sp,
-                                            "consider removing the borrow",
-                                            code[1..].to_string()),
-                                        );
-                                    }
-                                }
-                                return None;
-                            }
-                            if let Ok(code) = cm.span_to_snippet(expr.span) {
-                                return Some((sp, "consider removing the borrow", code));
-                            }
-                        }
-
-                        // Maybe add `*`? Only if `T: Copy`.
-                        _ => {
-                            if self.infcx.type_is_copy_modulo_regions(self.param_env,
-                                                                      checked,
-                                                                      sp) {
-                                // do not suggest if the span comes from a macro (#52783)
-                                if let (Ok(code), true) = (
-                                    cm.span_to_snippet(sp),
-                                    sp == expr.span,
-                                ) {
-                                    return Some((
-                                        sp,
-                                        "consider dereferencing the borrow",
-                                        if is_struct_pat_shorthand_field {
-                                            format!("{}: *{}", code, code)
-                                        } else {
-                                            format!("*{}", code)
-                                        },
-                                    ));
-                                }
-                            }
+                // we may want to suggest removing a `&`.
+                if !cm.span_to_filename(expr.span).is_real() {
+                    if let Ok(code) = cm.span_to_snippet(sp) {
+                        if code.chars().next() == Some('&') {
+                            return Some((
+                                sp,
+                                "consider removing the borrow",
+                                code[1..].to_string(),
+                            ));
                         }
                     }
+                    return None;
                 }
-            }
-            _ => {
-                // If neither type is a reference, then check for `Deref` implementations by
-                // constructing a predicate to prove: `<T as Deref>::Output == U`
+                if let Ok(code) = cm.span_to_snippet(expr.span) {
+                    return Some((sp, "consider removing the borrow", code));
+                }
+            },
+            _ if sp == expr.span && !is_macro => {
+                // Check for `Deref` implementations by constructing a predicate to
+                // prove: `<T as Deref>::Output == U`
                 let deref_trait = self.tcx.lang_items().deref_trait().unwrap();
                 let item_def_id = self.tcx.associated_items(deref_trait).next().unwrap().def_id;
                 let predicate = ty::Predicate::Projection(ty::Binder::bind(ty::ProjectionPredicate {
@@ -483,17 +453,28 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                     ty: expected,
                 }));
                 let obligation = traits::Obligation::new(self.misc(sp), self.param_env, predicate);
-                if self.infcx.predicate_may_hold(&obligation) {
-                    if let (Ok(code), true) = (cm.span_to_snippet(sp), sp == expr.span) {
-                        let msg = if is_struct_pat_shorthand_field {
+                let impls_deref = self.infcx.predicate_may_hold(&obligation);
+
+                // For a suggestion to make sense, the type would need to be `Copy`.
+                let is_copy = self.infcx.type_is_copy_modulo_regions(self.param_env, expected, sp);
+
+                if is_copy && impls_deref {
+                    if let Ok(code) = cm.span_to_snippet(sp) {
+                        let message = if checked_ty.is_region_ptr() {
+                            "consider dereferencing the borrow"
+                        } else {
+                            "consider dereferencing the type"
+                        };
+                        let suggestion = if is_struct_pat_shorthand_field {
                             format!("{}: *{}", code, code)
                         } else {
                             format!("*{}", code)
                         };
-                        return Some((sp, "consider dereferencing the type", msg));
+                        return Some((sp, message, suggestion));
                     }
                 }
             }
+            _ => {}
         }
         None
     }
diff --git a/src/test/ui/infinite/infinite-autoderef.stderr b/src/test/ui/infinite/infinite-autoderef.stderr
index 04c84bae2dc..a5cc66f4473 100644
--- a/src/test/ui/infinite/infinite-autoderef.stderr
+++ b/src/test/ui/infinite/infinite-autoderef.stderr
@@ -5,7 +5,7 @@ LL |         x = box x;
    |             ^^^^^
    |             |
    |             cyclic type of infinite size
-   |             help: consider dereferencing the type: `*box x`
+   |             help: try using a conversion method: `box x.to_string()`
 
 error[E0055]: reached the recursion limit while auto-dereferencing `Foo`
   --> $DIR/infinite-autoderef.rs:25:5
diff --git a/src/test/ui/occurs-check-2.stderr b/src/test/ui/occurs-check-2.stderr
index 7b475c3cb22..74e29a5aea7 100644
--- a/src/test/ui/occurs-check-2.stderr
+++ b/src/test/ui/occurs-check-2.stderr
@@ -5,7 +5,7 @@ LL |     f = box g;
    |         ^^^^^
    |         |
    |         cyclic type of infinite size
-   |         help: consider dereferencing the type: `*box g`
+   |         help: try using a conversion method: `box g.to_string()`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/occurs-check.stderr b/src/test/ui/occurs-check.stderr
index fe4248a95c9..61ce61b1cbe 100644
--- a/src/test/ui/occurs-check.stderr
+++ b/src/test/ui/occurs-check.stderr
@@ -5,7 +5,7 @@ LL |     f = box f;
    |         ^^^^^
    |         |
    |         cyclic type of infinite size
-   |         help: consider dereferencing the type: `*box f`
+   |         help: try using a conversion method: `box f.to_string()`
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/span/coerce-suggestions.stderr b/src/test/ui/span/coerce-suggestions.stderr
index 1d8400b20ac..996d80a07e0 100644
--- a/src/test/ui/span/coerce-suggestions.stderr
+++ b/src/test/ui/span/coerce-suggestions.stderr
@@ -44,7 +44,7 @@ LL |     f = box f;
    |         ^^^^^
    |         |
    |         cyclic type of infinite size
-   |         help: consider dereferencing the type: `*box f`
+   |         help: try using a conversion method: `box f.to_string()`
 
 error[E0308]: mismatched types
   --> $DIR/coerce-suggestions.rs:21:9
diff --git a/src/test/ui/suggestions/issue-59819.fixed b/src/test/ui/suggestions/issue-59819.fixed
index 0bf5d32daf9..644d2a4e41b 100644
--- a/src/test/ui/suggestions/issue-59819.fixed
+++ b/src/test/ui/suggestions/issue-59819.fixed
@@ -7,9 +7,18 @@
 
 struct Foo(i32);
 
+struct Bar(String);
+
 impl std::ops::Deref for Foo {
     type Target = i32;
-    fn deref(&self) -> &i32 {
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl std::ops::Deref for Bar {
+    type Target = String;
+    fn deref(&self) -> &Self::Target {
         &self.0
     }
 }
@@ -19,4 +28,8 @@ fn main() {
     let y: i32 = *x; //~ ERROR mismatched types
     let a = &42;
     let b: i32 = *a; //~ ERROR mismatched types
+
+    // Do not make a suggestion when adding a `*` wouldn't actually fix the issue:
+    let f = Bar("bar".to_string());
+    let g: String = f.to_string(); //~ ERROR mismatched types
 }
diff --git a/src/test/ui/suggestions/issue-59819.rs b/src/test/ui/suggestions/issue-59819.rs
index e7e9c7ae259..8e8ff8372e8 100644
--- a/src/test/ui/suggestions/issue-59819.rs
+++ b/src/test/ui/suggestions/issue-59819.rs
@@ -7,9 +7,18 @@
 
 struct Foo(i32);
 
+struct Bar(String);
+
 impl std::ops::Deref for Foo {
     type Target = i32;
-    fn deref(&self) -> &i32 {
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl std::ops::Deref for Bar {
+    type Target = String;
+    fn deref(&self) -> &Self::Target {
         &self.0
     }
 }
@@ -19,4 +28,8 @@ fn main() {
     let y: i32 = x; //~ ERROR mismatched types
     let a = &42;
     let b: i32 = a; //~ ERROR mismatched types
+
+    // Do not make a suggestion when adding a `*` wouldn't actually fix the issue:
+    let f = Bar("bar".to_string());
+    let g: String = f; //~ ERROR mismatched types
 }
diff --git a/src/test/ui/suggestions/issue-59819.stderr b/src/test/ui/suggestions/issue-59819.stderr
index 175c39cfc4f..66898115cbd 100644
--- a/src/test/ui/suggestions/issue-59819.stderr
+++ b/src/test/ui/suggestions/issue-59819.stderr
@@ -1,5 +1,5 @@
 error[E0308]: mismatched types
-  --> $DIR/issue-59819.rs:19:18
+  --> $DIR/issue-59819.rs:28:18
    |
 LL |     let y: i32 = x;
    |                  ^
@@ -11,7 +11,7 @@ LL |     let y: i32 = x;
               found type `Foo`
 
 error[E0308]: mismatched types
-  --> $DIR/issue-59819.rs:21:18
+  --> $DIR/issue-59819.rs:30:18
    |
 LL |     let b: i32 = a;
    |                  ^
@@ -22,6 +22,18 @@ LL |     let b: i32 = a;
    = note: expected type `i32`
               found type `&{integer}`
 
-error: aborting due to 2 previous errors
+error[E0308]: mismatched types
+  --> $DIR/issue-59819.rs:34:21
+   |
+LL |     let g: String = f;
+   |                     ^
+   |                     |
+   |                     expected struct `std::string::String`, found struct `Bar`
+   |                     help: try using a conversion method: `f.to_string()`
+   |
+   = note: expected type `std::string::String`
+              found type `Bar`
+
+error: aborting due to 3 previous errors
 
 For more information about this error, try `rustc --explain E0308`.
diff --git a/src/test/ui/terr-sorts.stderr b/src/test/ui/terr-sorts.stderr
index edc042c85ee..05b9fb43fe1 100644
--- a/src/test/ui/terr-sorts.stderr
+++ b/src/test/ui/terr-sorts.stderr
@@ -2,10 +2,7 @@ error[E0308]: mismatched types
   --> $DIR/terr-sorts.rs:10:14
    |
 LL |     want_foo(b);
-   |              ^
-   |              |
-   |              expected struct `Foo`, found struct `std::boxed::Box`
-   |              help: consider dereferencing the type: `*b`
+   |              ^ expected struct `Foo`, found struct `std::boxed::Box`
    |
    = note: expected type `Foo`
               found type `std::boxed::Box<Foo>`