From 8da6727e961915c153a782cd06b56bfbd796d8fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 27 Dec 2018 17:04:58 -0800 Subject: [PATCH 1/2] Suggest `.as_ref()` when appropriate for `Option` and `Result` --- src/librustc/infer/error_reporting/mod.rs | 74 +++++++++++++++++++++ src/test/ui/as-ref.stderr | 47 ------------- src/test/ui/{ => suggestions}/as-ref.rs | 10 +++ src/test/ui/suggestions/as-ref.stderr | 81 +++++++++++++++++++++++ 4 files changed, 165 insertions(+), 47 deletions(-) delete mode 100644 src/test/ui/as-ref.stderr rename src/test/ui/{ => suggestions}/as-ref.rs (52%) create mode 100644 src/test/ui/suggestions/as-ref.stderr diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 75407835a1b..c9fc896b1b2 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -956,6 +956,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { diag.span_label(span, message); } } + self.suggest_as_ref_where_appropriate(span, &exp_found, diag); } diag.note_expected_found(&"type", expected, found); @@ -972,6 +973,79 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { self.note_error_origin(diag, &cause); } + /// When encountering a case where `.as_ref()` on a `Result` or `Option` would be appropriate, + /// suggest it. + fn suggest_as_ref_where_appropriate( + &self, + span: Span, + exp_found: &ty::error::ExpectedFound>, + diag: &mut DiagnosticBuilder<'tcx>, + ) { + match (&exp_found.expected.sty, &exp_found.found.sty) { + (TyKind::Adt(exp_def, exp_substs), TyKind::Ref(_, found_ty, _)) => { + if let TyKind::Adt(found_def, found_substs) = found_ty.sty { + let path_str = format!("{:?}", exp_def); + if exp_def == &found_def { + let opt_msg = "you can convert from `&Option` to `Option<&T>` using \ + `.as_ref()`"; + let result_msg = "you can convert from `&Result` to \ + `Result<&T, &E>` using `.as_ref()`"; + let have_as_ref = &[ + ("std::option::Option", opt_msg), + ("core::option::Option", opt_msg), + ("std::result::Result", result_msg), + ("core::result::Result", result_msg), + ]; + if let Some(msg) = have_as_ref.iter() + .filter_map(|(path, msg)| if &path_str == path { + Some(msg) + } else { + None + }).next() + { + let mut show_suggestion = true; + for (exp_ty, found_ty) in exp_substs.types().zip(found_substs.types()) { + if let TyKind::Ref(_, exp_ty, _) = exp_ty.sty { + match (&exp_ty.sty, &found_ty.sty) { + (TyKind::Adt(exp_did, _), TyKind::Adt(found_did, _)) + if exp_did == found_did => {} + (TyKind::Bool, TyKind::Bool) | + (TyKind::Char, TyKind::Char) | + (TyKind::Str, TyKind::Str) | + (_, TyKind::Param(_)) | + (_, TyKind::Infer(_)) | + (TyKind::Param(_), _) | + (TyKind::Infer(_), _) => {} + (TyKind::Int(x), TyKind::Int(y)) if x == y => {} + (TyKind::Uint(x), TyKind::Uint(y)) if x == y => {} + (TyKind::Int(x), TyKind::Int(y)) if x == y => {} + (TyKind::Uint(x), TyKind::Uint(y)) if x == y => {} + (TyKind::Float(x), TyKind::Float(y)) if x == y => {} + _ => show_suggestion = false, + } + } else { + show_suggestion = false; + } + } + if let (Ok(snippet), true) = ( + self.tcx.sess.source_map().span_to_snippet(span), + show_suggestion, + ) { + diag.span_suggestion_with_applicability( + span, + msg, + format!("{}.as_ref()", snippet), + Applicability::MachineApplicable, + ); + } + } + } + } + } + _ => {} + } + } + pub fn report_and_explain_type_error( &self, trace: TypeTrace<'tcx>, diff --git a/src/test/ui/as-ref.stderr b/src/test/ui/as-ref.stderr deleted file mode 100644 index 8ca54e8006c..00000000000 --- a/src/test/ui/as-ref.stderr +++ /dev/null @@ -1,47 +0,0 @@ -error[E0308]: mismatched types - --> $DIR/as-ref.rs:6:27 - | -LL | opt.map(|arg| takes_ref(arg)); - | - ^^^ expected &Foo, found struct `Foo` - | | - | help: consider using `as_ref` instead: `as_ref().` - | - = note: expected type `&Foo` - found type `Foo` - -error[E0308]: mismatched types - --> $DIR/as-ref.rs:8:37 - | -LL | opt.and_then(|arg| Some(takes_ref(arg))); - | - ^^^ expected &Foo, found struct `Foo` - | | - | help: consider using `as_ref` instead: `as_ref().` - | - = note: expected type `&Foo` - found type `Foo` - -error[E0308]: mismatched types - --> $DIR/as-ref.rs:11:27 - | -LL | opt.map(|arg| takes_ref(arg)); - | - ^^^ expected &Foo, found struct `Foo` - | | - | help: consider using `as_ref` instead: `as_ref().` - | - = note: expected type `&Foo` - found type `Foo` - -error[E0308]: mismatched types - --> $DIR/as-ref.rs:13:35 - | -LL | opt.and_then(|arg| Ok(takes_ref(arg))); - | - ^^^ expected &Foo, found struct `Foo` - | | - | help: consider using `as_ref` instead: `as_ref().` - | - = note: expected type `&Foo` - found type `Foo` - -error: aborting due to 4 previous errors - -For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/as-ref.rs b/src/test/ui/suggestions/as-ref.rs similarity index 52% rename from src/test/ui/as-ref.rs rename to src/test/ui/suggestions/as-ref.rs index 822d10f2ca7..03f04c389f1 100644 --- a/src/test/ui/as-ref.rs +++ b/src/test/ui/suggestions/as-ref.rs @@ -12,4 +12,14 @@ fn main() { //~^ ERROR mismatched types [E0308] opt.and_then(|arg| Ok(takes_ref(arg))); //~^ ERROR mismatched types [E0308] + let x: &Option = &Some(3); + let y: Option<&usize> = x; + //~^ ERROR mismatched types [E0308] + let x: &Result = &Ok(3); + let y: Result<&usize, &usize> = x; + //~^ ERROR mismatched types [E0308] + // note: do not suggest because of `E: usize` + let x: &Result = &Ok(3); + let y: Result<&usize, usize> = x; + //~^ ERROR mismatched types [E0308] } diff --git a/src/test/ui/suggestions/as-ref.stderr b/src/test/ui/suggestions/as-ref.stderr new file mode 100644 index 00000000000..7273496a7ce --- /dev/null +++ b/src/test/ui/suggestions/as-ref.stderr @@ -0,0 +1,81 @@ +error[E0308]: mismatched types + --> $DIR/as-ref.rs:6:27 + | +LL | opt.map(|arg| takes_ref(arg)); + | - ^^^ expected &Foo, found struct `Foo` + | | + | help: consider using `as_ref` instead: `as_ref().` + | + = note: expected type `&Foo` + found type `Foo` + +error[E0308]: mismatched types + --> $DIR/as-ref.rs:8:37 + | +LL | opt.and_then(|arg| Some(takes_ref(arg))); + | - ^^^ expected &Foo, found struct `Foo` + | | + | help: consider using `as_ref` instead: `as_ref().` + | + = note: expected type `&Foo` + found type `Foo` + +error[E0308]: mismatched types + --> $DIR/as-ref.rs:11:27 + | +LL | opt.map(|arg| takes_ref(arg)); + | - ^^^ expected &Foo, found struct `Foo` + | | + | help: consider using `as_ref` instead: `as_ref().` + | + = note: expected type `&Foo` + found type `Foo` + +error[E0308]: mismatched types + --> $DIR/as-ref.rs:13:35 + | +LL | opt.and_then(|arg| Ok(takes_ref(arg))); + | - ^^^ expected &Foo, found struct `Foo` + | | + | help: consider using `as_ref` instead: `as_ref().` + | + = note: expected type `&Foo` + found type `Foo` + +error[E0308]: mismatched types + --> $DIR/as-ref.rs:16:27 + | +LL | let y: Option<&usize> = x; + | ^ + | | + | expected enum `std::option::Option`, found reference + | help: you can convert from `&Option` to `Option<&T>` using `.as_ref()`: `x.as_ref()` + | + = note: expected type `std::option::Option<&usize>` + found type `&std::option::Option` + +error[E0308]: mismatched types + --> $DIR/as-ref.rs:19:35 + | +LL | let y: Result<&usize, &usize> = x; + | ^ expected enum `std::result::Result`, found reference + | + = note: expected type `std::result::Result<&usize, &usize>` + found type `&std::result::Result` +help: you can convert from `&Result` to `Result<&T, &E>` using `.as_ref()` + | +LL | let y: Result<&usize, &usize> = x.as_ref(); + | ^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/as-ref.rs:23:34 + | +LL | let y: Result<&usize, usize> = x; + | ^ expected enum `std::result::Result`, found reference + | + = note: expected type `std::result::Result<&usize, usize>` + found type `&std::result::Result` + +error: aborting due to 7 previous errors + +For more information about this error, try `rustc --explain E0308`. From 0ecc128ccb43d0302288011a6919989e91da3bb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 29 Dec 2018 10:54:32 -0800 Subject: [PATCH 2/2] Use `same_type` instead of duplicating logic --- src/librustc/infer/error_reporting/mod.rs | 31 +++++++++-------------- src/librustc/ty/util.rs | 28 ++++++++++---------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index c9fc896b1b2..e34bb9f4f7b 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -1005,26 +1005,19 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { { let mut show_suggestion = true; for (exp_ty, found_ty) in exp_substs.types().zip(found_substs.types()) { - if let TyKind::Ref(_, exp_ty, _) = exp_ty.sty { - match (&exp_ty.sty, &found_ty.sty) { - (TyKind::Adt(exp_did, _), TyKind::Adt(found_did, _)) - if exp_did == found_did => {} - (TyKind::Bool, TyKind::Bool) | - (TyKind::Char, TyKind::Char) | - (TyKind::Str, TyKind::Str) | - (_, TyKind::Param(_)) | - (_, TyKind::Infer(_)) | - (TyKind::Param(_), _) | - (TyKind::Infer(_), _) => {} - (TyKind::Int(x), TyKind::Int(y)) if x == y => {} - (TyKind::Uint(x), TyKind::Uint(y)) if x == y => {} - (TyKind::Int(x), TyKind::Int(y)) if x == y => {} - (TyKind::Uint(x), TyKind::Uint(y)) if x == y => {} - (TyKind::Float(x), TyKind::Float(y)) if x == y => {} - _ => show_suggestion = false, + match exp_ty.sty { + TyKind::Ref(_, exp_ty, _) => { + match (&exp_ty.sty, &found_ty.sty) { + (_, TyKind::Param(_)) | + (_, TyKind::Infer(_)) | + (TyKind::Param(_), _) | + (TyKind::Infer(_), _) => {} + _ if ty::TyS::same_type(exp_ty, found_ty) => {} + _ => show_suggestion = false, + }; } - } else { - show_suggestion = false; + TyKind::Param(_) | TyKind::Infer(_) => {} + _ => show_suggestion = false, } } if let (Ok(snippet), true) = ( diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index b2e2955619e..1d30ccb87b5 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -658,6 +658,19 @@ impl<'a, 'tcx> ty::TyS<'tcx> { tcx.needs_drop_raw(param_env.and(self)) } + pub fn same_type(a: Ty<'tcx>, b: Ty<'tcx>) -> bool { + match (&a.sty, &b.sty) { + (&Adt(did_a, substs_a), &Adt(did_b, substs_b)) => { + if did_a != did_b { + return false; + } + + substs_a.types().zip(substs_b.types()).all(|(a, b)| Self::same_type(a, b)) + } + _ => a == b, + } + } + /// Check whether a type is representable. This means it cannot contain unboxed /// structural recursion. This check is needed for structs and enums. pub fn is_representable(&'tcx self, @@ -730,19 +743,6 @@ impl<'a, 'tcx> ty::TyS<'tcx> { } } - fn same_type<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool { - match (&a.sty, &b.sty) { - (&Adt(did_a, substs_a), &Adt(did_b, substs_b)) => { - if did_a != did_b { - return false; - } - - substs_a.types().zip(substs_b.types()).all(|(a, b)| same_type(a, b)) - } - _ => a == b, - } - } - // Does the type `ty` directly (without indirection through a pointer) // contain any types on stack `seen`? fn is_type_structurally_recursive<'a, 'tcx>( @@ -807,7 +807,7 @@ impl<'a, 'tcx> ty::TyS<'tcx> { // struct Foo { Option> } for &seen_type in iter { - if same_type(ty, seen_type) { + if ty::TyS::same_type(ty, seen_type) { debug!("ContainsRecursive: {:?} contains {:?}", seen_type, ty);