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 not satisfied #125194

Closed
BoxyUwU opened this issue May 17, 2024 · 9 comments · Fixed by #127568
Closed

regression: trait bound not satisfied #125194

BoxyUwU opened this issue May 17, 2024 · 9 comments · Fixed by #127568
Labels
P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone
1.80.0

Comments

@BoxyUwU
Copy link
Member

BoxyUwU commented May 17, 2024 

[INFO] [stdout] error[E0277]: the trait bound `for<'a> beef::generic::Cow<'a, str, beef::wide::internal::Wide>: Borrow<beef::generic::Cow<'_, str, lean::internal::Lean>>` is not satisfied
[INFO] [stdout]    --> src/page.rs:433:18
[INFO] [stdout]     |
[INFO] [stdout] 433 |             .get(template)
[INFO] [stdout]     |              --- ^^^^^^^^ the trait `for<'a> Borrow<beef::generic::Cow<'_, str, lean::internal::Lean>>` is not implemented for `beef::generic::Cow<'a, str, beef::wide::internal::Wide>`
[INFO] [stdout]     |              |
[INFO] [stdout]     |              required by a bound introduced by this call
[INFO] [stdout]     |

likely #119820 cc @lcnr, this was intended breakage afaik
@BoxyUwU BoxyUwU 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 May 17, 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 May 17, 2024
@BoxyUwU BoxyUwU mentioned this issue May 17, 2024
Closed
@BoxyUwU BoxyUwU added this to the 1.79.0 milestone May 17, 2024
@lcnr
Copy link
Contributor

lcnr commented May 17, 2024

likely caused by #119820. I am considering to reenable the old behavior for one more beta version to give this more time on nightly. Doing so is straightforward and non-breaking

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 18, 2024
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-critical

@rustbot rustbot added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 21, 2024
@lcnr lcnr mentioned this issue May 27, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 4, 2024
@bors
Auto merge of rust-lang#125629 - lcnr:undo-leak-check-changes, r=comp…
81ed1ce 
…iler-errors

[BETA] revert leak check changes

reverts the behavior changes from rust-lang#119820 to give `sqlx` and other affected projects another 6 weeks. fixes rust-lang#125194
@wesleywiser wesleywiser added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jun 6, 2024
@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.79.0, 1.80.0 Jun 7, 2024
@Mark-Simulacrum
Copy link
Member

Moving milestones since we reverted in beta (#125629).

@lcnr lcnr added P-high High priority and removed P-critical Critical priority labels Jun 10, 2024
@lcnr
Copy link
Contributor

lcnr commented Jun 10, 2024

lowering priority to P-high. We will have to decide whether to push back the stabilization of this change by yet another version however 🤔

@apiraino
Copy link
Contributor

Changes in #119820 were reverted so in theory these regressions are neutralized (IIUC)

@apiraino apiraino mentioned this issue Jun 20, 2024
Closed
@lcnr
Copy link
Contributor

lcnr commented Jun 20, 2024

only reverted on the previous beta (now stable), it still affects nightly (and the new beta)

@wesleywiser
Copy link
Member

Do we want to ship this in 1.80 or revert for another release cycle?

@lcnr lcnr mentioned this issue Jul 10, 2024
Merged
@bors bors closed this as completed in 7faef5d Jul 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 10, 2024
@rust-timer
Unrolled build for rust-lang#127568
f30522e 
Rollup merge of rust-lang#127568 - lcnr:undo-leakcheck, r=oli-obk

instantiate higher ranked goals in candidate selection again

This reverts rust-lang#119820 as that PR has a significant impact and breaks code which *feels like it should work*. The impact ended up being larger than we expected during the FCP and we've ended up with some ideas for how we can work around this issue in the next solver. This has been discussed in the previous high bandwidth t-types meeting: https://rust-lang.zulipchat.com/#narrow/stream/326132-t-types.2Fmeetings/topic/2024-07-09.20high.20bandwidth.20meeting.

We'll therefore keep this inconsistency between the two solvers for now and will have to deal with it before stabilizating the use of the new solver outside of coherence: rust-lang/trait-system-refactor-initiative#120.

fixes rust-lang#125194 after a beta-backport.

The pattern which is more widely used than expected and feels like it should work, especially without deep knowledge of the type system is
```rust
trait Trait<'a> {}
impl<'a, T> Trait<'a> for T {}

fn trait_bound<T: for<'a> Trait<'a>>() {}

// A function with a where-bound which is more restrictive than the impl.
fn function1<T: Trait<'static>>() {
    // stable: ok
    // with rust-lang#119820: error as we prefer the where-bound over the impl
    // with this PR: back to ok
    trait_bound::<T>();
}

```

r? `@rust-lang/types`
@lcnr
Copy link
Contributor

lcnr commented Jul 11, 2024

reopening until the beta-revert

@lcnr lcnr reopened this Jul 11, 2024
@Mark-Simulacrum
Copy link
Member

backported in #127618

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.80.0
Development

Successfully merging a pull request may close this issue.

instantiate higher ranked goals in candidate selection again lcnr/rust
7 participants
@wesleywiser @Mark-Simulacrum @apiraino @saethlin @BoxyUwU @lcnr @rustbot