-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
trait_selection: Keep type-op region constraints in borrowck #161423
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 |
|---|---|---|
|
|
@@ -76,13 +76,21 @@ pub struct QueryResponse<'tcx, R> { | |
| pub value: R, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] | ||
| #[derive(Clone, Debug, Default, PartialEq, Hash)] | ||
| #[derive(StableHash, TypeFoldable, TypeVisitable)] | ||
| pub struct QueryRegionConstraints<'tcx> { | ||
| pub constraints: Vec<QueryRegionConstraint<'tcx>>, | ||
| pub assumptions: Vec<ty::ArgOutlivesClause<'tcx>>, | ||
| /// Region constraints emitted by the next solver under | ||
| /// `-Zassumptions-on-binders`. | ||
| /// | ||
| /// These stay unspanned while passing through a canonical query. The type-op | ||
| /// caller attaches its origin span when consuming the response. | ||
| pub solver_constraints: ir::region_constraint::RegionConstraint<TyCtxt<'tcx>>, | ||
| } | ||
|
|
||
| impl Eq for QueryRegionConstraints<'_> {} | ||
|
|
||
| impl QueryRegionConstraints<'_> { | ||
| /// Represents an empty (trivially true) set of region constraints. | ||
| /// | ||
|
|
@@ -91,8 +99,16 @@ impl QueryRegionConstraints<'_> { | |
| /// discharge a requirement from another query, which is a potential problem if we did throw | ||
| /// away these assumptions because there were no constraints. | ||
| pub fn is_empty(&self) -> bool { | ||
| let QueryRegionConstraints { constraints, assumptions } = self; | ||
|
Member
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. the destructuring here is intentional. it means that adding new fields doesn't silently keep compiling and doing the wrong thing. please keep the |
||
| constraints.is_empty() && assumptions.is_empty() | ||
| self.constraints.is_empty() | ||
| && self.assumptions.is_empty() | ||
| && self.solver_constraints.is_true() | ||
| } | ||
|
|
||
| pub fn extend(&mut self, other: &Self) { | ||
| self.constraints.extend(other.constraints.iter().cloned()); | ||
| self.assumptions.extend(other.assumptions.iter().cloned()); | ||
| self.solver_constraints = | ||
| std::mem::take(&mut self.solver_constraints).and(other.solver_constraints.clone()); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,7 +82,11 @@ fn implied_outlives_bounds<'a, 'tcx>( | |
| // FIXME(higher_ranked_auto): Should we register assumptions here? | ||
| // We otherwise would get spurious errors if normalizing an implied | ||
| // outlives bound required proving some higher-ranked coroutine obl. | ||
| let QueryRegionConstraints { constraints, assumptions: _ } = constraints; | ||
| let QueryRegionConstraints { constraints, solver_constraints, .. } = constraints; | ||
| if !solver_constraints.is_true() { | ||
|
Member
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. what's with all of these checks for |
||
| infcx.register_solver_region_constraint(solver_constraints.with_span(span)); | ||
| } | ||
|
|
||
| let cause = ObligationCause::misc(span, body_def_id); | ||
| for &QueryRegionConstraint { constraint, visible_for_leak_check: vis, .. } in &constraints { | ||
| match constraint { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,8 +60,8 @@ impl<F> fmt::Debug for CustomTypeOp<F> { | |
| } | ||
| } | ||
|
|
||
| /// Executes `op` and then scrapes out all the "old style" region | ||
| /// constraints that result, creating query-region-constraints. | ||
| /// Executes `op` and then scrapes out all resulting region constraints, | ||
| /// creating query-region-constraints. | ||
| pub fn scrape_region_constraints<'tcx, Op, R>( | ||
| infcx: &InferCtxt<'tcx>, | ||
| root_def_id: LocalDefId, | ||
|
|
@@ -89,10 +89,11 @@ where | |
| "scrape_region_constraints: incoming region assumptions = {pre_assumptions:#?}", | ||
| ); | ||
|
|
||
| let value = infcx.commit_if_ok(|_| { | ||
| let ocx = ObligationCtxt::new(infcx); | ||
| let value = op(&ocx).map_err(|_| { | ||
| infcx.tcx.check_potentially_region_dependent_goals(root_def_id).err().unwrap_or_else( | ||
| let (value, solver_constraints) = infcx.with_fresh_solver_region_constraints(|| { | ||
| infcx.commit_if_ok(|_| { | ||
| let ocx = ObligationCtxt::new(infcx); | ||
| let value = op(&ocx).map_err(|_| { | ||
| infcx.tcx.check_potentially_region_dependent_goals(root_def_id).err().unwrap_or_else( | ||
| // FIXME: In this region-dependent context, `type_op` should only fail due to | ||
| // region-dependent goals. Any other kind of failure indicates a bug and we | ||
| // should ICE. | ||
|
|
@@ -125,31 +126,36 @@ where | |
| .dcx() | ||
| .span_delayed_bug(span, format!("error performing operation: {name}")) | ||
| }, | ||
| ) | ||
| })?; | ||
| let errors = ocx.evaluate_obligations_error_on_ambiguity(); | ||
| if errors.no_errors() { | ||
| Ok(value) | ||
| } else if let Err(guar) = infcx.tcx.check_potentially_region_dependent_goals(root_def_id) { | ||
| Err(guar) | ||
| } else { | ||
| Err(infcx.dcx().delayed_bug(format!( | ||
| "errors selecting obligation during MIR typeck: {name} {root_def_id:?} {errors:?}" | ||
| ))) | ||
| } | ||
| })?; | ||
| ) | ||
| })?; | ||
| let errors = ocx.evaluate_obligations_error_on_ambiguity(); | ||
| if errors.no_errors() { | ||
| Ok(value) | ||
| } else if let Err(guar) = | ||
| infcx.tcx.check_potentially_region_dependent_goals(root_def_id) | ||
| { | ||
| Err(guar) | ||
| } else { | ||
| Err(infcx.dcx().delayed_bug(format!( | ||
| "errors selecting obligation during MIR typeck: {name} {root_def_id:?} {errors:?}" | ||
| ))) | ||
| } | ||
| }) | ||
| }); | ||
| let value = value?; | ||
|
|
||
| // Next trait solver performs operations locally, and normalize goals should resolve vars. | ||
| let value = infcx.resolve_vars_if_possible(value); | ||
|
|
||
| let region_obligations = infcx.take_registered_region_obligations(); | ||
| let region_assumptions = infcx.take_registered_region_assumptions(); | ||
| let region_constraint_data = infcx.take_and_reset_region_constraints(); | ||
| let region_constraints = query_response::make_query_region_constraints( | ||
| let mut region_constraints = query_response::make_query_region_constraints( | ||
| region_obligations, | ||
| ®ion_constraint_data, | ||
| region_assumptions, | ||
| ); | ||
| region_constraints.solver_constraints = solver_constraints; | ||
|
Member
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. this generally feels kinda off, why doesn't this just work the same way as or |
||
|
|
||
| if region_constraints.is_empty() { | ||
| Ok(( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
why is this necessary for new style constraints but not old style?
View changes since the review