-
-
Notifications
You must be signed in to change notification settings - Fork 17.4k
wrap ty field of CoroutineSavedTy in EarlyBinder
#163152
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 |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ use rustc_macros::{StableHash, TyDecodable, TyEncodable, TypeFoldable, TypeVisit | |
| use rustc_span::{Span, Symbol}; | ||
|
|
||
| use super::{ConstValue, SourceInfo}; | ||
| use crate::ty::{self, CoroutineArgsExt, Ty}; | ||
| use crate::ty::{self, CoroutineArgsExt, EarlyBinder, GenericArgsRef, Ty, TyCtxt, Unnormalized}; | ||
|
|
||
| rustc_index::newtype_index! { | ||
| #[stable_hash] | ||
|
|
@@ -22,7 +22,9 @@ rustc_index::newtype_index! { | |
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| #[derive(TyEncodable, TyDecodable, StableHash, TypeFoldable, TypeVisitable)] | ||
| pub struct CoroutineSavedTy<'tcx> { | ||
| pub ty: Ty<'tcx>, | ||
| #[type_foldable(identity)] | ||
| #[type_visitable(ignore)] | ||
| pub ty: EarlyBinder<'tcx, Ty<'tcx>>, | ||
| /// Source info corresponding to the local in the original MIR body. | ||
| pub source_info: SourceInfo, | ||
| /// Whether the local should be ignored for trait bound computations. | ||
|
|
@@ -81,6 +83,60 @@ impl Debug for CoroutineLayout<'_> { | |
| } | ||
| } | ||
|
|
||
| /// The result of the `coroutine_layout` function. | ||
| /// | ||
| /// Wraps a regular `CoroutineLayout` with its arguments, providing accessors | ||
| /// that instantiate the stored `Ty` if necessary. | ||
| #[derive(Debug, Copy, Clone)] | ||
| pub struct QueriedCoroutineLayout<'tcx> { | ||
| layout: &'tcx CoroutineLayout<'tcx>, | ||
| args: Option<GenericArgsRef<'tcx>>, | ||
|
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. I made this an |
||
| } | ||
|
|
||
| impl<'tcx> QueriedCoroutineLayout<'tcx> { | ||
| pub fn new(layout: &'tcx CoroutineLayout<'tcx>, args: Option<GenericArgsRef<'tcx>>) -> Self { | ||
| Self { layout, args } | ||
| } | ||
|
|
||
| pub fn get_ty( | ||
| &self, | ||
| tcx: TyCtxt<'tcx>, | ||
| field: CoroutineSavedLocal, | ||
| ) -> Unnormalized<'tcx, Ty<'tcx>> { | ||
| if let Some(args) = self.args { | ||
| self.layout.field_tys[field].ty.instantiate(tcx, args) | ||
| } else { | ||
| self.layout.field_tys[field].ty.instantiate_identity() | ||
| } | ||
| } | ||
|
|
||
| pub fn get_identity_ty(&self, field: CoroutineSavedLocal) -> Unnormalized<'tcx, Ty<'tcx>> { | ||
| self.layout.field_tys[field].ty.instantiate_identity() | ||
| } | ||
|
|
||
| pub fn field_tys(&self) -> &'tcx IndexVec<CoroutineSavedLocal, CoroutineSavedTy<'tcx>> { | ||
| &self.layout.field_tys | ||
| } | ||
|
|
||
| pub fn variant_fields( | ||
| &self, | ||
| ) -> &'tcx IndexVec<VariantIdx, IndexVec<FieldIdx, CoroutineSavedLocal>> { | ||
| &self.layout.variant_fields | ||
| } | ||
|
|
||
| pub fn variant_source_info(&self) -> &'tcx IndexVec<VariantIdx, SourceInfo> { | ||
| &self.layout.variant_source_info | ||
| } | ||
|
|
||
| pub fn storage_conflicts(&self) -> &'tcx BitMatrix<CoroutineSavedLocal, CoroutineSavedLocal> { | ||
| &self.layout.storage_conflicts | ||
| } | ||
|
|
||
| pub fn raw_layout(self) -> &'tcx CoroutineLayout<'tcx> { | ||
| self.layout | ||
| } | ||
| } | ||
|
|
||
| /// The result of the `mir_const_qualif` query. | ||
| /// | ||
| /// Each field (except `tainted_by_errors`) corresponds to an implementer of the `Qualif` trait in | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,7 +101,8 @@ impl<'tcx> ty::CoroutineArgs<TyCtxt<'tcx>> { | |
| #[inline] | ||
| fn variant_range(&self, def_id: DefId, tcx: TyCtxt<'tcx>) -> Range<VariantIdx> { | ||
| // FIXME requires optimized MIR | ||
| FIRST_VARIANT..tcx.coroutine_layout(def_id, self.args).unwrap().variant_fields.next_index() | ||
| FIRST_VARIANT | ||
| ..tcx.coroutine_layout(def_id, self.args).unwrap().variant_fields().next_index() | ||
| } | ||
|
|
||
| /// The discriminant for the given variant. Panics if the `variant_index` is | ||
|
|
@@ -162,14 +163,13 @@ impl<'tcx> ty::CoroutineArgs<TyCtxt<'tcx>> { | |
| tcx: TyCtxt<'tcx>, | ||
| ) -> impl Iterator<Item: Iterator<Item = Ty<'tcx>>> { | ||
| let layout = tcx.coroutine_layout(def_id, self.args).unwrap(); | ||
| layout.variant_fields.iter().map(move |variant| { | ||
| layout.variant_fields().iter().map(move |variant| { | ||
| variant.iter().map(move |field| { | ||
| if tcx.is_async_drop_in_place_coroutine(def_id) { | ||
| layout.field_tys[*field].ty | ||
| // FIXME(async_drop): this needs a comment for why its correct | ||
| layout.get_identity_ty(*field).skip_norm_wip() | ||
|
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. This function is only used once here, and I am not sure if it's really needed or not, as ideally |
||
| } else { | ||
| ty::EarlyBinder::bind(tcx, layout.field_tys[*field].ty) | ||
| .instantiate(tcx, self.args) | ||
| .skip_norm_wip() | ||
| layout.get_ty(tcx, *field).skip_norm_wip() | ||
| } | ||
| }) | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -766,9 +766,12 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| // since we may be in the process of computing this MIR in the | ||
| // first place. | ||
| let layout = if def_id == self.caller_body.source.def_id() { | ||
| self.caller_body | ||
| .coroutine_layout_raw() | ||
| .or_else(|| self.tcx.coroutine_layout(def_id, args).ok()) | ||
| self.caller_body.coroutine_layout_raw().or_else(|| { | ||
| self.tcx | ||
| .coroutine_layout(def_id, args) | ||
| .ok() | ||
| .map(|queried| queried.raw_layout()) | ||
|
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.
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. To clarify the messiness: in this branch we have a pub struct QueriedCoroutineLayout<'a, 'tcx> {
layout: &'a CoroutineLayout<'tcx>,
args: Option<GenericArgsRef<'tcx>>,
}which seemed undesirable to me. |
||
| }) | ||
| } else if self.tcx.needs_coroutine_by_move_body_def_id(def_id) | ||
| && let ty::ClosureKind::FnOnce = | ||
| args.as_coroutine().kind_ty().to_opt_closure_kind().unwrap() | ||
|
|
@@ -778,7 +781,10 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| // Same if this is the by-move body of a coroutine-closure. | ||
| self.caller_body.coroutine_layout_raw() | ||
| } else { | ||
| self.tcx.coroutine_layout(def_id, args).ok() | ||
| self.tcx | ||
| .coroutine_layout(def_id, args) | ||
| .ok() | ||
| .map(|queried| queried.raw_layout()) | ||
| }; | ||
|
|
||
| let Some(layout) = layout else { | ||
|
|
@@ -802,9 +808,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | |
| return; | ||
| }; | ||
|
|
||
| ty::EarlyBinder::bind(self.tcx, f_ty.ty) | ||
| .instantiate(self.tcx, args) | ||
| .skip_norm_wip() | ||
| f_ty.ty.instantiate(self.tcx, args).skip_norm_wip() | ||
| } else if let Some(&f_ty) = args.as_coroutine().upvar_tys().get(f.index()) { | ||
| f_ty | ||
| } else { | ||
|
|
||
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.
oh, why is this
TypeFoldable?ignoring this field is wrong. We should not type fold
CoroutineSavedTy. What breaks if you remove thederivethere?View changes since the review
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.
if I just remove the
deriveI get an error about howCoroutineLayoutcan't implementTypeFoldablebecause its fields don't implement it.full error
If I add a
#[type_foldable(identity)]for thefield_tysfield which stores theCoroutineSavedTys thenx checkandx test tests/uiboth succeedThere 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 does
CoroutineLAyoutimplementTypeFoldable:>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.
the chain goes like this:
CoroutineSavedTy->CoroutineLayout->CoroutineInfo->Bodyand if I remove it from
BodyI get an error atrustc_public_bridge/src/builder.rsabout howBodyneeds to beTypeFoldablebecauseEarlyBinder::bindrequires that as a trait boundThere 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.
hmm, that's kind of scuffed.
Bodyshouldn't beTypeFoldableideally as some of the fields of MIR bodies don't make sense after it. Fixing that is annoying and non-trivial and ignoring theEArlyBinderhere is fine. Coolio