Auto merge of #53721 - arielb1:exhaustively-unpun, r=nikomatsakis

fix `is_non_exhaustive` confusion between structs and enums

Structs and enums can both be non-exhaustive, with a very different
meaning. This PR splits `is_non_exhaustive` to 2 separate functions - 1
for structs, and another for enums, and fixes the places that got the
usage confused.

Fixes #53549.

r? @eddyb
This commit is contained in:
bors 2018-09-06 06:42:19 +00:00
commit 20ca02569a
10 changed files with 156 additions and 58 deletions

View File

@ -344,13 +344,13 @@ impl<'a> HashStable<StableHashingContext<'a>> for ty::AdtFlags {
}
}
impl_stable_hash_for!(struct ty::VariantDef {
did,
name,
discr,
fields,
ctor_kind
});
impl<'a> HashStable<StableHashingContext<'a>> for ty::VariantFlags {
fn hash_stable<W: StableHasherResult>(&self,
_: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
std_hash::Hash::hash(self, hasher);
}
}
impl_stable_hash_for!(enum ty::VariantDiscr {
Explicit(def_id),

View File

@ -1692,12 +1692,17 @@ bitflags! {
const IS_FUNDAMENTAL = 1 << 2;
const IS_UNION = 1 << 3;
const IS_BOX = 1 << 4;
/// Indicates whether this abstract data type will be expanded on in future (new
/// fields/variants) and as such, whether downstream crates must match exhaustively on the
/// fields/variants of this data type.
///
/// See RFC 2008 (<https://github.com/rust-lang/rfcs/pull/2008>).
const IS_NON_EXHAUSTIVE = 1 << 5;
/// Indicates whether the variant list of this ADT is `#[non_exhaustive]`.
/// (i.e., this flag is never set unless this ADT is an enum).
const IS_VARIANT_LIST_NON_EXHAUSTIVE = 1 << 5;
}
}
bitflags! {
pub struct VariantFlags: u32 {
const NO_VARIANT_FLAGS = 0;
/// Indicates whether the field list of this variant is `#[non_exhaustive]`.
const IS_FIELD_LIST_NON_EXHAUSTIVE = 1 << 0;
}
}
@ -1710,8 +1715,56 @@ pub struct VariantDef {
pub discr: VariantDiscr,
pub fields: Vec<FieldDef>,
pub ctor_kind: CtorKind,
flags: VariantFlags,
}
impl<'a, 'gcx, 'tcx> VariantDef {
/// Create a new `VariantDef`.
///
/// - `did` is the DefId used for the variant - for tuple-structs, it is the constructor DefId,
/// and for everything else, it is the variant DefId.
/// - `attribute_def_id` is the DefId that has the variant's attributes.
pub fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>,
did: DefId,
name: Name,
discr: VariantDiscr,
fields: Vec<FieldDef>,
adt_kind: AdtKind,
ctor_kind: CtorKind)
-> Self
{
debug!("VariantDef::new({:?}, {:?}, {:?}, {:?}, {:?}, {:?})", did, name, discr, fields,
adt_kind, ctor_kind);
let mut flags = VariantFlags::NO_VARIANT_FLAGS;
if adt_kind == AdtKind::Struct && tcx.has_attr(did, "non_exhaustive") {
debug!("found non-exhaustive field list for {:?}", did);
flags = flags | VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE;
}
VariantDef {
did,
name,
discr,
fields,
ctor_kind,
flags
}
}
#[inline]
pub fn is_field_list_non_exhaustive(&self) -> bool {
self.flags.intersects(VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE)
}
}
impl_stable_hash_for!(struct VariantDef {
did,
name,
discr,
fields,
ctor_kind,
flags
});
#[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable)]
pub enum VariantDiscr {
/// Explicit value for this variant, i.e. `X = 123`.
@ -1850,7 +1903,7 @@ impl_stable_hash_for!(struct ReprFlags {
/// Represents the repr options provided by the user,
#[derive(Copy, Clone, Eq, PartialEq, RustcEncodable, RustcDecodable, Default)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, RustcEncodable, RustcDecodable, Default)]
pub struct ReprOptions {
pub int: Option<attr::IntType>,
pub align: u32,
@ -1939,6 +1992,7 @@ impl<'a, 'gcx, 'tcx> AdtDef {
kind: AdtKind,
variants: Vec<VariantDef>,
repr: ReprOptions) -> Self {
debug!("AdtDef::new({:?}, {:?}, {:?}, {:?})", did, kind, variants, repr);
let mut flags = AdtFlags::NO_ADT_FLAGS;
let attrs = tcx.get_attrs(did);
if attr::contains_name(&attrs, "fundamental") {
@ -1950,8 +2004,9 @@ impl<'a, 'gcx, 'tcx> AdtDef {
if Some(did) == tcx.lang_items().owned_box() {
flags = flags | AdtFlags::IS_BOX;
}
if tcx.has_attr(did, "non_exhaustive") {
flags = flags | AdtFlags::IS_NON_EXHAUSTIVE;
if kind == AdtKind::Enum && tcx.has_attr(did, "non_exhaustive") {
debug!("found non-exhaustive variant list for {:?}", did);
flags = flags | AdtFlags::IS_VARIANT_LIST_NON_EXHAUSTIVE;
}
match kind {
AdtKind::Enum => flags = flags | AdtFlags::IS_ENUM,
@ -1982,8 +2037,8 @@ impl<'a, 'gcx, 'tcx> AdtDef {
}
#[inline]
pub fn is_non_exhaustive(&self) -> bool {
self.flags.intersects(AdtFlags::IS_NON_EXHAUSTIVE)
pub fn is_variant_list_non_exhaustive(&self) -> bool {
self.flags.intersects(AdtFlags::IS_VARIANT_LIST_NON_EXHAUSTIVE)
}
/// Returns the kind of the ADT - Struct or Enum.

View File

@ -542,7 +542,13 @@ impl<'a, 'tcx> CrateMetadata {
self.def_path_table.def_path_hash(item_id))
}
fn get_variant(&self, item: &Entry, index: DefIndex) -> ty::VariantDef {
fn get_variant(&self,
tcx: TyCtxt<'a, 'tcx, 'tcx>,
item: &Entry,
index: DefIndex,
adt_kind: ty::AdtKind)
-> ty::VariantDef
{
let data = match item.kind {
EntryKind::Variant(data) |
EntryKind::Struct(data, _) |
@ -550,10 +556,12 @@ impl<'a, 'tcx> CrateMetadata {
_ => bug!(),
};
ty::VariantDef {
did: self.local_def_id(data.struct_ctor.unwrap_or(index)),
name: self.item_name(index).as_symbol(),
fields: item.children.decode(self).map(|index| {
ty::VariantDef::new(
tcx,
self.local_def_id(data.struct_ctor.unwrap_or(index)),
self.item_name(index).as_symbol(),
data.discr,
item.children.decode(self).map(|index| {
let f = self.entry(index);
ty::FieldDef {
did: self.local_def_id(index),
@ -561,9 +569,9 @@ impl<'a, 'tcx> CrateMetadata {
vis: f.visibility.decode(self)
}
}).collect(),
discr: data.discr,
ctor_kind: data.ctor_kind,
}
adt_kind,
data.ctor_kind
)
}
pub fn get_adt_def(&self,
@ -584,11 +592,11 @@ impl<'a, 'tcx> CrateMetadata {
item.children
.decode(self)
.map(|index| {
self.get_variant(&self.entry(index), index)
self.get_variant(tcx, &self.entry(index), index, kind)
})
.collect()
} else {
vec![self.get_variant(&item, item_id)]
vec![self.get_variant(tcx, &item, item_id, kind)]
};
tcx.alloc_adt_def(did, kind, variants, repr)

View File

@ -740,7 +740,9 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
// If the structure is marked as non_exhaustive then lower the visibility
// to within the crate.
if adt_def.is_non_exhaustive() && ctor_vis == ty::Visibility::Public {
if adt_def.non_enum_variant().is_field_list_non_exhaustive() &&
ctor_vis == ty::Visibility::Public
{
ctor_vis = ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX));
}

View File

@ -381,7 +381,7 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> {
fn is_non_exhaustive_enum(&self, ty: Ty<'tcx>) -> bool {
match ty.sty {
ty::Adt(adt_def, ..) => adt_def.is_enum() && adt_def.is_non_exhaustive(),
ty::Adt(adt_def, ..) => adt_def.is_variant_list_non_exhaustive(),
_ => false,
}
}

View File

@ -686,7 +686,9 @@ impl<'a, 'tcx> TypePrivacyVisitor<'a, 'tcx> {
// visibility to within the crate.
let struct_def_id = self.tcx.hir.get_parent_did(node_id);
let adt_def = self.tcx.adt_def(struct_def_id);
if adt_def.is_non_exhaustive() && ctor_vis == ty::Visibility::Public {
if adt_def.non_enum_variant().is_field_list_non_exhaustive()
&& ctor_vis == ty::Visibility::Public
{
ctor_vis = ty::Visibility::Restricted(
DefId::local(CRATE_DEF_INDEX));
}

View File

@ -950,7 +950,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects");
}
// Require `..` if struct has non_exhaustive attribute.
if adt.is_struct() && adt.is_non_exhaustive() && !adt.did.is_local() && !etc {
if variant.is_field_list_non_exhaustive() && !adt.did.is_local() && !etc {
span_err!(tcx.sess, span, E0638,
"`..` required with {} marked as non-exhaustive",
kind_name);

View File

@ -3471,7 +3471,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
// re-link the regions that EIfEO can erase.
self.demand_eqtype(span, adt_ty_hint, adt_ty);
let (substs, adt_kind, kind_name) = match &adt_ty.sty{
let (substs, adt_kind, kind_name) = match &adt_ty.sty {
&ty::Adt(adt, substs) => {
(substs, adt.adt_kind(), adt.variant_descr())
}
@ -3645,13 +3645,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
base_expr: &'gcx Option<P<hir::Expr>>) -> Ty<'tcx>
{
// Find the relevant variant
let (variant, struct_ty) =
if let Some(variant_ty) = self.check_struct_path(qpath, expr.id) {
variant_ty
} else {
self.check_struct_fields_on_error(fields, base_expr);
return self.tcx.types.err;
};
let (variant, adt_ty) =
if let Some(variant_ty) = self.check_struct_path(qpath, expr.id) {
variant_ty
} else {
self.check_struct_fields_on_error(fields, base_expr);
return self.tcx.types.err;
};
let path_span = match *qpath {
hir::QPath::Resolved(_, ref path) => path.span,
@ -3659,23 +3659,22 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
};
// Prohibit struct expressions when non exhaustive flag is set.
if let ty::Adt(adt, _) = struct_ty.sty {
if !adt.did.is_local() && adt.is_non_exhaustive() {
span_err!(self.tcx.sess, expr.span, E0639,
"cannot create non-exhaustive {} using struct expression",
adt.variant_descr());
}
let adt = adt_ty.ty_adt_def().expect("`check_struct_path` returned non-ADT type");
if !adt.did.is_local() && variant.is_field_list_non_exhaustive() {
span_err!(self.tcx.sess, expr.span, E0639,
"cannot create non-exhaustive {} using struct expression",
adt.variant_descr());
}
let error_happened = self.check_expr_struct_fields(struct_ty, expected, expr.id, path_span,
let error_happened = self.check_expr_struct_fields(adt_ty, expected, expr.id, path_span,
variant, fields, base_expr.is_none());
if let &Some(ref base_expr) = base_expr {
// If check_expr_struct_fields hit an error, do not attempt to populate
// the fields with the base_expr. This could cause us to hit errors later
// when certain fields are assumed to exist that in fact do not.
if !error_happened {
self.check_expr_has_type_or_error(base_expr, struct_ty);
match struct_ty.sty {
self.check_expr_has_type_or_error(base_expr, adt_ty);
match adt_ty.sty {
ty::Adt(adt, substs) if adt.is_struct() => {
let fru_field_types = adt.non_enum_variant().fields.iter().map(|f| {
self.normalize_associated_types_in(expr.span, &f.ty(self.tcx, substs))
@ -3693,8 +3692,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
}
}
self.require_type_is_sized(struct_ty, expr.span, traits::StructInitializerSized);
struct_ty
self.require_type_is_sized(adt_ty, expr.span, traits::StructInitializerSized);
adt_ty
}

View File

@ -549,12 +549,13 @@ fn convert_enum_variant_types<'a, 'tcx>(
}
}
fn convert_struct_variant<'a, 'tcx>(
fn convert_variant<'a, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
did: DefId,
name: ast::Name,
discr: ty::VariantDiscr,
def: &hir::VariantData,
adt_kind: ty::AdtKind
) -> ty::VariantDef {
let mut seen_fields: FxHashMap<ast::Ident, Span> = FxHashMap();
let node_id = tcx.hir.as_local_node_id(did).unwrap();
@ -585,13 +586,13 @@ fn convert_struct_variant<'a, 'tcx>(
}
})
.collect();
ty::VariantDef {
ty::VariantDef::new(tcx,
did,
name,
discr,
fields,
ctor_kind: CtorKind::from_hir(def),
}
adt_kind,
CtorKind::from_hir(def))
}
fn adt_def<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx ty::AdtDef {
@ -621,7 +622,7 @@ fn adt_def<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx ty::Ad
};
distance_from_explicit += 1;
convert_struct_variant(tcx, did, v.node.name, discr, &v.node.data)
convert_variant(tcx, did, v.node.name, discr, &v.node.data, AdtKind::Enum)
})
.collect(),
)
@ -635,23 +636,25 @@ fn adt_def<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx ty::Ad
};
(
AdtKind::Struct,
vec![convert_struct_variant(
vec![convert_variant(
tcx,
ctor_id.unwrap_or(def_id),
item.name,
ty::VariantDiscr::Relative(0),
def,
AdtKind::Struct
)],
)
}
ItemKind::Union(ref def, _) => (
AdtKind::Union,
vec![convert_struct_variant(
vec![convert_variant(
tcx,
def_id,
item.name,
ty::VariantDiscr::Relative(0),
def,
AdtKind::Union
)],
),
_ => bug!(),

View File

@ -30,4 +30,33 @@ fn main() {
match enum_unit {
_ => "no error with only wildcard"
};
// issue #53549 - check that variant constructors can still be called normally.
match NonExhaustiveEnum::Unit {
NonExhaustiveEnum::Unit => {},
_ => {}
};
match NonExhaustiveEnum::Tuple(2) {
NonExhaustiveEnum::Tuple(2) => {},
_ => {}
};
match (NonExhaustiveEnum::Unit {}) {
NonExhaustiveEnum::Unit {} => {},
_ => {}
};
match (NonExhaustiveEnum::Tuple { 0: 2 }) {
NonExhaustiveEnum::Tuple { 0: 2 } => {},
_ => {}
};
match (NonExhaustiveEnum::Struct { field: 2 }) {
NonExhaustiveEnum::Struct { field: 2 } => {},
_ => {}
};
}