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
7 changes: 2 additions & 5 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,20 +683,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let discr_val = self.cast_from_scalar(tag_bits, tag_layout, discr_layout.ty);
let discr_bits = discr_val.assert_bits(discr_layout.size);
// Convert discriminant to variant index, and catch invalid discriminants.
let index = match *op.layout.ty.kind() {
let index = match *op.layout.ty.strip_variant_type().kind() {
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()),
}
ty::Generator(def_id, substs, _) => {
let substs = substs.as_generator();
substs
.discriminants(def_id, *self.tcx)
.find(|(_, var)| var.val == discr_bits)
}
ty::Variant(..) => unreachable!(),
_ => span_bug!(self.cur_span(), "tagged layout for non-adt non-generator"),
}
.ok_or_else(|| err_ub!(InvalidTag(Scalar::from_uint(tag_bits, tag_layout.size))))?;
Expand Down
46 changes: 8 additions & 38 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,8 @@ 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() {
Copy link
Member

Choose a reason for hiding this comment

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

Variant types should never have Variants::Multiple layout so this should be unreachable.

Generally I am not very happy with strip_variant_type, from a conceptual perspective this rarely seems like the right thing to do -- but maybe my intuition is leading me astray here.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 Layout::for_variant needs to be fixed to return ty::Variant.

Copy link
Member

Choose a reason for hiding this comment

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

layout is the outer layout of the enum here, why should we not encounter adt/generators here?

It's the other match below in the same function where we should not encounter them any more.

ty::Adt(def, ..) if def.is_enum() => PathElem::EnumTag,
ty::Variant(ty, ..) => match ty.kind() {
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 All @@ -232,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;
Expand Down Expand Up @@ -276,20 +272,6 @@ 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() => {
// 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 @@ -513,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!(
Expand Down Expand Up @@ -585,17 +567,6 @@ 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() {
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 @@ -641,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!(),
}
}

Expand Down Expand Up @@ -756,15 +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();
Copy link
Member

Choose a reason for hiding this comment

The 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),
ty::Variant(ty, ..) => match ty.kind() {
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),
_ => bug!("Unexpected type with variant: {:?}", ty),
};
self.with_elem(name, move |this| this.visit_value(new_op))
}
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_infer/src/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ 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
11 changes: 1 addition & 10 deletions compiler/rustc_middle/src/ty/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,7 @@ 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()),
}
ty::Variant(ty, _) => Self::from_ty(ty),
ty::RawPtr(mt) => Some(CastTy::Ptr(mt)),
ty::FnPtr(..) => Some(CastTy::FnPtr),
_ => None,
Expand Down
7 changes: 2 additions & 5 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,7 @@ 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::Variant(ty, _) => ty.sort_string(tcx),
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 @@ -322,7 +319,7 @@ impl<'tcx> ty::TyS<'tcx> {
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
5 changes: 1 addition & 4 deletions compiler/rustc_middle/src/ty/fast_reject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,7 @@ 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::Variant(ty, _) => simplify_type(tcx, ty, can_simplify_params),
ty::Str => Some(StrSimplifiedType),
ty::Array(..) | ty::Slice(_) => Some(ArraySimplifiedType),
ty::RawPtr(_) => Some(PtrSimplifiedType),
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_middle/src/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,7 @@ 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::Variant(ty, _) => self.add_kind(ty.kind()),

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