-
Notifications
You must be signed in to change notification settings - Fork 13k
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
enum variant support #89745
enum variant support #89745
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 |
---|---|---|
|
@@ -744,6 +744,22 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> { | |
PlaceTy { ty: base_ty, variant_index: Some(index) } | ||
} | ||
} | ||
ty::Variant(ty, _) => match ty.kind() { | ||
ty::Adt(adt_def, _substs) if adt_def.is_enum() => { | ||
if index.as_usize() >= adt_def.variants.len() { | ||
PlaceTy::from_ty(span_mirbug_and_err!( | ||
self, | ||
place, | ||
"cast to variant #{:?} but enum only has {:?}", | ||
index, | ||
adt_def.variants.len() | ||
)) | ||
} else { | ||
PlaceTy { ty: *ty, variant_index: Some(index) } | ||
} | ||
} | ||
_ => bug!("unexpected type: {:?}", ty.kind()), | ||
}, | ||
Comment on lines
+747
to
+762
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. is downcasting a variant type an operation we would ever encounter? I think for now this arm can just mir-bug if that would allow re-downcasting to the same variant, but nothing else |
||
// We do not need to handle generators here, because this runs | ||
// before the generator transform stage. | ||
_ => { | ||
|
@@ -812,6 +828,12 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> { | |
let (variant, substs) = match base_ty { | ||
PlaceTy { ty, variant_index: Some(variant_index) } => match *ty.kind() { | ||
ty::Adt(adt_def, substs) => (&adt_def.variants[variant_index], substs), | ||
ty::Variant(ty, _) => match ty.kind() { | ||
ty::Adt(adt_def, substs) => { | ||
(adt_def.variants.get(variant_index).expect(""), *substs) | ||
} | ||
_ => bug!("unexpected type: {:?}", ty.kind()), | ||
}, | ||
Comment on lines
829
to
+836
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. if we get rid of 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. If we remove 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. If the above comment is implemented, then we should never encounter that arm, it should always be ty::Variant if we would have had a variant index in the PlaceTy |
||
ty::Generator(def_id, substs, _) => { | ||
let mut variants = substs.as_generator().state_tys(def_id, tcx); | ||
let mut variant = match variants.nth(variant_index.into()) { | ||
|
@@ -833,6 +855,12 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> { | |
ty::Adt(adt_def, substs) if !adt_def.is_enum() => { | ||
(&adt_def.variants[VariantIdx::new(0)], substs) | ||
} | ||
ty::Variant(ty, _) => match ty.kind() { | ||
ty::Adt(adt_def, substs) if adt_def.is_enum() => { | ||
(&adt_def.variants[VariantIdx::new(0)], *substs) | ||
} | ||
_ => bug!("unexpected type: {:?}", ty.kind()), | ||
}, | ||
ty::Closure(_, substs) => { | ||
return match substs | ||
.as_closure() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -217,7 +217,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' | |
match layout.variants { | ||
Variants::Multiple { tag_field, .. } => { | ||
if tag_field == field { | ||
return match layout.ty.kind() { | ||
return match layout.ty.strip_variant_type().kind() { | ||
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. Variant types should never have Generally I am not very happy with 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. strip_variant_type is the correct thing wherever we don't actually handle variant things on generators or adts right now, so here it is indeed wrong, we should not ever encounter adt/generator anymore, but for that 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.
It's the other |
||
ty::Adt(def, ..) if def.is_enum() => PathElem::EnumTag, | ||
ty::Generator(..) => PathElem::GeneratorTag, | ||
_ => bug!("non-variant type {:?}", layout.ty), | ||
|
@@ -228,7 +228,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' | |
} | ||
|
||
// Now we know we are projecting to a field, so figure out which one. | ||
match layout.ty.kind() { | ||
match layout.ty.strip_variant_type().kind() { | ||
// generators and closures. | ||
ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => { | ||
let mut name = None; | ||
|
@@ -495,7 +495,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' | |
) -> InterpResult<'tcx, bool> { | ||
// Go over all the primitive types | ||
let ty = value.layout.ty; | ||
match ty.kind() { | ||
match ty.strip_variant_type().kind() { | ||
ty::Bool => { | ||
let value = self.read_scalar(value)?; | ||
try_validation!( | ||
|
@@ -612,6 +612,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' | |
| ty::Opaque(..) | ||
| ty::Projection(..) | ||
| ty::GeneratorWitness(..) => bug!("Encountered invalid type {:?}", ty), | ||
|
||
ty::Variant(..) => unreachable!(), | ||
} | ||
} | ||
|
||
|
@@ -727,11 +729,12 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> | |
variant_id: VariantIdx, | ||
new_op: &OpTy<'tcx, M::PointerTag>, | ||
) -> InterpResult<'tcx> { | ||
let name = match old_op.layout.ty.kind() { | ||
let ty = old_op.layout.ty.strip_variant_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. Variant types don't have variants, so something seems wrong if we see a variant type here. |
||
let name = match ty.kind() { | ||
ty::Adt(adt, _) => PathElem::Variant(adt.variants[variant_id].ident.name), | ||
// Generators also have variants | ||
ty::Generator(..) => PathElem::GeneratorState(variant_id), | ||
_ => bug!("Unexpected type with variant: {:?}", old_op.layout.ty), | ||
_ => bug!("Unexpected type with variant: {:?}", ty), | ||
}; | ||
self.with_elem(name, move |this| this.visit_value(new_op)) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1919,6 +1919,7 @@ impl<'tcx> TyCtxt<'tcx> { | |
fmt, | ||
self.0, | ||
Adt, | ||
Variant, | ||
Array, | ||
Slice, | ||
RawPtr, | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -402,6 +402,30 @@ pub fn super_relate_tys<R: TypeRelation<'tcx>>( | |||
Ok(tcx.mk_adt(a_def, substs)) | ||||
} | ||||
|
||||
// TODO(zhamlin): handle this somewhere else? | ||||
// Enum <- Enum Variant | ||||
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 is the site for relating things that have no order in their equality. So you should not compare variants with anything other than variants here. 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 believe the right place to do this is to add an arm (and a function, like in the other arms) for ty::Variant here:
|
||||
(&ty::Adt(a_def, a_substs), &ty::Variant(b_ty, _)) if relation.a_is_expected() => { | ||||
match b_ty.kind() { | ||||
ty::Adt(b_def, b_substs) if a_def == *b_def => { | ||||
let substs = relation.relate_item_substs(a_def.did, a_substs, b_substs)?; | ||||
Ok(tcx.mk_adt(a_def, substs)) | ||||
} | ||||
_ => Err(TypeError::Sorts(expected_found(relation, a, b))), | ||||
} | ||||
} | ||||
|
||||
(&ty::Variant(a_ty, _), &ty::Adt(b_def, b_substs)) => match a_ty.kind() { | ||||
ty::Adt(a_def, a_substs) if *a_def == b_def => { | ||||
let substs = relation.relate_item_substs(a_def.did, a_substs, b_substs)?; | ||||
Ok(tcx.mk_adt(a_def, substs)) | ||||
} | ||||
_ => Err(TypeError::Sorts(expected_found(relation, a, b))), | ||||
}, | ||||
|
||||
(&ty::Variant(a_ty, a_idx), &ty::Variant(b_ty, b_idx)) if a_idx == b_idx => { | ||||
relation.relate(a_ty, b_ty).map(|ty| tcx.mk_ty(ty::Variant(ty, a_idx))) | ||||
} | ||||
|
||||
(&ty::Foreign(a_id), &ty::Foreign(b_id)) if a_id == b_id => Ok(tcx.mk_foreign(a_id)), | ||||
|
||||
(&ty::Dynamic(a_obj, a_region), &ty::Dynamic(b_obj, b_region)) => { | ||||
|
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.
Ideally we'd remove the
variant_index
field and replacety
here with the variant type