-
Notifications
You must be signed in to change notification settings - Fork 0
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
ambiguity on inductive cycles #20
Labels
Comments
This comment was marked as outdated.
This comment was marked as outdated.
lcnr
added
the
A-coherence
Having to do with regressions in `-Ztrait-solver=next-coherence`
label
Jul 24, 2023
20 tasks
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jan 8, 2024
…=lcnr Make inductive cycles in coherence ambiguous always Logical conclusion of rust-lang#114040 One step after rust-lang#116493 cc rust-lang/trait-system-refactor-initiative#20 r? lcnr to kick off the FCP after review... maybe we should wait until 1.75 is landed? In that case, I'd still like to get the FCP boxes checked sooner since that'll be near the holidays which means everyone's away.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jan 9, 2024
…=lcnr Make inductive cycles in coherence ambiguous always Logical conclusion of rust-lang#114040 One step after rust-lang#116493 cc rust-lang/trait-system-refactor-initiative#20 r? lcnr to kick off the FCP after review... maybe we should wait until 1.75 is landed? In that case, I'd still like to get the FCP boxes checked sooner since that'll be near the holidays which means everyone's away.
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this issue
Jan 9, 2024
Rollup merge of rust-lang#118649 - compiler-errors:coherence-ambig, r=lcnr Make inductive cycles in coherence ambiguous always Logical conclusion of rust-lang#114040 One step after rust-lang#116493 cc rust-lang/trait-system-refactor-initiative#20 r? lcnr to kick off the FCP after review... maybe we should wait until 1.75 is landed? In that case, I'd still like to get the FCP boxes checked sooner since that'll be near the holidays which means everyone's away.
@compiler-errors tried to also change this outside of coherence, resulting in 21 crater regressions apparently rust-lang/rust#116494 we should minimize the affected crates and FCP merge this in the old solver, this should simplify the regression to the new solver |
since rust-lang/rust#118649 this is now the default behavior in coherence, even with the old solver, still affects trait solving outside of it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
We now always fail with ambiguity on inductive cycles, even in the old solver rust-lang/rust#118649
The old solver returns
EvaluatedToRecur
on inductive cycles which allows candidates with such cycles to be dropped, allowing the following example to compile:more importantly, this affects coherence, e.g.
interval-map
:Returning
NoSolution
in the new solver would be unsound because the cycle may be with different inference variables due to canonicalization. For coinductive cycles we rerun the goal and continuously update the provisional result. Doing so for inductive cycles as well is non-trivial so I would like to ideally avoid it.The new solver instead treats inductive cycles as overflow, meaning that the above test fails. This will end up breaking anyways once we change traits to be coinductive by default. It therefore seems desirable to prevent people from relying on that behavior as soon as possible.
The text was updated successfully, but these errors were encountered: