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.

Sign up for GitHub

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

Jump to bottom

regression: trait bound is not satisfied #123279

Closed
Mark-Simulacrum opened this issue Mar 31, 2024 · 5 comments
Closed

regression: trait bound is not satisfied #123279

Mark-Simulacrum opened this issue Mar 31, 2024 · 5 comments
Assignees
@compiler-errors
Labels
A-traits Area: Trait system P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone
1.78.0

Comments

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Mar 31, 2024 •
edited by rustbot 

[INFO] [stdout] error[E0277]: the trait bound `(dyn LoopableJob + 'static): Job` is not satisfied
[INFO] [stdout]    --> src/service.rs:190:18
[INFO] [stdout]     |
[INFO] [stdout] 190 |         self.job.run_once().await
[INFO] [stdout]     |                  ^^^^^^^^ the trait `Job` is not implemented for `(dyn LoopableJob + 'static)`
[INFO] [stdout]     |
[INFO] [stdout] help: consider introducing a `where` clause, but there might be an alternative better way to express this requirement
[INFO] [stdout]     |
[INFO] [stdout] 188 | impl Job for LoopingJobService where (dyn LoopableJob + 'static): Job {
[INFO] [stdout]     |                                ++++++++++++++++++++++++++++++++++++++
@Mark-Simulacrum Mark-Simulacrum added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Mar 31, 2024
@Mark-Simulacrum Mark-Simulacrum added this to the 1.78.0 milestone Mar 31, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 31, 2024
This was referenced Mar 31, 2024
Closed
Closed
@compiler-errors
Copy link
Member

Regression in rust-lang-ci@e890bd9
The PR introducing the regression in this rollup is #120323: On E0277 be clearer about implicit Sized bounds on type p…

searched nightlies: from nightly-2024-01-01 to nightly-2024-03-18
regressed nightly: nightly-2024-02-06
searched commit range: 268dbbb...f067fd6
regressed commit: 4d87c4a

bisected with cargo-bisect-rustc v0.6.8

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2024-01-01 --end=2024-03-18

This PR definitely should not have changed behavior :/
cc @estebank

@compiler-errors compiler-errors added E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example and removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels Mar 31, 2024
@compiler-errors
Copy link
Member

I'll look into what's going on with it anyways -- it at least needs a MCVE

@rustbot claim

@compiler-errors
Copy link
Member

Implicitly reordering the Sized predicates as a side-effect of #120323 is the problem here.

pub trait Job: AsJob {
    fn run_once(&self);
}

impl<F: Fn()> Job for F {
    fn run_once(&self) {
        todo!()
    }
}

pub trait AsJob {
}

impl<T: Job + Sized> AsJob for T { // <----- changing this to `Sized + Job` or just `Job` will FIX it.
}

pub struct LoopingJobService {
    job: Box<dyn Job>,
}

impl Job for LoopingJobService {
    fn run_once(&self) {
        self.job.run_once()
    }
}

I assume this is because of an overflow on the AsJob impl that isn't cut off short by the Sized predicate being impossible to satisfy?

@compiler-errors compiler-errors mentioned this issue Mar 31, 2024
Merged
@apiraino
Copy link
Contributor

apiraino commented Apr 2, 2024

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Apr 2, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 2, 2024
@matthiaskrgr
Rollup merge of rust-lang#123302 - compiler-errors:sized-bound-first,…
843a367 
… 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 issue Apr 2, 2024
@matthiaskrgr
Rollup merge of rust-lang#123302 - compiler-errors:sized-bound-first,…
a38dde9 
… 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 issue Apr 2, 2024
@rust-timer
Unrolled build for rust-lang#123302
f8f2b86 
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.
@jieyouxu jieyouxu added A-traits Area: Trait system S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue and removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 3, 2024
@cuviper
Copy link
Member

cuviper commented Apr 12, 2024

#123302 was backported in #123466.

@cuviper cuviper closed this as completed Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@compiler-errors compiler-errors

Labels
A-traits Area: Trait system P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.78.0
Development

No branches or pull requests
6 participants
@cuviper @compiler-errors @Mark-Simulacrum @apiraino @jieyouxu @rustbot