-
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 1 commit
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 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -687,6 +687,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | |||||||||||
ty::Adt(adt, _) => { | ||||||||||||
adt.discriminants(*self.tcx).find(|(_, var)| var.val == discr_bits) | ||||||||||||
} | ||||||||||||
ty::Variant(ty, _) => match ty.kind() { | ||||||||||||
ty::Adt(adt, _) => adt.discriminants(*self.tcx).find(|(_, var)| var.val == discr_bits), | ||||||||||||
_ => bug!("unexpected type: {:?}", ty.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.
Suggested change
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 should return (VariantIdx, Discr) not just the index so this wouldn't work? 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. ah, yea, that makes sense. we should probably still assert somewhere that the index matches the expected one here |
||||||||||||
ty::Generator(def_id, substs, _) => { | ||||||||||||
let substs = substs.as_generator(); | ||||||||||||
substs | ||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -219,6 +219,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' | |
if tag_field == field { | ||
return match layout.ty.kind() { | ||
ty::Adt(def, ..) if def.is_enum() => PathElem::EnumTag, | ||
ty::Variant(ty, ..) => match ty.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. Instead of any of the changes in this function, we could do a if let ty::Variant(ty, ..) = *layout.ty.kind() {
return self.aggregate_field_path_elem(self.ecx.layout_of(ty), field),
} at the start 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 in a |
||
ty::Adt(def, ..) if def.is_enum() => PathElem::EnumTag, | ||
_ => bug!("non-variant type {:?}", layout.ty), | ||
}, | ||
ty::Generator(..) => PathElem::GeneratorTag, | ||
_ => bug!("non-variant type {:?}", layout.ty), | ||
}; | ||
|
@@ -272,6 +276,20 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' | |
} | ||
} | ||
|
||
ty::Variant(ty, ..) => match ty.kind() { | ||
ty::Adt(def, ..) if def.is_enum() => { | ||
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 seems to be a very common pattern to get the adtdef of the enum for a |
||
// we might be projecting *to* a variant, or to a field *in* a variant. | ||
match layout.variants { | ||
Variants::Single { index } => { | ||
// Inside a variant | ||
PathElem::Field(def.variants[index].fields[field].ident.name) | ||
} | ||
Variants::Multiple { .. } => bug!("we handled variants above"), | ||
} | ||
} | ||
_ => bug!("unexpected type: {:?}", ty.kind()), | ||
}, | ||
|
||
// other ADTs | ||
ty::Adt(def, _) => PathElem::Field(def.non_enum_variant().fields[field].ident.name), | ||
|
||
|
@@ -567,6 +585,17 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' | |
self.check_safe_pointer(value, "box")?; | ||
Ok(true) | ||
} | ||
ty::Variant(ty, _) => match ty.kind() { | ||
ty::Adt(def, _) => { | ||
if def.is_box() { | ||
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. Box is not an enum, this should be impossible. I think this can be handled together with the |
||
self.check_safe_pointer(value, "box")?; | ||
Ok(true) | ||
} else { | ||
Ok(false) | ||
} | ||
} | ||
_ => bug!("unexpected type: {:?}", ty.kind()), | ||
}, | ||
ty::FnPtr(_sig) => { | ||
let value = try_validation!( | ||
self.ecx.read_immediate(value), | ||
|
@@ -729,6 +758,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> | |
) -> InterpResult<'tcx> { | ||
let name = match old_op.layout.ty.kind() { | ||
ty::Adt(adt, _) => PathElem::Variant(adt.variants[variant_id].ident.name), | ||
ty::Variant(ty, ..) => match ty.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. Is this reachable? We should only reach this function for types with a multi-variant layout, which |
||
ty::Adt(adt, ..) => PathElem::Variant(adt.variants[variant_id].ident.name), | ||
_ => bug!("unexpected type {:?}", ty.kind()), | ||
}, | ||
// Generators also have variants | ||
ty::Generator(..) => PathElem::GeneratorState(variant_id), | ||
_ => bug!("Unexpected type with variant: {:?}", old_op.layout.ty), | ||
zhamlin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
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, | ||
|
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