review comments

Author: estebank

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 {

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::<Vec<_>>().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<Fn({}) -> {}>`",
+                    "a boxed closure type like `Box<dyn Fn({}) -> {}>`",
                     args,
                     fn_sig.output().skip_binder().to_string(),
                 ))

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;

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");
         }
     }
 

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<Fn((), ()) -> std::result::Result<(), _>>`
+   |         - consider giving `x` a boxed closure type like `Box<dyn Fn((), ()) -> std::result::Result<(), _>>`
 LL |         Err(a)?;
    |         ^^^^^^^ cannot infer type
 

src/test/ui/suggestions/suggest-box.stderr

@@ -10,8 +10,8 @@ LL | |     };
    |
    = note: expected type `std::boxed::Box<dyn std::ops::Fn() -> 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<dyn Fn() -> Result<(), ()>> = Box::new(|| {
 LL |         Err(())?;