Suggest ?Sized when applicable for ADTs
#73261
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
cc @lcnr |
This comment has been minimized.
This comment has been minimized.
src/test/ui/suggestions/adt-param-with-implicit-sized-bound.stderr
Outdated
Show resolved
Hide resolved
| hir::intravisit::NestedVisitorMap::None | ||
| } | ||
|
|
||
| fn visit_ty(&mut self, ty: &hir::Ty<'_>) { |
There was a problem hiding this comment.
So, this analysis doesn't seem right. For example, consider an example like this:
struct Foo<T> {
x: Vec<T>, // requires `T: Sized`
y: Box<T>, // does not require `T: Sized`
}if I'm reading the code correctly, it is going to highlight both of those uses of T?
Doing this correctly is going to be a touch tricky at least, but we could make it more precise in various ways. For example, we could go over the list of fields and look at their Ty<'tcx> type to see if its WF conditions require that T: Sized, and only then walk over the HIR for the type.
There was a problem hiding this comment.
We now only point at the uses of T when it is bare, otherwise we assume that they allow ?Sized. That example will suggest T: ?Sized:
error[E0277]: the size for values of type `T` cannot be known at compilation time
--> file6.rs:6:21
|
1 | struct Foo<T> {
| - required by this bound in `Foo`
...
6 | struct X<T: ?Sized>(Foo<T>);
| - ^^^^^^ doesn't have a size known at compile-time
| |
| this type parameter needs to be `std::marker::Sized`
|
= help: the trait `std::marker::Sized` is not implemented for `T`
= 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
|
1 | struct Foo<T: ?Sized> {
| ^^^^^^^^
When applying it you get
error[E0277]: the size for values of type `T` cannot be known at compilation time
--> file6.rs:2:5
|: aborting due to previous error
1 | struct Foo<T: ?Sized> {
| - this type parameter needs to be `std::marker::Sized`
2 | x: Vec<T>,
| ^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `T`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
I am indeed not verifying whether Vec or Box have a T: ?Sized or T: Sized constraint. That would be a good thing to add, but I feel that the current behavior is good enough before adding more code to probe Vec and Box to identify what their bounds are.
There was a problem hiding this comment.
OK -- can you maybe add some comments into the code explaining that a bit? i.e., what is the goal of this visitor and can you clarify where it makes 'safe assumptions'?
|
@bors r+ |
|
📌 Commit 40b27ff has been approved by |
bors
added
S-waiting-on-bors
…arth Rollup of 13 pull requests Successful merges: - rust-lang#71568 (Document unsafety in slice/sort.rs) - rust-lang#72709 (`#[deny(unsafe_op_in_unsafe_fn)]` in liballoc) - rust-lang#73214 (Add asm!() support for hexagon) - rust-lang#73248 (save_analysis: improve handling of enum struct variant) - rust-lang#73257 (ty: projections in `transparent_newtype_field`) - rust-lang#73261 (Suggest `?Sized` when applicable for ADTs) - rust-lang#73300 (Implement crate-level-only lints checking.) - rust-lang#73334 (Note numeric literals that can never fit in an expected type) - rust-lang#73357 (Use `LocalDefId` for import IDs in trait map) - rust-lang#73364 (asm: Allow multiple template string arguments; interpret them as newline-separated) - rust-lang#73382 (Only display other method receiver candidates if they actually apply) - rust-lang#73465 (Add specialization of `ToString for char`) - rust-lang#73489 (Refactor hir::Place) Failed merges: r? @ghost
Address #71790, fix #27964.