-Znext-solver=coherence: suggest increasing recursion limit
#121497
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
|
☔ The latest upstream changes (presumably #121549) made this pull request unmergeable. Please resolve the merge conflicts. |
lcnr
force-pushed
the
coherence-suggest-increasing-recursion-limit
branch
2 times, most recently
from
7b44eae to
8003148
Compare
|
☔ The latest upstream changes (presumably #119106) made this pull request unmergeable. Please resolve the merge conflicts. |
lcnr
force-pushed
the
coherence-suggest-increasing-recursion-limit
branch
from
8003148 to
327043d
Compare
This comment has been minimized.
This comment has been minimized.
lcnr
force-pushed
the
coherence-suggest-increasing-recursion-limit
branch
from
327043d to
eeba944
Compare
| fn impl_intersection_has_impossible_obligation<'a, 'cx, 'tcx>( | ||
| selcx: &mut SelectionContext<'cx, 'tcx>, | ||
| obligations: &'a [PredicateObligation<'tcx>], | ||
| ) -> Option<PredicateObligation<'tcx>> { | ||
| ) -> Result<(), Vec<ty::Predicate<'tcx>>> { |
There was a problem hiding this comment.
This logic is really confusing. Please turn this into an enum with proper states. Like, I cannot grasp the control flow between Err(vec![]) vs Err(vec![elements, inside]) for overflow vs Ok(()).
There was a problem hiding this comment.
the controlflow for Err(vec![]) and Err(vec![elements, inside]) is identical 🤔 changed it to a newtype instead
| ( | ||
| MaybeCause::Overflow { suggest_increasing_limit: a }, | ||
| MaybeCause::Overflow { suggest_increasing_limit: b }, | ||
| ) => MaybeCause::Overflow { suggest_increasing_limit: a || b }, |
There was a problem hiding this comment.
We don't suggest increasing the limit if we hit a true cycle, correct? I feel like this should be &&, because increasing the limit here when one of the branches is false (i.e. cycle) will not have a positive effect.
There was a problem hiding this comment.
Or hitting the fixpoint limit, which also won't be improved by increasing the limit.
There was a problem hiding this comment.
it will, in case one of the goals fails or has no constraints, if we have two nested goals:
- non recursion_limit cycle
- recursion_limit increase -> error
then we should suggest increasing the recursion limit as it would change the result to NoSolution which may then remove ambiguity elsewhere
rustbot
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #121489) made this pull request unmergeable. Please resolve the merge conflicts. |
also change the number of allowed fixpoint steps to be fixed instead of using the `log` of the total recursion depth.
lcnr
force-pushed
the
coherence-suggest-increasing-recursion-limit
branch
from
eeba944 to
8c5e83d
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#111505 (Made `INVALID_DOC_ATTRIBUTES` lint deny by default) - rust-lang#120305 (Delete line if suggestion would replace it with an empty line) - rust-lang#121153 (Suggest removing superfluous semicolon when statements used as expression) - rust-lang#121497 (`-Znext-solver=coherence`: suggest increasing recursion limit) - rust-lang#121634 (Clarify behavior of slice prefix/suffix operations in case of equality) - rust-lang#121706 (match lowering: Remove hacky branch in sort_candidate) - rust-lang#121730 (Add profiling support to AIX) - rust-lang#121750 (match lowering: Separate the `bool` case from other integers in `TestKind`) - rust-lang#121803 (Never say "`Trait` is implemented for `{type error}`") - rust-lang#121811 (Move sanitizer ui tests to sanitizer directory) - rust-lang#121824 (Implement missing ABI structures in StableMIR) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#121497 - lcnr:coherence-suggest-increasing-recursion-limit, r=compiler-errors `-Znext-solver=coherence`: suggest increasing recursion limit r? `@compiler-errors`
r? @compiler-errors