1
Fork 0

address review

This commit is contained in:
b-naber 2022-12-09 18:08:56 +01:00
parent dc93a28e98
commit ff41359e65
4 changed files with 61 additions and 92 deletions

View file

@ -875,6 +875,13 @@ pub struct Place<'tcx> {
pub projection: &'tcx List<PlaceElem<'tcx>>, pub projection: &'tcx List<PlaceElem<'tcx>>,
} }
/// The different kinds of projections that can be used in the projection of a `Place`.
///
/// `T1` is the generic type for a field projection. For an actual projection on a `Place`
/// this parameter will always be `Ty`, but the field type can be unavailable when
/// building (by using `PlaceBuilder`) places that correspond to upvars.
/// `T2` is the generic type for an `OpaqueCast` (is generic since it's abstracted over
/// in dataflow analysis, see `AbstractElem`).
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(TyEncodable, TyDecodable, HashStable, TypeFoldable, TypeVisitable)] #[derive(TyEncodable, TyDecodable, HashStable, TypeFoldable, TypeVisitable)]
pub enum ProjectionElem<V, T1, T2> { pub enum ProjectionElem<V, T1, T2> {
@ -942,7 +949,7 @@ pub enum ProjectionElem<V, T1, T2> {
/// and the index is a local. /// and the index is a local.
pub type PlaceElem<'tcx> = ProjectionElem<Local, Ty<'tcx>, Ty<'tcx>>; pub type PlaceElem<'tcx> = ProjectionElem<Local, Ty<'tcx>, Ty<'tcx>>;
/// Alias for projections that appear in `PlaceBuilder::UpVar`, for which /// Alias for projections that appear in `PlaceBuilder::Upvar`, for which
/// we cannot provide any field types. /// we cannot provide any field types.
pub type UpvarProjectionElem<'tcx> = ProjectionElem<Local, (), Ty<'tcx>>; pub type UpvarProjectionElem<'tcx> = ProjectionElem<Local, (), Ty<'tcx>>;

View file

@ -33,7 +33,7 @@ pub(in crate::build) enum PlaceBuilder<'tcx> {
/// Denotes the start of a `Place`. /// Denotes the start of a `Place`.
/// ///
/// We use `PlaceElem` since this has all `Field` types available. /// We use `PlaceElem` since this has all `Field` types available.
Local(Local, Vec<PlaceElem<'tcx>>), Local { local: Local, projection: Vec<PlaceElem<'tcx>> },
/// When building place for an expression within a closure, the place might start off a /// When building place for an expression within a closure, the place might start off a
/// captured path. When `capture_disjoint_fields` is enabled, we might not know the capture /// captured path. When `capture_disjoint_fields` is enabled, we might not know the capture
@ -67,11 +67,11 @@ pub(in crate::build) enum PlaceBuilder<'tcx> {
/// ///
/// Note: in contrast to `PlaceBuilder::Local` we have not yet determined all `Field` types /// Note: in contrast to `PlaceBuilder::Local` we have not yet determined all `Field` types
/// and will only do so once converting to `PlaceBuilder::Local`. /// and will only do so once converting to `PlaceBuilder::Local`.
UpVar(UpVar, Vec<UpvarProjectionElem<'tcx>>), Upvar { upvar: Upvar, projection: Vec<UpvarProjectionElem<'tcx>> },
} }
#[derive(Copy, Clone, Debug, PartialEq)] #[derive(Copy, Clone, Debug, PartialEq)]
pub(crate) struct UpVar { pub(crate) struct Upvar {
var_hir_id: LocalVarId, var_hir_id: LocalVarId,
closure_def_id: LocalDefId, closure_def_id: LocalDefId,
} }
@ -222,36 +222,7 @@ fn to_upvars_resolved_place_builder<'tcx>(
upvar_projection, upvar_projection,
); );
debug_assert!({ assert!(matches!(upvar_resolved_place_builder, PlaceBuilder::Local { .. }));
let builder = upvar_resolved_place_builder.clone();
let mut valid_conversion = true;
match builder {
PlaceBuilder::Local(_, projections) => {
for proj in projections.iter() {
match proj {
ProjectionElem::Field(_, field_ty) => {
if matches!(field_ty.kind(), ty::Infer(..)) {
debug!(
"field ty should have been converted for projection {:?} in PlaceBuilder {:?}",
proj,
upvar_resolved_place_builder.clone()
);
valid_conversion = false;
break;
}
}
_ => {}
}
}
}
PlaceBuilder::UpVar(..) => {
unreachable!()
}
}
valid_conversion
});
Some(upvar_resolved_place_builder) Some(upvar_resolved_place_builder)
} }
@ -269,9 +240,9 @@ fn strip_prefix<'a, 'tcx>(
) -> impl Iterator<Item = UpvarProjectionElem<'tcx>> + 'a { ) -> impl Iterator<Item = UpvarProjectionElem<'tcx>> + 'a {
let mut iter = projections let mut iter = projections
.iter() .iter()
.copied()
// Filter out opaque casts, they are unnecessary in the prefix. // Filter out opaque casts, they are unnecessary in the prefix.
.filter(|elem| !matches!(elem, ProjectionElem::OpaqueCast(..))) .filter(|elem| !matches!(elem, ProjectionElem::OpaqueCast(..)));
.map(|elem| *elem);
for projection in prefix_projections { for projection in prefix_projections {
debug!(?projection, ?projection.ty); debug!(?projection, ?projection.ty);
@ -305,8 +276,8 @@ impl<'tcx> PlaceBuilder<'tcx> {
pub(in crate::build) fn try_to_place(&self, cx: &Builder<'_, 'tcx>) -> Option<Place<'tcx>> { pub(in crate::build) fn try_to_place(&self, cx: &Builder<'_, 'tcx>) -> Option<Place<'tcx>> {
let resolved = self.resolve_upvar(cx); let resolved = self.resolve_upvar(cx);
let builder = resolved.as_ref().unwrap_or(self); let builder = resolved.as_ref().unwrap_or(self);
let PlaceBuilder::Local(local, projection) = builder else { return None }; let PlaceBuilder::Local{local, ref projection} = builder else { return None };
let projection = cx.tcx.intern_place_elems(&projection); let projection = cx.tcx.intern_place_elems(projection);
Some(Place { local: *local, projection }) Some(Place { local: *local, projection })
} }
@ -324,40 +295,31 @@ impl<'tcx> PlaceBuilder<'tcx> {
&self, &self,
cx: &Builder<'_, 'tcx>, cx: &Builder<'_, 'tcx>,
) -> Option<PlaceBuilder<'tcx>> { ) -> Option<PlaceBuilder<'tcx>> {
let PlaceBuilder::UpVar( UpVar {var_hir_id, closure_def_id }, projection) = self else { let PlaceBuilder::Upvar{ upvar: Upvar {var_hir_id, closure_def_id }, projection} = self else {
return None; return None;
}; };
to_upvars_resolved_place_builder(cx, *var_hir_id, *closure_def_id, &projection) to_upvars_resolved_place_builder(cx, *var_hir_id, *closure_def_id, &projection)
} }
pub(crate) fn get_local_projection(&self) -> &[PlaceElem<'tcx>] {
match self {
Self::Local(_, projection) => projection,
Self::UpVar(..) => {
bug!("get_local_projection_mut can only be called on PlaceBuilder::Local")
}
}
}
#[instrument(skip(cx), level = "debug")] #[instrument(skip(cx), level = "debug")]
pub(crate) fn field(self, cx: &Builder<'_, 'tcx>, f: Field) -> Self { pub(crate) fn field(self, cx: &Builder<'_, 'tcx>, f: Field) -> Self {
let field_ty = match self.clone() { match self.clone() {
PlaceBuilder::Local(local, projection) => { PlaceBuilder::Local { local, projection } => {
let base_place = PlaceBuilder::Local(local, projection); let base_place = PlaceBuilder::Local { local, projection };
let PlaceTy { ty, variant_index } = let PlaceTy { ty, variant_index } =
base_place.to_place(cx).ty(&cx.local_decls, cx.tcx); base_place.to_place(cx).ty(&cx.local_decls, cx.tcx);
let base_ty = cx.tcx.normalize_erasing_regions(cx.param_env, ty); let base_ty = cx.tcx.normalize_erasing_regions(cx.param_env, ty);
PlaceBuilder::compute_field_ty(cx, f, base_ty, variant_index) let field_ty = PlaceBuilder::compute_field_ty(cx, f, base_ty, variant_index);
}
PlaceBuilder::UpVar(..) => {
let dummy_ty = cx.tcx.mk_ty_infer(ty::FreshTy(0));
dummy_ty
}
};
self.project(ProjectionElem::Field(f, field_ty)) self.project(ProjectionElem::Field(f, field_ty))
}
PlaceBuilder::Upvar { upvar, mut projection } => {
projection.push(ProjectionElem::Field(f, ()));
PlaceBuilder::Upvar { upvar, projection }
}
}
} }
pub(crate) fn deref(self) -> Self { pub(crate) fn deref(self) -> Self {
@ -375,13 +337,13 @@ impl<'tcx> PlaceBuilder<'tcx> {
#[instrument(level = "debug")] #[instrument(level = "debug")]
pub(crate) fn project(self, elem: PlaceElem<'tcx>) -> Self { pub(crate) fn project(self, elem: PlaceElem<'tcx>) -> Self {
let result = match self { let result = match self {
PlaceBuilder::Local(local, mut proj) => { PlaceBuilder::Local { local, mut projection } => {
proj.push(elem); projection.push(elem);
PlaceBuilder::Local(local, proj) PlaceBuilder::Local { local, projection }
} }
PlaceBuilder::UpVar(upvar, mut proj) => { PlaceBuilder::Upvar { upvar, mut projection } => {
proj.push(elem.into()); projection.push(elem.into());
PlaceBuilder::UpVar(upvar, proj) PlaceBuilder::Upvar { upvar, projection }
} }
}; };
@ -392,14 +354,14 @@ impl<'tcx> PlaceBuilder<'tcx> {
/// Same as `.clone().project(..)` but more efficient /// Same as `.clone().project(..)` but more efficient
pub(crate) fn clone_project(&self, elem: PlaceElem<'tcx>) -> Self { pub(crate) fn clone_project(&self, elem: PlaceElem<'tcx>) -> Self {
match self { match self {
PlaceBuilder::Local(local, proj) => PlaceBuilder::Local( PlaceBuilder::Local { local, projection } => PlaceBuilder::Local {
*local, local: *local,
Vec::from_iter(proj.iter().copied().chain([elem.into()])), projection: Vec::from_iter(projection.iter().copied().chain([elem.into()])),
), },
PlaceBuilder::UpVar(upvar, proj) => PlaceBuilder::UpVar( PlaceBuilder::Upvar { upvar, projection } => PlaceBuilder::Upvar {
*upvar, upvar: *upvar,
Vec::from_iter(proj.iter().copied().chain([elem.into()])), projection: Vec::from_iter(projection.iter().copied().chain([elem.into()])),
), },
} }
} }
@ -463,7 +425,11 @@ impl<'tcx> PlaceBuilder<'tcx> {
f_ty f_ty
} else { } else {
let Some(f_ty) = substs.as_generator().prefix_tys().nth(field.index()) else { let Some(f_ty) = substs.as_generator().prefix_tys().nth(field.index()) else {
bug!("expected to take index {:?} in {:?}", field.index(), substs.as_generator().prefix_tys().collect::<Vec<_>>()); bug!(
"expected to take index {:?} in {:?}",
field.index(),
substs.as_generator().prefix_tys().collect::<Vec<_>>()
);
}; };
f_ty f_ty
@ -475,7 +441,7 @@ impl<'tcx> PlaceBuilder<'tcx> {
cx.tcx.normalize_erasing_regions(cx.param_env, field_ty) cx.tcx.normalize_erasing_regions(cx.param_env, field_ty)
} }
/// Creates a `PlaceBuilder::Local` from a `PlaceBuilder::UpVar` whose upvars /// Creates a `PlaceBuilder::Local` from a `PlaceBuilder::Upvar` whose upvars
/// are resolved. This function takes two kinds of projections: `local_projection` /// are resolved. This function takes two kinds of projections: `local_projection`
/// contains the projections of the captured upvar and `upvar_projection` the /// contains the projections of the captured upvar and `upvar_projection` the
/// projections that are applied to the captured upvar. The main purpose of this /// projections that are applied to the captured upvar. The main purpose of this
@ -516,19 +482,19 @@ impl<'tcx> PlaceBuilder<'tcx> {
} }
} }
PlaceBuilder::Local(local, local_projection) PlaceBuilder::Local { local, projection: local_projection }
} }
} }
impl<'tcx> From<Local> for PlaceBuilder<'tcx> { impl<'tcx> From<Local> for PlaceBuilder<'tcx> {
fn from(local: Local) -> Self { fn from(local: Local) -> Self {
Self::Local(local, Vec::new()) Self::Local { local, projection: Vec::new() }
} }
} }
impl<'tcx> From<Place<'tcx>> for PlaceBuilder<'tcx> { impl<'tcx> From<Place<'tcx>> for PlaceBuilder<'tcx> {
fn from(p: Place<'tcx>) -> Self { fn from(p: Place<'tcx>) -> Self {
Self::Local(p.local, p.projection.to_vec()) Self::Local { local: p.local, projection: p.projection.to_vec() }
} }
} }
@ -564,13 +530,7 @@ fn project_ty<'tcx>(
(ty, None) (ty, None)
} }
ProjectionElem::Downcast(_, variant_idx) => (ty, Some(variant_idx)), ProjectionElem::Downcast(_, variant_idx) => (ty, Some(variant_idx)),
ProjectionElem::Field(_, ty) => { ProjectionElem::Field(_, ty) => (ty, None),
if matches!(ty.kind(), ty::Infer(..)) {
bug!("Field ty should have been resolved");
}
(ty, None)
}
ProjectionElem::OpaqueCast(..) => bug!("didn't expect OpaqueCast"), ProjectionElem::OpaqueCast(..) => bug!("didn't expect OpaqueCast"),
} }
} }
@ -836,7 +796,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} }
/// Lower a captured upvar. Note we might not know the actual capture index, /// Lower a captured upvar. Note we might not know the actual capture index,
/// so we create a place starting from `UpVar`, which will be resolved /// so we create a place starting from `Upvar`, which will be resolved
/// once all projections that allow us to identify a capture have been applied. /// once all projections that allow us to identify a capture have been applied.
fn lower_captured_upvar( fn lower_captured_upvar(
&mut self, &mut self,
@ -844,7 +804,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
closure_def_id: LocalDefId, closure_def_id: LocalDefId,
var_hir_id: LocalVarId, var_hir_id: LocalVarId,
) -> BlockAnd<PlaceBuilder<'tcx>> { ) -> BlockAnd<PlaceBuilder<'tcx>> {
block.and(PlaceBuilder::UpVar(UpVar { var_hir_id, closure_def_id }, vec![])) block.and(PlaceBuilder::Upvar {
upvar: Upvar { var_hir_id, closure_def_id },
projection: vec![],
})
} }
/// Lower an index expression /// Lower an index expression

View file

@ -654,11 +654,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// We are capturing a path that starts off a local variable in the parent. // We are capturing a path that starts off a local variable in the parent.
// The mutability of the current capture is same as the mutability // The mutability of the current capture is same as the mutability
// of the local declaration in the parent. // of the local declaration in the parent.
PlaceBuilder::Local(local, _) => this.local_decls[local].mutability, PlaceBuilder::Local { local, .. } => this.local_decls[local].mutability,
// Parent is a closure and we are capturing a path that is captured // Parent is a closure and we are capturing a path that is captured
// by the parent itself. The mutability of the current capture // by the parent itself. The mutability of the current capture
// is same as that of the capture in the parent closure. // is same as that of the capture in the parent closure.
PlaceBuilder::UpVar(..) => { PlaceBuilder::Upvar { .. } => {
let enclosing_upvars_resolved = arg_place_builder.to_place(this); let enclosing_upvars_resolved = arg_place_builder.to_place(this);
match enclosing_upvars_resolved.as_ref() { match enclosing_upvars_resolved.as_ref() {

View file

@ -108,9 +108,8 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
// Only add the OpaqueCast projection if the given place is an opaque type and the // Only add the OpaqueCast projection if the given place is an opaque type and the
// expected type from the pattern is not. // expected type from the pattern is not.
let may_need_cast = match place { let may_need_cast = match place {
PlaceBuilder::Local(local, _) => { PlaceBuilder::Local { local, ref projection } => {
let ty = let ty = Place::ty_from(local, projection, &cx.local_decls, cx.tcx).ty;
Place::ty_from(local, place.get_local_projection(), &cx.local_decls, cx.tcx).ty;
ty != pattern.ty && ty.has_opaque_types() ty != pattern.ty && ty.has_opaque_types()
} }
_ => true, _ => true,