From 993e7e2622da632705661007d5a3bce812cc6d3d Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 26 Aug 2018 16:36:50 +0300 Subject: [PATCH] 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. --- src/librustc/ty/mod.rs | 35 +++++++++++++++++- src/librustc_metadata/encoder.rs | 2 +- src/librustc_mir/hair/pattern/_match.rs | 2 +- src/librustc_privacy/lib.rs | 4 +- src/librustc_typeck/check/_match.rs | 2 +- src/librustc_typeck/check/mod.rs | 37 +++++++++---------- .../run-pass/rfc-2008-non-exhaustive/enums.rs | 29 +++++++++++++++ 7 files changed, 87 insertions(+), 24 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 77b4d32c397d7..c60de9c386e08 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -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 { @@ -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", diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 8219ec3df248a..27f4577c442a6 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -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)); } diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index e9d65caf08743..4f14a45bfc835 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -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, } } diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index f68f2d0da6850..ea13a3d00331d 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -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)); } diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index ae68584f2446f..b4dcea0eb329a 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -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); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 2b33f28934630..d4013b10c319a 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -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()) } @@ -3639,13 +3639,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { base_expr: &'gcx Option>) -> 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, @@ -3653,23 +3653,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() && 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)) @@ -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 } diff --git a/src/test/run-pass/rfc-2008-non-exhaustive/enums.rs b/src/test/run-pass/rfc-2008-non-exhaustive/enums.rs index 9d41eca8fe5d2..83fb24cda088c 100644 --- a/src/test/run-pass/rfc-2008-non-exhaustive/enums.rs +++ b/src/test/run-pass/rfc-2008-non-exhaustive/enums.rs @@ -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 } => {}, + _ => {} + }; + }