From 939c1cb349f81a3ce488f5c17f195a5fcd84691c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 13 Aug 2019 11:24:08 -0700 Subject: [PATCH] review comments --- src/librustc/hir/map/mod.rs | 4 +- .../infer/error_reporting/need_type_info.rs | 8 ++-- src/librustc_typeck/check/demand.rs | 2 +- src/librustc_typeck/check/mod.rs | 37 ++++++++++--------- .../ui/inference/cannot-infer-closure.stderr | 2 +- src/test/ui/suggestions/suggest-box.stderr | 4 +- 6 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index b68cef2b6ea..571ee393782 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -649,7 +649,9 @@ impl<'hir> Map<'hir> { } } - pub fn is_const_scope(&self, hir_id: HirId) -> bool { + /// Whether the expression pointed at by `hir_id` belongs to a `const` evaluation context. + /// Used exclusively for diagnostics, to avoid suggestion function calls. + pub fn is_const_context(&self, hir_id: HirId) -> bool { let parent_id = self.get_parent_item(hir_id); match self.get(parent_id) { Node::Item(&Item { diff --git a/src/librustc/infer/error_reporting/need_type_info.rs b/src/librustc/infer/error_reporting/need_type_info.rs index 6d16580384e..b5d78a80bf6 100644 --- a/src/librustc/infer/error_reporting/need_type_info.rs +++ b/src/librustc/infer/error_reporting/need_type_info.rs @@ -153,7 +153,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let (ty_msg, suffix) = match &local_visitor.found_ty { Some(ty) if &ty.to_string() != "_" && name == "_" && - // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) && !ty.is_closure() => // The suggestion doesn't make sense for closures. { @@ -163,7 +163,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } Some(ty) if &ty.to_string() != "_" && ty.to_string() != name && - // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) && !ty.is_closure() => // The suggestion doesn't make sense for closures. { @@ -185,12 +185,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { .map(|args| args.tuple_fields() .map(|arg| arg.to_string()) .collect::>().join(", ")) - .unwrap_or_else(String::new); + .unwrap_or_default(); // This suggestion is incomplete, as the user will get further type inference // errors due to the `_` placeholders and the introduction of `Box`, but it does // nudge them in the right direction. (msg, format!( - "a boxed closure type like `Box {}>`", + "a boxed closure type like `Box {}>`", args, fn_sig.output().skip_binder().to_string(), )) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index bb499a97a2d..ed25601208a 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -549,7 +549,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { checked_ty: Ty<'tcx>, expected_ty: Ty<'tcx>, ) -> bool { - if self.tcx.hir().is_const_scope(expr.hir_id) { + if self.tcx.hir().is_const_context(expr.hir_id) { // Shouldn't suggest `.into()` on `const`s. // FIXME(estebank): modify once we decide to suggest `as` casts return false; diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 5b708b69e2d..8565cbd3708 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3990,27 +3990,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected: Ty<'tcx>, found: Ty<'tcx>, ) { - if self.tcx.hir().is_const_scope(expr.hir_id) { + if self.tcx.hir().is_const_context(expr.hir_id) { // Do not suggest `Box::new` in const context. return; } - if expected.is_box() && !found.is_box() { - let boxed_found = self.tcx.mk_box(found); - if let (true, Ok(snippet)) = ( - self.can_coerce(boxed_found, expected), - self.sess().source_map().span_to_snippet(expr.span), - ) { - err.span_suggestion( - expr.span, - "you can store this in the heap calling `Box::new`", - format!("Box::new({})", snippet), - Applicability::MachineApplicable, - ); - err.note("for more information about the distinction between the stack and the \ - heap, read https://doc.rust-lang.org/book/ch15-01-box.html, \ - https://doc.rust-lang.org/rust-by-example/std/box.html and \ - https://doc.rust-lang.org/std/boxed/index.html"); - } + if !expected.is_box() || found.is_box() { + return; + } + let boxed_found = self.tcx.mk_box(found); + if let (true, Ok(snippet)) = ( + self.can_coerce(boxed_found, expected), + self.sess().source_map().span_to_snippet(expr.span), + ) { + err.span_suggestion( + expr.span, + "store this in the heap by calling `Box::new`", + format!("Box::new({})", snippet), + Applicability::MachineApplicable, + ); + err.note("for more on the distinction between the stack and the \ + heap, read https://doc.rust-lang.org/book/ch15-01-box.html, \ + https://doc.rust-lang.org/rust-by-example/std/box.html, and \ + https://doc.rust-lang.org/std/boxed/index.html"); } } diff --git a/src/test/ui/inference/cannot-infer-closure.stderr b/src/test/ui/inference/cannot-infer-closure.stderr index 20b2784a45d..fb78dedc887 100644 --- a/src/test/ui/inference/cannot-infer-closure.stderr +++ b/src/test/ui/inference/cannot-infer-closure.stderr @@ -2,7 +2,7 @@ error[E0282]: type annotations needed for the closure --> $DIR/cannot-infer-closure.rs:3:9 | LL | let x = |a: (), b: ()| { - | - consider giving `x` a boxed closure type like `Box std::result::Result<(), _>>` + | - consider giving `x` a boxed closure type like `Box std::result::Result<(), _>>` LL | Err(a)?; | ^^^^^^^ cannot infer type diff --git a/src/test/ui/suggestions/suggest-box.stderr b/src/test/ui/suggestions/suggest-box.stderr index aacee2cda13..50c106d63a0 100644 --- a/src/test/ui/suggestions/suggest-box.stderr +++ b/src/test/ui/suggestions/suggest-box.stderr @@ -10,8 +10,8 @@ LL | | }; | = note: expected type `std::boxed::Box std::result::Result<(), ()>>` found type `[closure@$DIR/suggest-box.rs:4:47: 7:6]` - = note: for more information about the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html and https://doc.rust-lang.org/std/boxed/index.html -help: you can store this in the heap calling `Box::new` + = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html +help: store this in the heap by calling `Box::new` | LL | let _x: Box Result<(), ()>> = Box::new(|| { LL | Err(())?;