Rollup merge of rust-lang#74228 - estebank:unsized-param, r=estebank

Provide structured suggestion on unsized fields and fn params

* Suggest borrowing or boxing unsized fields
* Suggest borrowing fn parameters
* Remove some verbosity of unsized errors
* Remove `on_unimplemented` note from `trait Sized`

Fix rust-lang#23286, fix rust-lang#28653.

r? @davidtwco

src/libcore/marker.rs

@@ -84,11 +84,8 @@ impl<T: ?Sized> !Send for *mut T {}
 #[stable(feature = "rust1", since = "1.0.0")]
 #[lang = "sized"]
 #[rustc_on_unimplemented(
-    on(parent_trait = "std::path::Path", label = "borrow the `Path` instead"),
     message = "the size for values of type `{Self}` cannot be known at compilation time",
-    label = "doesn't have a size known at compile-time",
-    note = "to learn more, visit <https://doc.rust-lang.org/book/\
-            ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>"
+    label = "doesn't have a size known at compile-time"
 )]
 #[fundamental] // for Default, for example, which requires that `[T]: !Default` be evaluatable
 #[rustc_specialization_trait]

src/librustc_ast_lowering/expr.rs

@@ -526,7 +526,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             Ident::with_dummy_span(sym::_task_context),
             hir::BindingAnnotation::Mutable,
         );
-        let param = hir::Param { attrs: &[], hir_id: self.next_id(), pat, span };
+        let param = hir::Param { attrs: &[], hir_id: self.next_id(), pat, ty_span: span, span };
         let params = arena_vec![self; param];
 
         let body_id = self.lower_body(move |this| {

src/librustc_ast_lowering/item.rs

@@ -972,6 +972,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             attrs: self.lower_attrs(&param.attrs),
             hir_id: self.lower_node_id(param.id),
             pat: self.lower_pat(&param.pat),
+            ty_span: param.ty.span,
             span: param.span,
         }
     }
@@ -1098,6 +1099,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     attrs: parameter.attrs,
                     hir_id: parameter.hir_id,
                     pat: new_parameter_pat,
+                    ty_span: parameter.ty_span,
                     span: parameter.span,
                 };
 

src/librustc_hir/hir.rs

@@ -2148,6 +2148,7 @@ pub struct Param<'hir> {
     pub attrs: &'hir [Attribute],
     pub hir_id: HirId,
     pub pat: &'hir Pat<'hir>,
+    pub ty_span: Span,
     pub span: Span,
 }
 

src/librustc_middle/traits/mod.rs

@@ -215,7 +215,7 @@ pub enum ObligationCauseCode<'tcx> {
     /// Type of each variable must be `Sized`.
     VariableType(hir::HirId),
     /// Argument type must be `Sized`.
-    SizedArgumentType,
+    SizedArgumentType(Option<Span>),
     /// Return type must be `Sized`.
     SizedReturnType,
     /// Yield type must be `Sized`.
@@ -229,6 +229,7 @@ pub enum ObligationCauseCode<'tcx> {
     /// Types of fields (other than the last, except for packed structs) in a struct must be sized.
     FieldSized {
         adt_kind: AdtKind,
+        span: Span,
         last: bool,
     },
 

src/librustc_middle/traits/structural_impls.rs

@@ -151,12 +151,14 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
             super::VariableType(id) => Some(super::VariableType(id)),
             super::ReturnValue(id) => Some(super::ReturnValue(id)),
             super::ReturnType => Some(super::ReturnType),
-            super::SizedArgumentType => Some(super::SizedArgumentType),
+            super::SizedArgumentType(sp) => Some(super::SizedArgumentType(sp)),
             super::SizedReturnType => Some(super::SizedReturnType),
             super::SizedYieldType => Some(super::SizedYieldType),
             super::InlineAsmSized => Some(super::InlineAsmSized),
             super::RepeatVec(suggest_flag) => Some(super::RepeatVec(suggest_flag)),
-            super::FieldSized { adt_kind, last } => Some(super::FieldSized { adt_kind, last }),
+            super::FieldSized { adt_kind, span, last } => {
+                Some(super::FieldSized { adt_kind, span, last })
+            }
             super::ConstSized => Some(super::ConstSized),
             super::ConstPatternStructural => Some(super::ConstPatternStructural),
             super::SharedStatic => Some(super::SharedStatic),

src/librustc_trait_selection/traits/error_reporting/mod.rs

@@ -376,7 +376,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                             // If it has a custom `#[rustc_on_unimplemented]`
                             // error message, let's display it as the label!
                             err.span_label(span, s.as_str());
-                            err.help(&explanation);
+                            if !matches!(trait_ref.skip_binder().self_ty().kind, ty::Param(_)) {
+                                // When the self type is a type param We don't need to "the trait
+                                // `std::marker::Sized` is not implemented for `T`" as we will point
+                                // at the type param with a label to suggest constraining it.
+                                err.help(&explanation);
+                            }
                         } else {
                             err.span_label(span, explanation);
                         }
@@ -403,7 +408,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                         }
 
                         self.suggest_dereferences(&obligation, &mut err, &trait_ref, points_at_arg);
-                        self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err);
                         self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg);
                         self.suggest_remove_reference(&obligation, &mut err, &trait_ref);
                         self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref);

src/librustc_trait_selection/traits/error_reporting/suggestions.rs

@@ -43,12 +43,6 @@ pub trait InferCtxtExt<'tcx> {
         body_id: hir::HirId,
     );
 
-    fn suggest_borrow_on_unsized_slice(
-        &self,
-        code: &ObligationCauseCode<'tcx>,
-        err: &mut DiagnosticBuilder<'_>,
-    );
-
     fn suggest_dereferences(
         &self,
         obligation: &PredicateObligation<'tcx>,
@@ -515,32 +509,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
         }
     }
 
-    /// When encountering an assignment of an unsized trait, like `let x = ""[..];`, provide a
-    /// suggestion to borrow the initializer in order to use have a slice instead.
-    fn suggest_borrow_on_unsized_slice(
-        &self,
-        code: &ObligationCauseCode<'tcx>,
-        err: &mut DiagnosticBuilder<'_>,
-    ) {
-        if let &ObligationCauseCode::VariableType(hir_id) = code {
-            let parent_node = self.tcx.hir().get_parent_node(hir_id);
-            if let Some(Node::Local(ref local)) = self.tcx.hir().find(parent_node) {
-                if let Some(ref expr) = local.init {
-                    if let hir::ExprKind::Index(_, _) = expr.kind {
-                        if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(expr.span) {
-                            err.span_suggestion(
-                                expr.span,
-                                "consider borrowing here",
-                                format!("&{}", snippet),
-                                Applicability::MachineApplicable,
-                            );
-                        }
-                    }
-                }
-            }
-        }
-    }
-
     /// Given a closure's `DefId`, return the given name of the closure.
     ///
     /// This doesn't account for reassignments, but it's only used for suggestions.
@@ -1817,15 +1785,56 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                     }
                 }
             }
-            ObligationCauseCode::VariableType(_) => {
-                err.note("all local variables must have a statically known size");
+            ObligationCauseCode::VariableType(hir_id) => {
+                let parent_node = self.tcx.hir().get_parent_node(hir_id);
+                match self.tcx.hir().find(parent_node) {
+                    Some(Node::Local(hir::Local {
+                        init: Some(hir::Expr { kind: hir::ExprKind::Index(_, _), span, .. }),
+                        ..
+                    })) => {
+                        // When encountering an assignment of an unsized trait, like
+                        // `let x = ""[..];`, provide a suggestion to borrow the initializer in
+                        // order to use have a slice instead.
+                        err.span_suggestion_verbose(
+                            span.shrink_to_lo(),
+                            "consider borrowing here",
+                            "&".to_owned(),
+                            Applicability::MachineApplicable,
+                        );
+                        err.note("all local variables must have a statically known size");
+                    }
+                    Some(Node::Param(param)) => {
+                        err.span_suggestion_verbose(
+                            param.ty_span.shrink_to_lo(),
+                            "function arguments must have a statically known size, borrowed types \
+                            always have a known size",
+                            "&".to_owned(),
+                            Applicability::MachineApplicable,
+                        );
+                    }
+                    _ => {
+                        err.note("all local variables must have a statically known size");
+                    }
+                }
                 if !self.tcx.features().unsized_locals {
                     err.help("unsized locals are gated as an unstable feature");
                 }
             }
-            ObligationCauseCode::SizedArgumentType => {
-                err.note("all function arguments must have a statically known size");
-                if !self.tcx.features().unsized_locals {
+            ObligationCauseCode::SizedArgumentType(sp) => {
+                if let Some(span) = sp {
+                    err.span_suggestion_verbose(
+                        span.shrink_to_lo(),
+                        "function arguments must have a statically known size, borrowed types \
+                         always have a known size",
+                        "&".to_string(),
+                        Applicability::MachineApplicable,
+                    );
+                } else {
+                    err.note("all function arguments must have a statically known size");
+                }
+                if tcx.sess.opts.unstable_features.is_nightly_build()
+                    && !self.tcx.features().unsized_locals
+                {
                     err.help("unsized locals are gated as an unstable feature");
                 }
             }
@@ -1844,26 +1853,43 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
             ObligationCauseCode::StructInitializerSized => {
                 err.note("structs must have a statically known size to be initialized");
             }
-            ObligationCauseCode::FieldSized { adt_kind: ref item, last } => match *item {
-                AdtKind::Struct => {
-                    if last {
-                        err.note(
-                            "the last field of a packed struct may only have a \
-                             dynamically sized type if it does not need drop to be run",
-                        );
-                    } else {
-                        err.note(
-                            "only the last field of a struct may have a dynamically sized type",
-                        );
+            ObligationCauseCode::FieldSized { adt_kind: ref item, last, span } => {
+                match *item {
+                    AdtKind::Struct => {
+                        if last {
+                            err.note(
+                                "the last field of a packed struct may only have a \
+                                dynamically sized type if it does not need drop to be run",
+                            );
+                        } else {
+                            err.note(
+                                "only the last field of a struct may have a dynamically sized type",
+                            );
+                        }
+                    }
+                    AdtKind::Union => {
+                        err.note("no field of a union may have a dynamically sized type");
+                    }
+                    AdtKind::Enum => {
+                        err.note("no field of an enum variant may have a dynamically sized type");
                     }
                 }
-                AdtKind::Union => {
-                    err.note("no field of a union may have a dynamically sized type");
-                }
-                AdtKind::Enum => {
-                    err.note("no field of an enum variant may have a dynamically sized type");
-                }
-            },
+                err.help("change the field's type to have a statically known size");
+                err.span_suggestion(
+                    span.shrink_to_lo(),
+                    "borrowed types always have a statically known size",
+                    "&".to_string(),
+                    Applicability::MachineApplicable,
+                );
+                err.multipart_suggestion(
+                    "heap allocated types always have a statically known size",
+                    vec![
+                        (span.shrink_to_lo(), "Box<".to_string()),
+                        (span.shrink_to_hi(), ">".to_string()),
+                    ],
+                    Applicability::MachineApplicable,
+                );
+            }
             ObligationCauseCode::ConstSized => {
                 err.note("constant expressions must have a statically known size");
             }

src/librustc_typeck/check/expr.rs

@@ -487,7 +487,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     self.require_type_is_sized_deferred(
                         input,
                         expr.span,
-                        traits::SizedArgumentType,
+                        traits::SizedArgumentType(None),
                     );
                 }
             }

src/librustc_typeck/check/mod.rs

@@ -1342,14 +1342,15 @@ fn check_fn<'a, 'tcx>(
     let inputs_fn = fn_sig.inputs().iter().copied();
     for (idx, (param_ty, param)) in inputs_fn.chain(maybe_va_list).zip(body.params).enumerate() {
         // Check the pattern.
-        fcx.check_pat_top(&param.pat, param_ty, try { inputs_hir?.get(idx)?.span }, false);
+        let ty_span = try { inputs_hir?.get(idx)?.span };
+        fcx.check_pat_top(&param.pat, param_ty, ty_span, false);
 
         // Check that argument is Sized.
         // The check for a non-trivial pattern is a hack to avoid duplicate warnings
         // for simple cases like `fn foo(x: Trait)`,
         // where we would error once on the parameter as a whole, and once on the binding `x`.
         if param.pat.simple_ident().is_none() && !tcx.features().unsized_locals {
-            fcx.require_type_is_sized(param_ty, param.pat.span, traits::SizedArgumentType);
+            fcx.require_type_is_sized(param_ty, param.pat.span, traits::SizedArgumentType(ty_span));
         }
 
         fcx.write_ty(param.hir_id, param_ty);

src/librustc_typeck/check/wfcheck.rs

@@ -394,6 +394,7 @@ fn check_type_defn<'tcx, F>(
                                 Some(i) => i,
                                 None => bug!(),
                             },
+                            span: field.span,
                             last,
                         },
                     ),
@@ -1326,10 +1327,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             .iter()
             .map(|field| {
                 let field_ty = self.tcx.type_of(self.tcx.hir().local_def_id(field.hir_id));
-                let field_ty = self.normalize_associated_types_in(field.span, &field_ty);
+                let field_ty = self.normalize_associated_types_in(field.ty.span, &field_ty);
                 let field_ty = self.resolve_vars_if_possible(&field_ty);
                 debug!("non_enum_variant: type of field {:?} is {:?}", field, field_ty);
-                AdtField { ty: field_ty, span: field.span }
+                AdtField { ty: field_ty, span: field.ty.span }
             })
             .collect();
         AdtVariant { fields, explicit_discr: None }

src/test/ui/asm/type-check-1.stderr

@@ -17,7 +17,6 @@ LL |         asm!("{}", in(reg) v[..]);
    |                            ^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `std::marker::Sized` is not implemented for `[u64]`
-   = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
    = note: all inline asm arguments must have a statically known size
 
 error[E0277]: the size for values of type `[u64]` cannot be known at compilation time
@@ -27,7 +26,6 @@ LL |         asm!("{}", out(reg) v[..]);
    |                             ^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `std::marker::Sized` is not implemented for `[u64]`
-   = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
    = note: all inline asm arguments must have a statically known size
 
 error[E0277]: the size for values of type `[u64]` cannot be known at compilation time
@@ -37,7 +35,6 @@ LL |         asm!("{}", inout(reg) v[..]);
    |                               ^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `std::marker::Sized` is not implemented for `[u64]`
-   = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
    = note: all inline asm arguments must have a statically known size
 
 error: aborting due to 5 previous errors

src/test/ui/associated-types/associated-types-unsized.stderr

@@ -5,7 +5,6 @@ LL |     let x = t.get();
    |         ^ doesn't have a size known at compile-time
    |
    = help: the trait `std::marker::Sized` is not implemented for `<T as Get>::Value`
-   = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
    = note: all local variables must have a statically known size
    = help: unsized locals are gated as an unstable feature
 help: consider further restricting the associated type

src/test/ui/associated-types/defaults-suitability.stderr

@@ -139,7 +139,6 @@ LL | pub struct Vec<T> {
    |                - required by this bound in `std::vec::Vec`
    |
    = help: the trait `std::marker::Sized` is not implemented for `[u8]`
-   = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
 
 error: aborting due to 11 previous errors
 

src/test/ui/associated-types/defaults-unsound-62211-1.stderr

@@ -21,7 +21,6 @@ LL | trait UncheckedCopy: Sized {
 LL |     + AddAssign<&'static str>
    |       ^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `Self += &'static str`
    |
-   = help: the trait `std::ops::AddAssign<&'static str>` is not implemented for `Self`
 help: consider further restricting `Self`
    |
 LL | trait UncheckedCopy: Sized + std::ops::AddAssign<&'static str> {
@@ -50,7 +49,6 @@ LL | trait UncheckedCopy: Sized {
 LL |     + Display = Self;
    |       ^^^^^^^ `Self` cannot be formatted with the default formatter
    |
-   = help: the trait `std::fmt::Display` is not implemented for `Self`
    = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
 help: consider further restricting `Self`
    |
@@ -69,7 +67,6 @@ LL |     + Display = Self;
 LL | impl<T> UncheckedCopy for T {}
    |         ^^^^^^^^^^^^^ `T` cannot be formatted with the default formatter
    |
-   = help: the trait `std::fmt::Display` is not implemented for `T`
    = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
 help: consider restricting type parameter `T`
    |
@@ -105,7 +102,6 @@ LL |     + AddAssign<&'static str>
 LL | impl<T> UncheckedCopy for T {}
    |         ^^^^^^^^^^^^^ no implementation for `T += &'static str`
    |
-   = help: the trait `std::ops::AddAssign<&'static str>` is not implemented for `T`
 help: consider restricting type parameter `T`
    |
 LL | impl<T: std::ops::AddAssign<&'static str>> UncheckedCopy for T {}