-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
add a FCW for overflow errors with the next solver #159224
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ use std::{debug_assert_matches, fmt}; | |
|
|
||
| use rustc_errors::ErrorGuaranteed; | ||
| use rustc_hir as hir; | ||
| use rustc_hir::CRATE_HIR_ID; | ||
| use rustc_hir::def::{CtorKind, DefKind}; | ||
| use rustc_hir::def_id::{DefId, LocalDefId}; | ||
| use rustc_hir::lang_items::LangItem; | ||
|
|
@@ -786,8 +787,27 @@ impl<'tcx> Interner for TyCtxt<'tcx> { | |
| fn evaluate_root_goal_for_proof_tree_raw( | ||
| self, | ||
| canonical_goal: CanonicalInput<'tcx>, | ||
| root_depth: usize, | ||
| ) -> (QueryResult<'tcx>, &'tcx inspect::Probe<TyCtxt<'tcx>>) { | ||
| self.evaluate_root_goal_for_proof_tree_raw(canonical_goal) | ||
| self.evaluate_root_goal_for_proof_tree_raw((canonical_goal, root_depth)) | ||
| } | ||
|
|
||
| fn emit_next_solver_overflow_fcw(self, span: Option<Span>) { | ||
| self.emit_node_span_lint( | ||
| rustc_session::lint::builtin::NEXT_TRAIT_SOLVER_OVERFLOW, | ||
| CRATE_HIR_ID, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah yeah, I guess that works 👍 we should somehow adjust the error message to say that this lint is always attached to the whole crate and can't be disabled on a per function basis |
||
| span.unwrap_or(DUMMY_SP), | ||
| rustc_errors::DiagDecorator(|diag| { | ||
| diag.primary_message(format!( | ||
| "reached the recursion limit {} when resolving trait bounds", | ||
| self.recursion_limit() | ||
| )); | ||
| diag.help(format!( | ||
| "consider increasing it by adding a `#![recursion_limit = \"{}\"]`", | ||
| self.recursion_limit() * 2 | ||
| )); | ||
| }), | ||
| ) | ||
| } | ||
|
|
||
| fn item_name(self, id: DefId) -> Symbol { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -246,10 +246,40 @@ where | |
| return Ok(res); | ||
| } | ||
|
|
||
| let result = EvalCtxt::enter_root(self, self.cx().recursion_limit(), span, |ecx| { | ||
| // Fast paths handled above | ||
| ecx.evaluate_goal_no_fast_paths(GoalSource::Misc, goal) | ||
| }); | ||
| let eval_with_recursion_limit = |limit| { | ||
| EvalCtxt::enter_root(self, limit, span, |ecx| { | ||
| ecx.evaluate_goal_no_fast_paths(GoalSource::Misc, goal) | ||
| }) | ||
| }; | ||
| let is_overflow_and_has_no_stalled_infers = |eval_result: &Result<GoalEvaluation<I>, _>| { | ||
| let predicate = match eval_result { | ||
| Err(_) => return false, | ||
| Ok(goal_evaluation) if !goal_evaluation.certainty.is_overflow() => return false, | ||
| Ok(goal_evaluation) => goal_evaluation.goal.predicate, | ||
| }; | ||
|
|
||
| let has_no_stalled_infers = match predicate.kind().skip_binder() { | ||
| ty::PredicateKind::Clause(ty::ClauseKind::Projection(projection)) => { | ||
| !projection.projection_term.has_non_region_infer() | ||
| } | ||
| _ => !predicate.has_non_region_infer(), | ||
| }; | ||
| has_no_stalled_infers | ||
| }; | ||
|
|
||
| // The old solver doesn't check depth requirement when looking up cache | ||
| // while the next solver does so. Thus the next solver is more prone to | ||
| // overflow. We emit a FCW for this. | ||
| let mut result = eval_with_recursion_limit(self.cx().recursion_limit()); | ||
| if is_overflow_and_has_no_stalled_infers(&result) { | ||
| let new_result = eval_with_recursion_limit(self.cx().recursion_limit() * 2); | ||
| if let Ok(goal_evaluation) = &new_result | ||
| && goal_evaluation.certainty.is_yes() | ||
| { | ||
| self.cx().emit_next_solver_overflow_fcw(Some(span)); | ||
| result = new_result; | ||
| } | ||
| } | ||
|
Comment on lines
+249
to
+282
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you move this into a separate functions, e.g. can you do the reevaluation in a |
||
|
|
||
| match result { | ||
| Ok(i) => Ok(i), | ||
|
|
@@ -302,7 +332,45 @@ where | |
| goal: Goal<I, I::Predicate>, | ||
| span: I::Span, | ||
| ) -> (Result<NestedNormalizationGoals<I>, NoSolution>, inspect::GoalEvaluation<I>) { | ||
| evaluate_root_goal_for_proof_tree(self, goal, span) | ||
| let is_overflow_and_has_no_stalled_infers = | ||
| |goal_evaluation: &inspect::GoalEvaluation<I>| { | ||
| match goal_evaluation.result { | ||
| Err(_) => return false, | ||
| Ok(response) if !response.value.certainty.is_overflow() => return false, | ||
| Ok(_) => {} | ||
| } | ||
|
|
||
| let predicate: I::Predicate = goal_evaluation.uncanonicalized_goal.predicate; | ||
| let has_no_stalled_infers = match predicate.kind().skip_binder() { | ||
| ty::PredicateKind::Clause(ty::ClauseKind::Projection(projection)) => { | ||
| !projection.projection_term.has_non_region_infer() | ||
| } | ||
| _ => !predicate.has_non_region_infer(), | ||
| }; | ||
| has_no_stalled_infers | ||
| }; | ||
|
|
||
| // The old solver doesn't check depth requirement when looking up cache | ||
| // while the next solver does so. Thus the next solver is more prone to | ||
| // overflow. We emit a FCW for this. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. link to an issue/the lint track issue/somewhere here |
||
| let (mut result, mut goal_evaluation) = | ||
| evaluate_root_goal_for_proof_tree(self, goal, span, self.cx().recursion_limit()); | ||
| if is_overflow_and_has_no_stalled_infers(&goal_evaluation) { | ||
| let (new_result, new_goal_evaluation) = evaluate_root_goal_for_proof_tree( | ||
| self, | ||
| goal, | ||
| span, | ||
| self.cx().recursion_limit() * 2, | ||
| ); | ||
| if let Ok(response) = &new_goal_evaluation.result | ||
| && response.value.certainty.is_yes() | ||
| { | ||
| self.cx().emit_next_solver_overflow_fcw(Some(span)); | ||
| result = new_result; | ||
| goal_evaluation = new_goal_evaluation; | ||
| } | ||
| } | ||
| (result, goal_evaluation) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1701,11 +1769,12 @@ pub fn evaluate_root_goal_for_proof_tree_raw_provider< | |
| >( | ||
| cx: I, | ||
| canonical_goal: CanonicalInput<I>, | ||
| root_depth: usize, | ||
| ) -> (QueryResult<I>, I::Probe) { | ||
| let mut inspect = inspect::ProofTreeBuilder::new(); | ||
| let (canonical_result, accessed_opaques) = SearchGraph::<D>::evaluate_root_goal_for_proof_tree( | ||
| cx, | ||
| cx.recursion_limit(), | ||
| root_depth, | ||
| canonical_goal, | ||
| &mut inspect, | ||
| ); | ||
|
|
@@ -1723,6 +1792,7 @@ pub(super) fn evaluate_root_goal_for_proof_tree<D: SolverDelegate<Interner = I>, | |
| delegate: &D, | ||
| goal: Goal<I, I::Predicate>, | ||
| origin_span: I::Span, | ||
| root_depth: usize, | ||
| ) -> (Result<NestedNormalizationGoals<I>, NoSolution>, inspect::GoalEvaluation<I>) { | ||
| let opaque_types = delegate.clone_opaque_types_lookup_table(); | ||
| let (goal, opaque_types) = eager_resolve_vars(&**delegate, (goal, opaque_types)); | ||
|
|
@@ -1732,7 +1802,7 @@ pub(super) fn evaluate_root_goal_for_proof_tree<D: SolverDelegate<Interner = I>, | |
| canonicalize_goal(delegate, goal, &opaque_types, typing_mode.into()); | ||
|
|
||
| let (canonical_result, final_revision) = | ||
| delegate.cx().evaluate_root_goal_for_proof_tree_raw(canonical_goal); | ||
| delegate.cx().evaluate_root_goal_for_proof_tree_raw(canonical_goal, root_depth); | ||
|
|
||
| let proof_tree = inspect::GoalEvaluation { | ||
| uncanonicalized_goal: goal, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| warning: reached the recursion limit 8 when resolving trait bounds | ||
| --> $DIR/fcw-on-auto-trait.rs:22:5 | ||
| | | ||
| LL | require_sync::<Foo<Foo<Foo<Foo<Foo<Foo<()>>>>>>>(); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| = help: consider increasing it by adding a `#![recursion_limit = "16"]` | ||
| = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
| = note: for more information, see issue #159228 <https://github.com/rust-lang/rust/issues/159228> | ||
| = note: `#[warn(next_trait_solver_overflow)]` (part of `#[warn(future_incompatible)]`) on by default | ||
|
|
||
| warning: reached the recursion limit 8 when resolving trait bounds | ||
| --> $DIR/fcw-on-auto-trait.rs:22:5 | ||
| | | ||
| LL | require_sync::<Foo<Foo<Foo<Foo<Foo<Foo<()>>>>>>>(); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| = help: consider increasing it by adding a `#![recursion_limit = "16"]` | ||
| = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
| = note: for more information, see issue #159228 <https://github.com/rust-lang/rust/issues/159228> | ||
| = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` | ||
|
|
||
| warning: 2 warnings emitted | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| //@ revisions: old next | ||
| //@[next] compile-flags: -Znext-solver | ||
| //@ check-pass | ||
|
|
||
| // The old solver doesn't verify depth when looking up cache. | ||
| // To avoid breakage, we evaluate with higher recursion limit in the next solver | ||
| // and emit an FCW for this. | ||
| // See the `NEXT_TRAIT_SOLVER_OVERFLOW` FCW. | ||
|
|
||
| #![recursion_limit = "8"] | ||
|
|
||
| // The field order matters 😂 | ||
| #[allow(dead_code)] | ||
| struct Foo<T> { | ||
| t: T, | ||
| opt_t: Option<T>, | ||
| } | ||
|
|
||
| fn require_sync<T: Sync>() {} | ||
|
|
||
| fn main() { | ||
| require_sync::<Foo<Foo<Foo<Foo<Foo<Foo<()>>>>>>>(); | ||
| //[next]~^ WARN: reached the recursion limit 8 when resolving trait bounds [next_trait_solver_overflow] | ||
| //[next]~| WARN: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
| //[next]~| WARN: reached the recursion limit 8 when resolving trait bounds [next_trait_solver_overflow] | ||
| //[next]~| WARN: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
take a
Spanand supply aDUMMY_SPin the caller where necessary