Rollup merge of #73261 - estebank:generics-sized, r=nikomatsakis

Suggest `?Sized` when applicable for ADTs

Address #71790, fix #27964.

src/librustc_hir/hir.rs

@@ -2741,14 +2741,8 @@ impl Node<'_> {
     pub fn generics(&self) -> Option<&Generics<'_>> {
         match self {
             Node::TraitItem(TraitItem { generics, .. })
-            | Node::ImplItem(ImplItem { generics, .. })
-            | Node::Item(Item {
-                kind:
-                    ItemKind::Trait(_, _, generics, ..)
-                    | ItemKind::Impl { generics, .. }
-                    | ItemKind::Fn(_, generics, _),
-                ..
-            }) => Some(generics),
+            | Node::ImplItem(ImplItem { generics, .. }) => Some(generics),
+            Node::Item(item) => item.kind.generics(),
             _ => None,
         }
     }

src/librustc_trait_selection/traits/error_reporting/mod.rs

@@ -15,6 +15,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder, ErrorReported};
 use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE};
+use rustc_hir::intravisit::Visitor;
 use rustc_hir::Node;
 use rustc_middle::mir::interpret::ErrorHandled;
 use rustc_middle::ty::error::ExpectedFound;
@@ -25,7 +26,7 @@ use rustc_middle::ty::{
     TypeFoldable, WithConstness,
 };
 use rustc_session::DiagnosticMessageId;
-use rustc_span::{ExpnKind, Span, DUMMY_SP};
+use rustc_span::{ExpnKind, MultiSpan, Span, DUMMY_SP};
 use std::fmt;
 
 use crate::traits::query::evaluate_obligation::InferCtxtExt as _;
@@ -1695,36 +1696,95 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
         err: &mut DiagnosticBuilder<'tcx>,
         obligation: &PredicateObligation<'tcx>,
     ) {
-        if let (
-            ty::PredicateKind::Trait(pred, _),
-            ObligationCauseCode::BindingObligation(item_def_id, span),
-        ) = (obligation.predicate.kind(), &obligation.cause.code)
-        {
-            if let (Some(generics), true) = (
-                self.tcx.hir().get_if_local(*item_def_id).as_ref().and_then(|n| n.generics()),
-                Some(pred.def_id()) == self.tcx.lang_items().sized_trait(),
-            ) {
-                for param in generics.params {
-                    if param.span == *span
-                        && !param.bounds.iter().any(|bound| {
-                            bound.trait_ref().and_then(|trait_ref| trait_ref.trait_def_id())
-                                == self.tcx.lang_items().sized_trait()
-                        })
-                    {
-                        let (span, separator) = match param.bounds {
-                            [] => (span.shrink_to_hi(), ":"),
-                            [.., bound] => (bound.span().shrink_to_hi(), " +"),
-                        };
-                        err.span_suggestion_verbose(
-                            span,
-                            "consider relaxing the implicit `Sized` restriction",
-                            format!("{} ?Sized", separator),
-                            Applicability::MachineApplicable,
+        let (pred, item_def_id, span) =
+            match (obligation.predicate.kind(), &obligation.cause.code.peel_derives()) {
+                (
+                    ty::PredicateKind::Trait(pred, _),
+                    ObligationCauseCode::BindingObligation(item_def_id, span),
+                ) => (pred, item_def_id, span),
+                _ => return,
+            };
+
+        let node = match (
+            self.tcx.hir().get_if_local(*item_def_id),
+            Some(pred.def_id()) == self.tcx.lang_items().sized_trait(),
+        ) {
+            (Some(node), true) => node,
+            _ => return,
+        };
+        let generics = match node.generics() {
+            Some(generics) => generics,
+            None => return,
+        };
+        for param in generics.params {
+            if param.span != *span
+                || param.bounds.iter().any(|bound| {
+                    bound.trait_ref().and_then(|trait_ref| trait_ref.trait_def_id())
+                        == self.tcx.lang_items().sized_trait()
+                })
+            {
+                continue;
+            }
+            match node {
+                hir::Node::Item(
+                    item
+                    @
+                    hir::Item {
+                        kind:
+                            hir::ItemKind::Enum(..)
+                            | hir::ItemKind::Struct(..)
+                            | hir::ItemKind::Union(..),
+                        ..
+                    },
+                ) => {
+                    // Suggesting `T: ?Sized` is only valid in an ADT if `T` is only used in a
+                    // borrow. `struct S<'a, T: ?Sized>(&'a T);` is valid, `struct S<T: ?Sized>(T);`
+                    // is not.
+                    let mut visitor = FindTypeParam {
+                        param: param.name.ident().name,
+                        invalid_spans: vec![],
+                        nested: false,
+                    };
+                    visitor.visit_item(item);
+                    if !visitor.invalid_spans.is_empty() {
+                        let mut multispan: MultiSpan = param.span.into();
+                        multispan.push_span_label(
+                            param.span,
+                            format!("this could be changed to `{}: ?Sized`...", param.name.ident()),
+                        );
+                        for sp in visitor.invalid_spans {
+                            multispan.push_span_label(
+                                sp,
+                                format!(
+                                    "...if indirection was used here: `Box<{}>`",
+                                    param.name.ident(),
+                                ),
+                            );
+                        }
+                        err.span_help(
+                            multispan,
+                            &format!(
+                                "you could relax the implicit `Sized` bound on `{T}` if it were \
+                                 used through indirection like `&{T}` or `Box<{T}>`",
+                                T = param.name.ident(),
+                            ),
                         );
                         return;
                     }
                 }
+                _ => {}
             }
+            let (span, separator) = match param.bounds {
+                [] => (span.shrink_to_hi(), ":"),
+                [.., bound] => (bound.span().shrink_to_hi(), " +"),
+            };
+            err.span_suggestion_verbose(
+                span,
+                "consider relaxing the implicit `Sized` restriction",
+                format!("{} ?Sized", separator),
+                Applicability::MachineApplicable,
+            );
+            return;
         }
     }
 
@@ -1744,6 +1804,50 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
     }
 }
 
+/// Look for type `param` in an ADT being used only through a reference to confirm that suggesting
+/// `param: ?Sized` would be a valid constraint.
+struct FindTypeParam {
+    param: rustc_span::Symbol,
+    invalid_spans: Vec<Span>,
+    nested: bool,
+}
+
+impl<'v> Visitor<'v> for FindTypeParam {
+    type Map = rustc_hir::intravisit::ErasedMap<'v>;
+
+    fn nested_visit_map(&mut self) -> hir::intravisit::NestedVisitorMap<Self::Map> {
+        hir::intravisit::NestedVisitorMap::None
+    }
+
+    fn visit_ty(&mut self, ty: &hir::Ty<'_>) {
+        // We collect the spans of all uses of the "bare" type param, like in `field: T` or
+        // `field: (T, T)` where we could make `T: ?Sized` while skipping cases that are known to be
+        // valid like `field: &'a T` or `field: *mut T` and cases that *might* have further `Sized`
+        // obligations like `Box<T>` and `Vec<T>`, but we perform no extra analysis for those cases
+        // and suggest `T: ?Sized` regardless of their obligations. This is fine because the errors
+        // in that case should make what happened clear enough.
+        match ty.kind {
+            hir::TyKind::Ptr(_) | hir::TyKind::Rptr(..) | hir::TyKind::TraitObject(..) => {}
+            hir::TyKind::Path(hir::QPath::Resolved(None, path))
+                if path.segments.len() == 1 && path.segments[0].ident.name == self.param =>
+            {
+                if !self.nested {
+                    self.invalid_spans.push(ty.span);
+                }
+            }
+            hir::TyKind::Path(_) => {
+                let prev = self.nested;
+                self.nested = true;
+                hir::intravisit::walk_ty(self, ty);
+                self.nested = prev;
+            }
+            _ => {
+                hir::intravisit::walk_ty(self, ty);
+            }
+        }
+    }
+}
+
 pub fn recursive_type_with_infinite_size_error(
     tcx: TyCtxt<'tcx>,
     type_def_id: DefId,

src/test/ui/dst/dst-sized-trait-param.stderr

@@ -9,6 +9,10 @@ LL | impl Foo<[isize]> for usize { }
    |
    = help: the trait `std::marker::Sized` is not implemented for `[isize]`
    = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
+help: consider relaxing the implicit `Sized` restriction
+   |
+LL | trait Foo<T: ?Sized> : Sized { fn take(self, x: &T) { } } // Note: T is sized
+   |            ^^^^^^^^
 
 error[E0277]: the size for values of type `[usize]` cannot be known at compilation time
   --> $DIR/dst-sized-trait-param.rs:10:6

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

@@ -26,6 +26,10 @@ LL |     assert_sized::<Foo>();
    = help: within `Foo`, the trait `std::marker::Sized` is not implemented for `A`
    = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
    = note: required because it appears within the type `Foo`
+help: consider relaxing the implicit `Sized` restriction
+   |
+LL | fn assert_sized<T: ?Sized>() { }
+   |                  ^^^^^^^^
 
 error[E0277]: the size for values of type `A` cannot be known at compilation time
   --> $DIR/extern-types-unsized.rs:28:5
@@ -39,6 +43,10 @@ LL |     assert_sized::<Bar<A>>();
    = help: within `Bar<A>`, the trait `std::marker::Sized` is not implemented for `A`
    = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
    = note: required because it appears within the type `Bar<A>`
+help: consider relaxing the implicit `Sized` restriction
+   |
+LL | fn assert_sized<T: ?Sized>() { }
+   |                  ^^^^^^^^
 
 error[E0277]: the size for values of type `A` cannot be known at compilation time
   --> $DIR/extern-types-unsized.rs:31:5
@@ -53,6 +61,10 @@ LL |     assert_sized::<Bar<Bar<A>>>();
    = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
    = note: required because it appears within the type `Bar<A>`
    = note: required because it appears within the type `Bar<Bar<A>>`
+help: consider relaxing the implicit `Sized` restriction
+   |
+LL | fn assert_sized<T: ?Sized>() { }
+   |                  ^^^^^^^^
 
 error: aborting due to 4 previous errors
 

src/test/ui/issues/issue-10412.stderr

@@ -57,6 +57,10 @@ LL | impl<'self> Serializable<str> for &'self str {
    |
    = help: the trait `std::marker::Sized` is not implemented for `str`
    = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
+help: consider relaxing the implicit `Sized` restriction
+   |
+LL | trait Serializable<'self, T: ?Sized> {
+   |                            ^^^^^^^^
 
 error: aborting due to 9 previous errors
 

src/test/ui/issues/issue-18919.stderr

@@ -9,6 +9,13 @@ LL | enum Option<T> {
    |
    = help: the trait `std::marker::Sized` is not implemented for `dyn for<'r> std::ops::Fn(&'r isize) -> isize`
    = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
+help: you could relax the implicit `Sized` bound on `T` if it were used through indirection like `&T` or `Box<T>`
+  --> $DIR/issue-18919.rs:7:13
+   |
+LL | enum Option<T> {
+   |             ^ this could be changed to `T: ?Sized`...
+LL |     Some(T),
+   |          - ...if indirection was used here: `Box<T>`
 
 error: aborting due to previous error
 

src/test/ui/issues/issue-23281.stderr

@@ -9,6 +9,13 @@ LL | struct Vec<T> {
    |
    = help: the trait `std::marker::Sized` is not implemented for `(dyn std::ops::Fn() + 'static)`
    = note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
+help: you could relax the implicit `Sized` bound on `T` if it were used through indirection like `&T` or `Box<T>`
+  --> $DIR/issue-23281.rs:8:12
+   |
+LL | struct Vec<T> {
+   |            ^ this could be changed to `T: ?Sized`...
+LL |     t: T,
+   |        - ...if indirection was used here: `Box<T>`
 
 error: aborting due to previous error
 

src/test/ui/suggestions/adt-param-with-implicit-sized-bound.rs

@@ -0,0 +1,28 @@
+trait Trait {
+    fn func1() -> Struct1<Self>; //~ ERROR E0277
+    fn func2<'a>() -> Struct2<'a, Self>; //~ ERROR E0277
+    fn func3() -> Struct3<Self>; //~ ERROR E0277
+    fn func4() -> Struct4<Self>; //~ ERROR E0277
+}
+
+struct Struct1<T>{
+    _t: std::marker::PhantomData<*const T>,
+}
+struct Struct2<'a, T>{
+    _t: &'a T,
+}
+struct Struct3<T>{
+    _t: T,
+}
+
+struct X<T>(T);
+
+struct Struct4<T>{
+    _t: X<T>,
+}
+
+struct Struct5<T: ?Sized>{
+    _t: X<T>, //~ ERROR E0277
+}
+
+fn main() {}