Skip to content
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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,22 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
PlaceTy { ty: base_ty, variant_index: Some(index) }
Copy link
Contributor

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 replace ty here with the variant type

}
}
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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 index != var_index where var_index is the second field of ty::Variant.

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.
_ => {
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we get rid of variant_index as mentioned earlier, then we can grab the variant index from the ty::Variant

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we remove variant_index how would we handle the Adt match arm above?

Copy link
Contributor

Choose a reason for hiding this comment

The 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()) {
Expand All @@ -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()
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ fn push_debuginfo_type_name<'tcx>(
t
);
}
ty::Variant(..) => unimplemented!("TODO(zhamlin)"),
}

/// MSVC names enums differently than other platforms so that the debugging visualization
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/const_eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ fn const_to_valtree_inner<'tcx>(
| ty::Closure(..)
| ty::Generator(..)
| ty::GeneratorWitness(..) => None,
ty::Variant(..) => unimplemented!("TODO(zhamlin)"),
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ crate fn eval_nullary_intrinsic<'tcx>(
| ty::Never
| ty::Tuple(_)
| ty::Error(_) => ConstValue::from_machine_usize(0u64, &tcx),
ty::Variant(..) => unimplemented!("TODO(zhamlin)"),
},
other => bug!("`{}` is not a zero arg intrinsic", other),
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> {
ty::Foreign(def_id) => self.print_def_path(def_id, &[]),

ty::GeneratorWitness(_) => bug!("type_name: unexpected `GeneratorWitness`"),
ty::Variant(..) => unimplemented!("TODO(zhamlin)"),
}
}

Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ty::Variant(ty, _) => match ty.kind() {
ty::Adt(adt, _) => adt.discriminants(*self.tcx).find(|(_, var)| var.val == discr_bits),
_ => bug!("unexpected type: {:?}", ty.kind()),
}
ty::Variant(_ty, idx) => idx,

Copy link
Author

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Down
33 changes: 33 additions & 0 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in a Variants::Multiple match arm; we should never see ty::Variant here I think.

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),
};
Expand Down Expand Up @@ -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() => {
Copy link
Member

Choose a reason for hiding this comment

The 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 ty::Variant; should maybe ty_adt_def have support for ty::Variant so it could be used instead? That would also avoid all the rightwards drift.

// 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),

Expand Down Expand Up @@ -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() {
Copy link
Member

Choose a reason for hiding this comment

The 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 Adt fallback case below.

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),
Expand Down Expand Up @@ -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() {
Copy link
Member

Choose a reason for hiding this comment

The 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::Variant is not.

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
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/canonical/canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for Canonicalizer<'cx, 'tcx> {
| ty::Uint(..)
| ty::Float(..)
| ty::Adt(..)
| ty::Variant(..)
| ty::Str
| ty::Error(_)
| ty::Array(..)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ impl<'infcx, 'tcx> InferCtxt<'infcx, 'tcx> {
{
let a_is_expected = relation.a_is_expected();

debug!("super_combine_tys: {:?} | {:?}", a.kind(), b.kind());
match (a.kind(), b.kind()) {
// Relate integral variables to other types
(&ty::Infer(ty::IntVar(a_id)), &ty::Infer(ty::IntVar(b_id))) => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/freshen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ impl<'a, 'tcx> TypeFolder<'tcx> for TypeFreshener<'a, 'tcx> {
| ty::Uint(..)
| ty::Float(..)
| ty::Adt(..)
| ty::Variant(..)
| ty::Str
| ty::Error(_)
| ty::Array(..)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1130,6 +1130,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
| ty::GeneratorWitness(..)
| ty::Placeholder(..)
| ty::FnDef(..) => bug!("unexpected type in foreign function: {:?}", ty),
ty::Variant(..) => unimplemented!("TODO(zhamlin)"),
}
}

Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_middle/src/ty/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ impl<'tcx> CastTy<'tcx> {
ty::Uint(u) => Some(CastTy::Int(IntTy::U(u))),
ty::Float(_) => Some(CastTy::Float),
ty::Adt(d, _) if d.is_enum() && d.is_payloadfree() => Some(CastTy::Int(IntTy::CEnum)),
ty::Variant(ty, _) => match ty.kind() {
ty::Adt(d, _) => {
if d.is_enum() && d.is_payloadfree() {
Some(CastTy::Int(IntTy::CEnum))
} else {
None
}
}
_ => bug!("unexpected type: {:?}", ty.kind()),
}
zhamlin marked this conversation as resolved.
Show resolved Hide resolved
ty::RawPtr(mt) => Some(CastTy::Ptr(mt)),
ty::FnPtr(..) => Some(CastTy::FnPtr),
_ => None,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ macro_rules! impl_decodable_via_ref {
})*
}
}

// TODO(zhamlin): enum variant here?
impl<'tcx, D: TyDecoder<'tcx>> RefDecodable<'tcx, D> for ty::AdtDef {
fn decode(decoder: &mut D) -> Result<&'tcx Self, D::Error> {
let def_id = <DefId as Decodable<D>>::decode(decoder)?;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,7 @@ impl<'tcx> TyCtxt<'tcx> {
fmt,
self.0,
Adt,
Variant,
Array,
Slice,
RawPtr,
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ impl<'tcx> ty::TyS<'tcx> {
ty::Tuple(ref tys) if tys.is_empty() => format!("`{}`", self).into(),

ty::Adt(def, _) => format!("{} `{}`", def.descr(), tcx.def_path_str(def.did)).into(),
ty::Variant(ty, _) => match ty.kind() {
ty::Adt(def, _) => format!("{} `{}`", def.descr(), tcx.def_path_str(def.did)).into(),
_ => bug!("unexpected type: {:?}", ty.kind()),
}
ty::Foreign(def_id) => format!("extern type `{}`", tcx.def_path_str(def_id)).into(),
ty::Array(t, n) => {
if t.is_simple_ty() {
Expand Down Expand Up @@ -315,6 +319,10 @@ impl<'tcx> ty::TyS<'tcx> {
| ty::Never => "type".into(),
ty::Tuple(ref tys) if tys.is_empty() => "unit type".into(),
ty::Adt(def, _) => def.descr().into(),
ty::Variant(ty, _) => match ty.kind() {
ty::Adt(def, _) => format!("{} variant", def.descr()).into(),
_ => bug!("unexpected type: {:?}", ty.kind()),
}
ty::Foreign(_) => "extern type".into(),
ty::Array(..) => "array".into(),
ty::Slice(_) => "slice".into(),
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/fast_reject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ pub fn simplify_type(
ty::Uint(uint_type) => Some(UintSimplifiedType(uint_type)),
ty::Float(float_type) => Some(FloatSimplifiedType(float_type)),
ty::Adt(def, _) => Some(AdtSimplifiedType(def.did)),
ty::Variant(ref ty, _) => match ty.kind() {
ty::Adt(def, _) => Some(AdtSimplifiedType(def.did)),
_ => bug!("unexpected type: {:?}", ty.kind()),
}
ty::Str => Some(StrSimplifiedType),
ty::Array(..) | ty::Slice(_) => Some(ArraySimplifiedType),
ty::RawPtr(_) => Some(PtrSimplifiedType),
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ impl FlagComputation {
self.add_substs(substs);
}

&ty::Variant(ty, _) => match ty.kind() {
ty::Adt(_, substs) => self.add_substs(substs),
_ => bug!("unexpected type: {:?}", ty.kind()),
}

&ty::Projection(data) => {
self.add_flags(TypeFlags::HAS_TY_PROJECTION);
self.add_projection_ty(data);
Expand Down
Loading