Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On E0277 be clearer about implicit Sized bounds on type params and assoc types #120323

Merged
merged 4 commits into from
Feb 5, 2024

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jan 24, 2024

error[E0277]: the size for values of type `[i32]` cannot be known at compilation time
   --> f100.rs:2:33
    |
2   |     let _ = std::mem::size_of::<[i32]>();
    |                                 ^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `Sized` is not implemented for `[i32]`
note: required by an implicit `Sized` bound in `std::mem::size_of`
   --> /home/gh-estebank/rust/library/core/src/mem/mod.rs:312:22
    |
312 | pub const fn size_of<T>() -> usize {
    |                      ^ required by the implicit `Sized` requirement on this type parameter in `size_of`

Fix #120178.

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2024

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 24, 2024
@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor Author

estebank commented Jan 25, 2024

This change as is affects incremental compilation, making the change fn foo<T>() {} to fn foo<T: Sized>() {} now be picked up as a change. Haven't looked at the incr compilation machinery in a while, so I'm not sure how long it might take me to fix that.

Edit: figured out what the issue is. My change now only adds the implicit Sized obligation if there isn't an explicit one already. When testing incr comp, the first few cases use a flag to ignore spans, but because the predicate list doesn't change (there's always 1 Sized obligation, and it doesn't have Span state), adding an explicit Sized bound doesn't trigger a recompilation requirement (as long as spans are ignored).

@bors

This comment was marked as resolved.

@fmease
Copy link
Member

fmease commented Jan 26, 2024

I will review this tomorrow together with your other PR :)

r? fmease

@rustbot rustbot assigned fmease and unassigned b-naber Jan 26, 2024
@fmease fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2024
@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 30, 2024
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@fmease
Copy link
Member

fmease commented Jan 30, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 30, 2024

📌 Commit 43fdb12 has been approved by fmease

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2024
Nadrieril added a commit to Nadrieril/rust that referenced this pull request Jan 31, 2024
On E0277 be clearer about implicit `Sized` bounds on type params and assoc types

```
error[E0277]: the size for values of type `[i32]` cannot be known at compilation time
   --> f100.rs:2:33
    |
2   |     let _ = std::mem::size_of::<[i32]>();
    |                                 ^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `Sized` is not implemented for `[i32]`
note: required by an implicit `Sized` bound in `std::mem::size_of`
   --> /home/gh-estebank/rust/library/core/src/mem/mod.rs:312:22
    |
312 | pub const fn size_of<T>() -> usize {
    |                      ^ required by the implicit `Sized` requirement on this bound in `size_of`
```

Fix rust-lang#120178.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2024
Rollup of 12 pull requests

Successful merges:

 - rust-lang#120207 (check `RUST_BOOTSTRAP_CONFIG` in `profile_user_dist` test)
 - rust-lang#120321 (pattern_analysis: cleanup the contexts)
 - rust-lang#120323 (On E0277 be clearer about implicit `Sized` bounds on type params and assoc types)
 - rust-lang#120355 (document `FromIterator for Vec` allocation behaviors)
 - rust-lang#120396 (Account for unbounded type param receiver in suggestions)
 - rust-lang#120430 (std: thread_local::register_dtor fix proposal for FreeBSD.)
 - rust-lang#120435 (Suggest name value cfg when only value is used for check-cfg)
 - rust-lang#120470 (Mark "unused binding" suggestion as maybe incorrect)
 - rust-lang#120472 (Make duplicate lang items fatal)
 - rust-lang#120490 (Don't hash lints differently to non-lints.)
 - rust-lang#120495 (Remove the `abi_amdgpu_kernel` feature)
 - rust-lang#120501 (rustdoc: Correctly handle attribute merge if this is a glob reexport)

Failed merges:

 - rust-lang#120346 (hir: Refactor getters for owner nodes)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Jan 31, 2024

☔ The latest upstream changes (presumably #120346) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 31, 2024
…assoc types

```
error[E0277]: the size for values of type `[i32]` cannot be known at compilation time
   --> f100.rs:2:33
    |
2   |     let _ = std::mem::size_of::<[i32]>();
    |                                 ^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `Sized` is not implemented for `[i32]`
note: required by an implicit `Sized` bound in `std::mem::size_of`
   --> /home/gh-estebank/rust/library/core/src/mem/mod.rs:312:22
    |
312 | pub const fn size_of<T>() -> usize {
    |                      ^ required by the implicit `Sized` requirement on this bound in `size_of`
```

Fix rust-lang#120178.
Given the previous change to add implicit `Sized` bounds only if there
isn't already an explicit `Sized` bound, now the incr comp machinery
doesn't consider adding the explicit bound as being dirty, as long as
`-Zincremental-ignore-spans` is set.
@fmease
Copy link
Member

fmease commented Feb 1, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Feb 1, 2024

📌 Commit 8b0ab54 has been approved by fmease

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Feb 1, 2024

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 1, 2024
@rust-log-analyzer

This comment has been minimized.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 1, 2024
On E0277 be clearer about implicit `Sized` bounds on type params and assoc types

```
error[E0277]: the size for values of type `[i32]` cannot be known at compilation time
   --> f100.rs:2:33
    |
2   |     let _ = std::mem::size_of::<[i32]>();
    |                                 ^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `Sized` is not implemented for `[i32]`
note: required by an implicit `Sized` bound in `std::mem::size_of`
   --> /home/gh-estebank/rust/library/core/src/mem/mod.rs:312:22
    |
312 | pub const fn size_of<T>() -> usize {
    |                      ^ required by the implicit `Sized` requirement on this bound in `size_of`
```

Fix rust-lang#120178.
@estebank
Copy link
Contributor Author

estebank commented Feb 1, 2024

@bors r=fmease

@bors
Copy link
Contributor

bors commented Feb 1, 2024

📌 Commit 0df6dfd has been approved by fmease

It is now in the queue for this repository.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 1, 2024
On E0277 be clearer about implicit `Sized` bounds on type params and assoc types

```
error[E0277]: the size for values of type `[i32]` cannot be known at compilation time
   --> f100.rs:2:33
    |
2   |     let _ = std::mem::size_of::<[i32]>();
    |                                 ^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `Sized` is not implemented for `[i32]`
note: required by an implicit `Sized` bound in `std::mem::size_of`
   --> /home/gh-estebank/rust/library/core/src/mem/mod.rs:312:22
    |
312 | pub const fn size_of<T>() -> usize {
    |                      ^ required by the implicit `Sized` requirement on this bound in `size_of`
```

Fix rust-lang#120178.
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119759 (Add FileCheck annotations to dataflow-const-prop tests)
 - rust-lang#120323 (On E0277 be clearer about implicit `Sized` bounds on type params and assoc types)
 - rust-lang#120473 (Only suggest removal of `as_*` and `to_` conversion methods on E0308)
 - rust-lang#120520 (Some cleanups around diagnostic levels.)
 - rust-lang#120540 (add test for try-block-in-match-arm)
 - rust-lang#120547 (`#![feature(inline_const_pat)]` is no longer incomplete)
 - rust-lang#120552 (Correctly check `never_type` feature gating)
 - rust-lang#120555 (put pnkfelix (me) back on the review queue.)
 - rust-lang#120556 (Improve the diagnostics for unused generic parameters)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119759 (Add FileCheck annotations to dataflow-const-prop tests)
 - rust-lang#120323 (On E0277 be clearer about implicit `Sized` bounds on type params and assoc types)
 - rust-lang#120473 (Only suggest removal of `as_*` and `to_` conversion methods on E0308)
 - rust-lang#120520 (Some cleanups around diagnostic levels.)
 - rust-lang#120540 (add test for try-block-in-match-arm)
 - rust-lang#120547 (`#![feature(inline_const_pat)]` is no longer incomplete)
 - rust-lang#120552 (Correctly check `never_type` feature gating)
 - rust-lang#120555 (put pnkfelix (me) back on the review queue.)
 - rust-lang#120556 (Improve the diagnostics for unused generic parameters)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#119759 (Add FileCheck annotations to dataflow-const-prop tests)
 - rust-lang#120323 (On E0277 be clearer about implicit `Sized` bounds on type params and assoc types)
 - rust-lang#120473 (Only suggest removal of `as_*` and `to_` conversion methods on E0308)
 - rust-lang#120540 (add test for try-block-in-match-arm)
 - rust-lang#120547 (`#![feature(inline_const_pat)]` is no longer incomplete)
 - rust-lang#120552 (Correctly check `never_type` feature gating)
 - rust-lang#120555 (put pnkfelix (me) back on the review queue.)
 - rust-lang#120556 (Improve the diagnostics for unused generic parameters)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6af0458 into rust-lang:master Feb 5, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2024
Rollup merge of rust-lang#120323 - estebank:issue-120178, r=fmease

On E0277 be clearer about implicit `Sized` bounds on type params and assoc types

```
error[E0277]: the size for values of type `[i32]` cannot be known at compilation time
   --> f100.rs:2:33
    |
2   |     let _ = std::mem::size_of::<[i32]>();
    |                                 ^^^^^ doesn't have a size known at compile-time
    |
    = help: the trait `Sized` is not implemented for `[i32]`
note: required by an implicit `Sized` bound in `std::mem::size_of`
   --> /home/gh-estebank/rust/library/core/src/mem/mod.rs:312:22
    |
312 | pub const fn size_of<T>() -> usize {
    |                      ^ required by the implicit `Sized` requirement on this bound in `size_of`
```

Fix rust-lang#120178.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 2, 2024
… r=estebank

Make sure to insert `Sized` bound first into clauses list

rust-lang#120323 made it so that we don't insert an implicit `Sized` bound whenever we see an *explicit* `Sized` bound. However, since the code that inserts implicit sized bounds puts the bound as the *first* in the list, that means that it had the **side-effect** of possibly meaning we check `Sized` *after* checking other trait bounds.

If those trait bounds result in ambiguity or overflow or something, it may change how we winnow candidates. (**edit: SEE** rust-lang#123303) This is likely the cause for the regression in rust-lang#123279 (comment), since the impl...

```rust
impl<T: Job + Sized> AsJob for T { // <----- changing this to `Sized + Job` or just `Job` (which turns into `Sized + Job`) will FIX the issue.
}
```

...looks incredibly suspicious.

Fixes [after beta-backport] rust-lang#123279.

Alternative is to revert rust-lang#120323. I don't have a strong opinion about this, but think it may be nice to keep the diagnostic changes around.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 2, 2024
… r=estebank

Make sure to insert `Sized` bound first into clauses list

rust-lang#120323 made it so that we don't insert an implicit `Sized` bound whenever we see an *explicit* `Sized` bound. However, since the code that inserts implicit sized bounds puts the bound as the *first* in the list, that means that it had the **side-effect** of possibly meaning we check `Sized` *after* checking other trait bounds.

If those trait bounds result in ambiguity or overflow or something, it may change how we winnow candidates. (**edit: SEE** rust-lang#123303) This is likely the cause for the regression in rust-lang#123279 (comment), since the impl...

```rust
impl<T: Job + Sized> AsJob for T { // <----- changing this to `Sized + Job` or just `Job` (which turns into `Sized + Job`) will FIX the issue.
}
```

...looks incredibly suspicious.

Fixes [after beta-backport] rust-lang#123279.

Alternative is to revert rust-lang#120323. I don't have a strong opinion about this, but think it may be nice to keep the diagnostic changes around.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 2, 2024
Rollup merge of rust-lang#123302 - compiler-errors:sized-bound-first, r=estebank

Make sure to insert `Sized` bound first into clauses list

rust-lang#120323 made it so that we don't insert an implicit `Sized` bound whenever we see an *explicit* `Sized` bound. However, since the code that inserts implicit sized bounds puts the bound as the *first* in the list, that means that it had the **side-effect** of possibly meaning we check `Sized` *after* checking other trait bounds.

If those trait bounds result in ambiguity or overflow or something, it may change how we winnow candidates. (**edit: SEE** rust-lang#123303) This is likely the cause for the regression in rust-lang#123279 (comment), since the impl...

```rust
impl<T: Job + Sized> AsJob for T { // <----- changing this to `Sized + Job` or just `Job` (which turns into `Sized + Job`) will FIX the issue.
}
```

...looks incredibly suspicious.

Fixes [after beta-backport] rust-lang#123279.

Alternative is to revert rust-lang#120323. I don't have a strong opinion about this, but think it may be nice to keep the diagnostic changes around.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wonky diagnostics on unsatisfied implicit Sized bounds
6 participants