-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Correctly register lifetime constraints from opaque types from canonical queries called from nll #123035
Correctly register lifetime constraints from opaque types from canonical queries called from nll #123035
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 |
---|---|---|
|
@@ -23,7 +23,7 @@ use rustc_index::IndexVec; | |
use rustc_middle::arena::ArenaAllocatable; | ||
use rustc_middle::mir::ConstraintCategory; | ||
use rustc_middle::ty::fold::TypeFoldable; | ||
use rustc_middle::ty::{self, BoundVar, Ty, TyCtxt}; | ||
use rustc_middle::ty::{self, BoundVar, OpaqueTypeKey, Ty, TyCtxt}; | ||
use rustc_middle::ty::{GenericArg, GenericArgKind}; | ||
use std::fmt::Debug; | ||
use std::iter; | ||
|
@@ -248,20 +248,69 @@ impl<'tcx> InferCtxt<'tcx> { | |
where | ||
R: Debug + TypeFoldable<TyCtxt<'tcx>>, | ||
{ | ||
let InferOk { value: result_args, mut obligations } = self | ||
.query_response_instantiation_guess( | ||
cause, | ||
param_env, | ||
original_values, | ||
query_response, | ||
)?; | ||
let (result_args, opaques) = | ||
self.query_response_instantiation_guess(cause, original_values, query_response); | ||
|
||
let mut obligations = vec![]; | ||
|
||
let constraint_category = cause.to_constraint_category(); | ||
|
||
// Carry all newly resolved opaque types to the caller's scope | ||
for &(a, b) in &opaques { | ||
let a = instantiate_value(self.tcx, &result_args, a); | ||
let b = instantiate_value(self.tcx, &result_args, b); | ||
debug!(?a, ?b, "constrain opaque type"); | ||
if let ty::Alias(ty::Opaque, alias_ty) = b.kind() | ||
&& alias_ty.def_id == a.def_id.into() | ||
{ | ||
// When an inference variable is registered as the hidden type of an opaque type, | ||
// it may end up resolved to the opaque type itself. | ||
// In that case, do not bubble up the opaque type, but instead just equate its generic params. | ||
assert_eq!(a.args.len(), alias_ty.args.len()); | ||
for (a, b) in a.args.iter().zip(alias_ty.args) { | ||
match (a.unpack(), b.unpack()) { | ||
(GenericArgKind::Lifetime(v_o), GenericArgKind::Lifetime(v_r)) => { | ||
// To make `v_o = v_r`, we emit `v_o: v_r` and `v_r: v_o`. | ||
if v_o != v_r { | ||
output_query_region_constraints.outlives.push(( | ||
ty::OutlivesPredicate(v_o.into(), v_r), | ||
constraint_category, | ||
)); | ||
output_query_region_constraints.outlives.push(( | ||
ty::OutlivesPredicate(v_r.into(), v_o), | ||
constraint_category, | ||
)); | ||
} | ||
} | ||
// FIXME: actually equate types and consts by just registering a AliasRelate obligations for the opaque type | ||
// just like the new solver does. Needs AliasRelate support in the old solver. | ||
_ => { | ||
if a != b { | ||
span_bug!( | ||
cause.span(), | ||
"self-referential opaque type can only differ in lifetime parameters, but got {a} != {b}" | ||
) | ||
} | ||
} | ||
} | ||
} | ||
} else { | ||
obligations.extend( | ||
self.handle_opaque_type( | ||
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. After #122077 this will become a |
||
Ty::new_opaque(self.tcx, a.def_id.into(), a.args), | ||
b, | ||
cause, | ||
param_env, | ||
)? | ||
.obligations, | ||
); | ||
} | ||
} | ||
|
||
// Compute `QueryOutlivesConstraint` values that unify each of | ||
// the original values `v_o` that was canonicalized into a | ||
// variable... | ||
|
||
let constraint_category = cause.to_constraint_category(); | ||
|
||
for (index, original_value) in original_values.var_values.iter().enumerate() { | ||
// ...with the value `v_r` of that variable from the query. | ||
let result_value = query_response.instantiate_projected(self.tcx, &result_args, |v| { | ||
|
@@ -360,25 +409,35 @@ impl<'tcx> InferCtxt<'tcx> { | |
original_values, query_response, | ||
); | ||
|
||
let mut value = self.query_response_instantiation_guess( | ||
cause, | ||
param_env, | ||
original_values, | ||
query_response, | ||
)?; | ||
let (result_args, opaques) = | ||
self.query_response_instantiation_guess(cause, original_values, query_response); | ||
|
||
let mut obligations = vec![]; | ||
|
||
for (key, hidden) in opaques { | ||
obligations.extend( | ||
self.at(cause, param_env) | ||
.eq( | ||
DefineOpaqueTypes::Yes, | ||
Ty::new_opaque(self.tcx, key.def_id.into(), key.args), | ||
hidden, | ||
)? | ||
.obligations, | ||
) | ||
} | ||
Comment on lines
+417
to
+427
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. all non-nll query callers still need their opaque types registered. After #122077 this will become a simple |
||
|
||
value.obligations.extend( | ||
obligations.extend( | ||
self.unify_query_response_instantiation_guess( | ||
cause, | ||
param_env, | ||
original_values, | ||
&value.value, | ||
&result_args, | ||
query_response, | ||
)? | ||
.into_obligations(), | ||
); | ||
|
||
Ok(value) | ||
Ok(InferOk { value: result_args, obligations }) | ||
} | ||
|
||
/// Given the original values and the (canonicalized) result from | ||
|
@@ -390,14 +449,13 @@ impl<'tcx> InferCtxt<'tcx> { | |
/// will instantiate fresh inference variables for each canonical | ||
/// variable instead. Therefore, the result of this method must be | ||
/// properly unified | ||
#[instrument(level = "debug", skip(self, param_env))] | ||
#[instrument(level = "debug", skip(self))] | ||
fn query_response_instantiation_guess<R>( | ||
&self, | ||
cause: &ObligationCause<'tcx>, | ||
param_env: ty::ParamEnv<'tcx>, | ||
original_values: &OriginalQueryValues<'tcx>, | ||
query_response: &Canonical<'tcx, QueryResponse<'tcx, R>>, | ||
) -> InferResult<'tcx, CanonicalVarValues<'tcx>> | ||
) -> (CanonicalVarValues<'tcx>, Vec<(OpaqueTypeKey<'tcx>, Ty<'tcx>)>) | ||
where | ||
R: Debug + TypeFoldable<TyCtxt<'tcx>>, | ||
{ | ||
|
@@ -497,32 +555,17 @@ impl<'tcx> InferCtxt<'tcx> { | |
), | ||
}; | ||
|
||
let mut obligations = vec![]; | ||
let mut opaques = vec![]; | ||
|
||
// Carry all newly resolved opaque types to the caller's scope | ||
for &(a, b) in &query_response.value.opaque_types { | ||
let a = instantiate_value(self.tcx, &result_args, a); | ||
let b = instantiate_value(self.tcx, &result_args, b); | ||
debug!(?a, ?b, "constrain opaque type"); | ||
// We use equate here instead of, for example, just registering the | ||
// opaque type's hidden value directly, because we may be instantiating | ||
// a query response that was canonicalized in an InferCtxt that had | ||
// a different defining anchor. In that case, we may have inferred | ||
// `NonLocalOpaque := LocalOpaque` but can only instantiate it in | ||
// the other direction as `LocalOpaque := NonLocalOpaque`. Using eq | ||
// here allows us to try both directions (in `InferCtxt::handle_opaque_type`). | ||
obligations.extend( | ||
self.at(cause, param_env) | ||
.eq( | ||
DefineOpaqueTypes::Yes, | ||
Ty::new_opaque(self.tcx, a.def_id.to_def_id(), a.args), | ||
b, | ||
)? | ||
.obligations, | ||
); | ||
opaques.push((a, b)); | ||
} | ||
|
||
Ok(InferOk { value: result_args, obligations }) | ||
(result_args, opaques) | ||
} | ||
|
||
/// Given a "guess" at the values for the canonical variables in | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
//@ check-pass | ||
|
||
use std::iter::FromIterator; | ||
|
||
struct DynamicAlt<P>(P); | ||
|
||
impl<P> FromIterator<P> for DynamicAlt<P> { | ||
fn from_iter<T: IntoIterator<Item = P>>(_iter: T) -> Self { | ||
loop {} | ||
} | ||
} | ||
|
||
fn owned_context<I, F>(_: F) -> impl FnMut(I) -> I { | ||
|i| i | ||
} | ||
|
||
trait Parser<I> {} | ||
|
||
impl<T, I> Parser<I> for T where T: FnMut(I) -> I {} | ||
|
||
fn alt<I, P: Parser<I>>(_: DynamicAlt<P>) -> impl FnMut(I) -> I { | ||
|i| i | ||
} | ||
|
||
fn rule_to_parser<'c>() -> impl Parser<&'c str> { | ||
move |input| { | ||
let v: Vec<()> = vec![]; | ||
alt(v.iter().map(|()| owned_context(rule_to_parser())).collect::<DynamicAlt<_>>())(input) | ||
} | ||
} | ||
|
||
fn main() {} |
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.
This is super hacky, but at least it's a high signal ICE instead of some hard-to-debug issues. We also can't work on the FIXME before we get rid of
DefiningAnchor
, so I think we should just roll with this for now and address it later.