-
-
Notifications
You must be signed in to change notification settings - Fork 15.6k
-Znext-solver Allow method calls on chains of assoc types of not-yet defined opaque types
#161414
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
24a9dfb
5fbbf9d
d673eac
ed9a5a1
6dda852
e148df7
ae38d45
1698fef
614b33f
5ae44bd
aab5d79
66ea362
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 |
|---|---|---|
|
|
@@ -419,7 +419,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
| } else { | ||
| ty::List::empty() | ||
| }; | ||
| let value = query::MethodAutoderefSteps { predefined_opaques_in_body, self_ty }; | ||
| let opaque_hidden_ty_bounds_in_body = if self.next_trait_solver() { | ||
| self.tcx.mk_opaque_hidden_ty_bounds_in_body_from_iter( | ||
| self.inner.borrow_mut().opaque_types().iter_opaque_hidden_ty_bounds(), | ||
| ) | ||
| } else { | ||
| ty::List::empty() | ||
| }; | ||
| let value = query::MethodAutoderefSteps { | ||
| predefined_opaques_in_body, | ||
| opaque_hidden_ty_bounds_in_body, | ||
| self_ty, | ||
| }; | ||
| let query_input = self | ||
| .canonicalize_query(ParamEnvAnd { param_env: self.param_env, value }, &mut orig_values); | ||
|
|
||
|
|
@@ -434,7 +445,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
| let infcx = &self.infcx; | ||
| let (ParamEnvAnd { param_env: _, value }, var_values) = | ||
| infcx.instantiate_canonical(span, &query_input.canonical); | ||
| let query::MethodAutoderefSteps { predefined_opaques_in_body: _, self_ty } = value; | ||
| let query::MethodAutoderefSteps { | ||
| predefined_opaques_in_body: _, | ||
| opaque_hidden_ty_bounds_in_body: _, | ||
| self_ty, | ||
| } = value; | ||
| debug!(?self_ty, ?query_input, "probe_op: Mode::Path"); | ||
| let prev_opaque_entries = self.inner.borrow_mut().opaque_types().num_entries(); | ||
| MethodAutoderefStepsResult { | ||
|
|
@@ -444,7 +459,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
| self_ty, | ||
| prev_opaque_entries, | ||
| ), | ||
| self_ty_is_opaque: false, | ||
| self_ty_is_hidden_ty_of_opaque: false, | ||
| autoderefs: 0, | ||
| from_unsafe_deref: false, | ||
| unsize: false, | ||
|
|
@@ -643,7 +658,12 @@ pub(crate) fn method_autoderef_steps<'tcx>( | |
| let (ref infcx, goal, inference_vars) = tcx.infer_ctxt().build_with_canonical(DUMMY_SP, &goal); | ||
| let ParamEnvAnd { | ||
| param_env, | ||
| value: query::MethodAutoderefSteps { predefined_opaques_in_body, self_ty }, | ||
| value: | ||
| query::MethodAutoderefSteps { | ||
| predefined_opaques_in_body, | ||
| opaque_hidden_ty_bounds_in_body, | ||
| self_ty, | ||
| }, | ||
| } = goal; | ||
| for (key, ty) in predefined_opaques_in_body { | ||
| let prev = infcx | ||
|
|
@@ -663,14 +683,15 @@ pub(crate) fn method_autoderef_steps<'tcx>( | |
| debug!(?key, ?ty, ?prev, "ignore duplicate in `opaque_types_storage`"); | ||
| } | ||
| } | ||
| infcx.add_opaque_hidden_ty_bounds_in_storage(opaque_hidden_ty_bounds_in_body); | ||
| let prev_opaque_entries = infcx.inner.borrow_mut().opaque_types().num_entries(); | ||
|
|
||
| // We accept not-yet-defined opaque types in the autoderef | ||
| // chain to support recursive calls. We do error if the final | ||
| // infer var is not an opaque. | ||
| let self_ty_is_opaque = |ty: Ty<'_>| { | ||
| let self_ty_is_hidden_ty_of_opaque = |ty: Ty<'_>| { | ||
| if let &ty::Infer(ty::TyVar(vid)) = ty.kind() { | ||
| infcx.has_opaques_with_sub_unified_hidden_type(vid) | ||
| infcx.has_hidden_types_of_opaques_modulo_sub_unification(vid) | ||
|
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. This might be obvious but I'm struggling to understand why we can detect the projection term via hidden infer var. 😢
Member
Author
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.
Yes, it is. So It is necessary to check the later as we have the cases with trait Foo {
fn foo(&self) {}
}
trait Bar {
type Assoc: Foo;
fn bar(&self) -> Self::Assoc {
loop {}
}
}
trait Baz {
type Assoc: Bar;
fn baz(&self) -> Self::Assoc {
loop {}
}
}
impl Foo for () {}
impl Bar for () {
type Assoc = ();
}
impl Baz for () {
type Assoc = ();
}
fn heck() -> impl Baz {
heck().baz().bar().foo()
}
fn main() {}
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.
In the method body,
Member
Author
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.
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. Finally got it. Thank you for the explanation!
Member
Author
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. Yeah, I really should fix those namings 😄 I hope most of the correctness things(except folding binders and some canonicalization things) might be fixed by now so I'll try the perf and the naming/comments sides soon
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. namings please :> I can see this is hard. |
||
| } else { | ||
| false | ||
| } | ||
|
|
@@ -712,7 +733,7 @@ pub(crate) fn method_autoderef_steps<'tcx>( | |
| ty, | ||
| prev_opaque_entries, | ||
| ), | ||
| self_ty_is_opaque: self_ty_is_opaque(ty), | ||
| self_ty_is_hidden_ty_of_opaque: self_ty_is_hidden_ty_of_opaque(ty), | ||
| autoderefs: d, | ||
| from_unsafe_deref: reached_raw_pointer, | ||
| unsize: false, | ||
|
|
@@ -736,7 +757,7 @@ pub(crate) fn method_autoderef_steps<'tcx>( | |
| ty, | ||
| prev_opaque_entries, | ||
| ), | ||
| self_ty_is_opaque: self_ty_is_opaque(ty), | ||
| self_ty_is_hidden_ty_of_opaque: self_ty_is_hidden_ty_of_opaque(ty), | ||
| autoderefs: d, | ||
| from_unsafe_deref: reached_raw_pointer, | ||
| unsize: false, | ||
|
|
@@ -753,14 +774,16 @@ pub(crate) fn method_autoderef_steps<'tcx>( | |
| }; | ||
| let final_ty = autoderef_via_deref.final_ty(); | ||
| let opt_bad_ty = match final_ty.kind() { | ||
| ty::Infer(ty::TyVar(_)) if !self_ty_is_opaque(final_ty) => Some(MethodAutoderefBadTy { | ||
| reached_raw_pointer, | ||
| ty: infcx.make_query_response_ignoring_pending_obligations( | ||
| inference_vars, | ||
| final_ty, | ||
| prev_opaque_entries, | ||
| ), | ||
| }), | ||
| ty::Infer(ty::TyVar(_)) if !self_ty_is_hidden_ty_of_opaque(final_ty) => { | ||
| Some(MethodAutoderefBadTy { | ||
| reached_raw_pointer, | ||
| ty: infcx.make_query_response_ignoring_pending_obligations( | ||
| inference_vars, | ||
| final_ty, | ||
| prev_opaque_entries, | ||
| ), | ||
| }) | ||
| } | ||
| ty::Error(_) => Some(MethodAutoderefBadTy { | ||
| reached_raw_pointer, | ||
| ty: infcx.make_query_response_ignoring_pending_obligations( | ||
|
|
@@ -777,7 +800,7 @@ pub(crate) fn method_autoderef_steps<'tcx>( | |
| Ty::new_slice(infcx.tcx, *elem_ty), | ||
| prev_opaque_entries, | ||
| ), | ||
| self_ty_is_opaque: false, | ||
| self_ty_is_hidden_ty_of_opaque: false, | ||
| autoderefs, | ||
| // this could be from an unsafe deref if we had | ||
| // a *mut/const [T; N] | ||
|
|
@@ -2304,10 +2327,10 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { | |
| } | ||
| } | ||
|
|
||
| // Check whether any opaque types in the autoderef chain have been | ||
| // Check whether any hidden type of opaque in the autoderef chain have been | ||
| // constrained. | ||
| for step in self.steps { | ||
| if step.self_ty_is_opaque { | ||
| if step.self_ty_is_hidden_ty_of_opaque { | ||
| debug!(?step.autoderefs, ?step.self_ty, "self_type_is_opaque"); | ||
| let constrained_opaque = self.probe(|_| { | ||
| // If we fail to instantiate the self type of this | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ use region_constraints::{ | |
| GenericKind, RegionConstraintCollector, RegionConstraintStorage, VarInfos, VerifyBound, | ||
| }; | ||
| pub use relate::combine::PredicateEmittingRelation; | ||
| use rustc_data_structures::fx::{FxHashSet, FxIndexMap}; | ||
| use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet}; | ||
| use rustc_data_structures::snapshot_vec as sv; | ||
| use rustc_data_structures::undo_log::{Rollback, UndoLogs}; | ||
| use rustc_data_structures::unify::{self as ut, UnifyKey, UnifyValue}; | ||
|
|
@@ -1121,25 +1121,32 @@ impl<'tcx> InferCtxt<'tcx> { | |
| } | ||
|
|
||
| #[instrument(level = "debug", skip(self), ret)] | ||
| pub fn take_opaque_types(&self) -> Vec<(OpaqueTypeKey<'tcx>, ProvisionalHiddenType<'tcx>)> { | ||
| self.inner.borrow_mut().opaque_type_storage.take_opaque_types().collect() | ||
| pub fn take_opaque_types( | ||
| &self, | ||
| ) -> ( | ||
| Vec<(OpaqueTypeKey<'tcx>, ProvisionalHiddenType<'tcx>)>, | ||
| Vec<(Ty<'tcx>, FxIndexSet<ty::OpaqueHiddenTyBound<'tcx>>)>, | ||
| ) { | ||
| let mut inner = self.inner.borrow_mut(); | ||
| let (opaques, hiddens) = inner.opaque_type_storage.take_opaque_types(); | ||
| (opaques.collect(), hiddens.collect()) | ||
| } | ||
|
|
||
| #[instrument(level = "debug", skip(self), ret)] | ||
| pub fn clone_opaque_types(&self) -> Vec<(OpaqueTypeKey<'tcx>, ProvisionalHiddenType<'tcx>)> { | ||
| self.inner.borrow_mut().opaque_type_storage.iter_opaque_types().collect() | ||
| } | ||
|
|
||
| pub fn has_opaques_with_sub_unified_hidden_type(&self, ty_vid: TyVid) -> bool { | ||
| pub fn has_hidden_types_of_opaques_modulo_sub_unification(&self, ty_vid: TyVid) -> bool { | ||
| if !self.next_trait_solver() { | ||
| return false; | ||
| } | ||
|
|
||
| let ty_sub_vid = self.sub_unification_table_root_var(ty_vid); | ||
| let inner = &mut *self.inner.borrow_mut(); | ||
| let mut type_variables = inner.type_variable_storage.with_log(&mut inner.undo_log); | ||
| inner.opaque_type_storage.iter_opaque_types().any(|(_, hidden_ty)| { | ||
| if let ty::Infer(ty::TyVar(hidden_vid)) = *hidden_ty.ty.kind() { | ||
| inner.opaque_type_storage.iter_hidden_types_of_opaques().any(|(hidden_ty, _)| { | ||
| if let ty::Infer(ty::TyVar(hidden_vid)) = *hidden_ty.kind() { | ||
| let opaque_sub_vid = type_variables.sub_unification_table_root_var(hidden_vid); | ||
| if opaque_sub_vid == ty_sub_vid { | ||
| return true; | ||
|
|
@@ -1187,6 +1194,36 @@ impl<'tcx> InferCtxt<'tcx> { | |
| .collect() | ||
| } | ||
|
|
||
| pub fn hidden_types_of_opaques_modulo_sub_unification( | ||
| &self, | ||
| ty_vid: TyVid, | ||
| ) -> Vec<(Ty<'tcx>, Vec<ty::OpaqueHiddenTyBound<'tcx>>)> { | ||
|
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. This can return an iterator and then |
||
| // Avoid accidentally allowing more code to compile with the old solver. | ||
| if !self.next_trait_solver() { | ||
| return vec![]; | ||
| } | ||
|
|
||
| let ty_sub_vid = self.sub_unification_table_root_var(ty_vid); | ||
| let inner = &mut *self.inner.borrow_mut(); | ||
| // This is iffy, can't call `type_variables()` as we're already | ||
| // borrowing the `opaque_type_storage` here. | ||
| let mut type_variables = inner.type_variable_storage.with_log(&mut inner.undo_log); | ||
| inner | ||
| .opaque_type_storage | ||
| .iter_hidden_types_of_opaques() | ||
| .filter_map(|(hidden_ty, bounds)| { | ||
| if let ty::Infer(ty::TyVar(hidden_vid)) = *hidden_ty.kind() { | ||
| let opaque_sub_vid = type_variables.sub_unification_table_root_var(hidden_vid); | ||
| if opaque_sub_vid == ty_sub_vid { | ||
| return Some((hidden_ty, bounds.iter().copied().collect())); | ||
| } | ||
| } | ||
|
|
||
| None | ||
| }) | ||
| .collect() | ||
| } | ||
|
|
||
| #[inline(always)] | ||
| pub fn can_define_opaque_ty(&self, id: impl Into<DefId>) -> bool { | ||
| debug_assert!(!self.next_trait_solver()); | ||
|
|
||
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.
debug_assertis enough? probably doesn't matter sinceis_emptyis cheap