Skip to content

Commit

Permalink
fix is_non_exhaustive confusion between structs and enums
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
arielb1 committed Aug 26, 2018
1 parent caed80b commit 993e7e2
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 24 deletions.
35 changes: 34 additions & 1 deletion src/librustc/ty/mod.rs
Expand Up @@ -1981,10 +1981,30 @@ impl<'a, 'gcx, 'tcx> AdtDef {
}

#[inline]
pub fn is_non_exhaustive(&self) -> bool {
fn is_non_exhaustive(&self) -> bool {
self.flags.intersects(AdtFlags::IS_NON_EXHAUSTIVE)
}

#[inline]
pub fn is_enum_non_exhaustive(&self) -> bool {
match self.adt_kind() {
AdtKind::Enum => self.is_non_exhaustive(),
AdtKind::Struct | AdtKind::Union => {
bug!("is_non_exhaustive_enum called on non-enum `{:?}`", self);
}
}
}

#[inline]
pub fn is_univariant_non_exhaustive(&self) -> bool {
match self.adt_kind() {
AdtKind::Struct | AdtKind::Union => self.is_non_exhaustive(),
AdtKind::Enum => {
bug!("is_non_exhaustive_enum called on non-enum `{:?}`", self);
}
}
}

/// Returns the kind of the ADT - Struct or Enum.
#[inline]
pub fn adt_kind(&self) -> AdtKind {
Expand All @@ -1997,6 +2017,19 @@ impl<'a, 'gcx, 'tcx> AdtDef {
}
}

/// Return whether `variant` is non-exhaustive as a *struct* (i.e., whether
/// it can have additional fields).
pub fn is_variant_non_exhaustive(&self, _variant: &ty::VariantDef) -> bool {
match self.adt_kind() {
// A struct is non-exhaustive if it has a `#[non_exhaustive]` attribute.
AdtKind::Struct => self.is_non_exhaustive(),
// At this moment, all enum variants are exhaustive.
AdtKind::Enum => false,
// All unions are "exhaustive", as far as that makes sense.
AdtKind::Union => false,
}
}

pub fn descr(&self) -> &'static str {
match self.adt_kind() {
AdtKind::Struct => "struct",
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_metadata/encoder.rs
Expand Up @@ -740,7 +740,7 @@ 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.is_univariant_non_exhaustive() && ctor_vis == ty::Visibility::Public {
ctor_vis = ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX));
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/hair/pattern/_match.rs
Expand Up @@ -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_enum() && adt_def.is_enum_non_exhaustive(),
_ => false,
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_privacy/lib.rs
Expand Up @@ -684,7 +684,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.is_univariant_non_exhaustive()
&& ctor_vis == ty::Visibility::Public
{
ctor_vis = ty::Visibility::Restricted(
DefId::local(CRATE_DEF_INDEX));
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/_match.rs
Expand Up @@ -948,7 +948,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 adt.is_variant_non_exhaustive(variant) && !adt.did.is_local() && !etc {
span_err!(tcx.sess, span, E0638,
"`..` required with {} marked as non-exhaustive",
kind_name);
Expand Down
37 changes: 18 additions & 19 deletions src/librustc_typeck/check/mod.rs
Expand Up @@ -3465,7 +3465,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())
}
Expand Down Expand Up @@ -3639,37 +3639,36 @@ 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,
hir::QPath::TypeRelative(ref qself, _) => qself.span
};

// 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() && adt.is_variant_non_exhaustive(variant) {
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))
Expand All @@ -3687,8 +3686,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
}


Expand Down
29 changes: 29 additions & 0 deletions src/test/run-pass/rfc-2008-non-exhaustive/enums.rs
Expand Up @@ -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 } => {},
_ => {}
};

}

0 comments on commit 993e7e2

Please sign in to comment.