From bbcf5cfede28dd088f05eec0ef9508b92ff12f64 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 13 Apr 2020 15:16:43 +0100 Subject: [PATCH 1/8] Factor the code that generates TyErrs --- src/librustc_mir_build/hair/pattern/_match.rs | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index 51ba84416d64d..536506175a29d 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -886,36 +886,37 @@ impl<'tcx> Constructor<'tcx> { .fields .iter() .map(|field| { + let ty = field.ty(cx.tcx, substs); let is_visible = adt.is_enum() || field.vis.is_accessible_from(cx.module, cx.tcx); - let is_uninhabited = cx.is_uninhabited(field.ty(cx.tcx, substs)); - match (is_visible, is_non_exhaustive, is_uninhabited) { - // Treat all uninhabited types in non-exhaustive variants as - // `TyErr`. - (_, true, true) => cx.tcx.types.err, - // Treat all non-visible fields as `TyErr`. They can't appear - // in any other pattern from this match (because they are - // private), so their type does not matter - but we don't want - // to know they are uninhabited. - (false, ..) => cx.tcx.types.err, - (true, ..) => { - let ty = field.ty(cx.tcx, substs); - match ty.kind { - // If the field type returned is an array of an unknown - // size return an TyErr. - ty::Array(_, len) - if len + let is_uninhabited = cx.is_uninhabited(ty); + // Treat all non-visible fields as `TyErr`. They can't appear + // in any other pattern from this match (because they are + // private), so their type does not matter - but we don't want + // to know they are uninhabited. + let allowed_to_inspect = is_visible + && match (is_non_exhaustive, is_uninhabited) { + // Treat all uninhabited types in non-exhaustive variants as + // `TyErr`. + (true, true) => false, + (_, _) => { + match ty.kind { + // If the field type returned is an array of an unknown + // size return an TyErr. + ty::Array(_, len) => len .try_eval_usize(cx.tcx, cx.param_env) - .is_none() => - { - cx.tcx.types.err + .is_some(), + _ => true, } - _ => ty, } - } + }; + + if allowed_to_inspect { + Pat::wildcard_from_ty(ty) + } else { + Pat::wildcard_from_ty(cx.tcx.types.err) } }) - .map(Pat::wildcard_from_ty) .collect() } } From a18caf6118db3794d8f63616396ce30b8be44b2d Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 13 Apr 2020 16:23:39 +0100 Subject: [PATCH 2/8] We already handle arrays of unknown length correctly --- src/librustc_mir_build/hair/pattern/_match.rs | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index 536506175a29d..850311222ba4d 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -894,22 +894,10 @@ impl<'tcx> Constructor<'tcx> { // in any other pattern from this match (because they are // private), so their type does not matter - but we don't want // to know they are uninhabited. - let allowed_to_inspect = is_visible - && match (is_non_exhaustive, is_uninhabited) { - // Treat all uninhabited types in non-exhaustive variants as - // `TyErr`. - (true, true) => false, - (_, _) => { - match ty.kind { - // If the field type returned is an array of an unknown - // size return an TyErr. - ty::Array(_, len) => len - .try_eval_usize(cx.tcx, cx.param_env) - .is_some(), - _ => true, - } - } - }; + // Also treat all uninhabited types in non-exhaustive variants as + // `TyErr`. + let allowed_to_inspect = + is_visible && !(is_non_exhaustive && is_uninhabited); if allowed_to_inspect { Pat::wildcard_from_ty(ty) From 4e992ead0487edfd8ef832f5e3162e3bdd880180 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 13 Apr 2020 17:45:12 +0100 Subject: [PATCH 3/8] Only need TyErr for uninhabited types --- src/librustc_mir_build/hair/pattern/_match.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index 850311222ba4d..b841a385eed5a 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -889,15 +889,15 @@ impl<'tcx> Constructor<'tcx> { let ty = field.ty(cx.tcx, substs); let is_visible = adt.is_enum() || field.vis.is_accessible_from(cx.module, cx.tcx); - let is_uninhabited = cx.is_uninhabited(ty); - // Treat all non-visible fields as `TyErr`. They can't appear - // in any other pattern from this match (because they are + let is_inhabited = !cx.is_uninhabited(ty); + // Treat all uninhabited non-visible fields as `TyErr`. They can't + // appear in any other pattern from this match (because they are // private), so their type does not matter - but we don't want // to know they are uninhabited. // Also treat all uninhabited types in non-exhaustive variants as // `TyErr`. let allowed_to_inspect = - is_visible && !(is_non_exhaustive && is_uninhabited); + is_inhabited || (is_visible && !is_non_exhaustive); if allowed_to_inspect { Pat::wildcard_from_ty(ty) From d6d31ffd323863fe5b93fbd6860d491c269f3021 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 13 Apr 2020 18:35:46 +0100 Subject: [PATCH 4/8] Small tweaks --- src/librustc_mir_build/hair/pattern/_match.rs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index b841a385eed5a..7195bfe356ba0 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -602,7 +602,7 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { } } - // Returns whether the given type is an enum from another crate declared `#[non_exhaustive]`. + /// Returns whether the given type is an enum from another crate declared `#[non_exhaustive]`. crate fn is_foreign_non_exhaustive_enum(&self, ty: Ty<'tcx>) -> bool { match ty.kind { ty::Adt(def, ..) => { @@ -612,8 +612,8 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { } } - // Returns whether the given variant is from another crate and has its fields declared - // `#[non_exhaustive]`. + /// Returns whether the given variant is from another crate and has its fields declared + /// `#[non_exhaustive]`. fn is_foreign_non_exhaustive_variant(&self, ty: Ty<'tcx>, variant: &VariantDef) -> bool { match ty.kind { ty::Adt(def, ..) => variant.is_field_list_non_exhaustive() && !def.did.is_local(), @@ -733,8 +733,8 @@ impl Slice { #[derive(Clone, Debug, PartialEq)] enum Constructor<'tcx> { - /// The constructor of all patterns that don't vary by constructor, - /// e.g., struct patterns and fixed-length arrays. + /// The constructor for patterns that have a single constructor, like tuples, struct patterns + /// and fixed-length arrays. Single, /// Enum variants. Variant(DefId), @@ -2349,12 +2349,17 @@ fn specialize_one_pattern<'p, 'tcx>( PatKind::Variant { adt_def, variant_index, ref subpatterns, .. } => { let variant = &adt_def.variants[variant_index]; - let is_non_exhaustive = cx.is_foreign_non_exhaustive_variant(pat.ty, variant); - Some(Variant(variant.def_id)) - .filter(|variant_constructor| variant_constructor == constructor) - .map(|_| { - patterns_for_variant(cx, subpatterns, ctor_wild_subpatterns, is_non_exhaustive) - }) + if constructor == Variant(variant.def_id) { + let is_non_exhaustive = cx.is_foreign_non_exhaustive_variant(pat.ty, variant); + Some(patterns_for_variant( + cx, + subpatterns, + ctor_wild_subpatterns, + is_non_exhaustive, + )) + } else { + None + } } PatKind::Leaf { ref subpatterns } => { From b258a650a2f8ac8aaf5f8a0e18f9b09b8620d723 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 13 Apr 2020 18:36:05 +0100 Subject: [PATCH 5/8] Factor out the condition for hiding fields --- src/librustc_mir_build/hair/pattern/_match.rs | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index 7195bfe356ba0..cf8126b458665 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -620,6 +620,27 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { _ => false, } } + + /// In the cases of either a `#[non_exhaustive]` field list or a non-public field, we hide + /// uninhabited fields in order not to reveal the uninhabitedness of the whole variant. + fn hide_uninhabited_field( + &self, + adt_ty: Ty<'tcx>, + variant: &VariantDef, + field: &FieldDef, + ) -> bool { + match adt_ty.kind { + ty::Adt(def, substs) => { + let is_non_exhaustive = self.is_foreign_non_exhaustive_variant(adt_ty, variant); + let field_ty = field.ty(self.tcx, substs); + let is_visible = + adt.is_enum() || field.vis.is_accessible_from(self.module, self.tcx); + let is_uninhabited = self.is_uninhabited(field_ty); + is_uninhabited && (!is_visible || is_non_exhaustive) + } + _ => false, + } + } } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -881,30 +902,19 @@ impl<'tcx> Constructor<'tcx> { vec![Pat::wildcard_from_ty(substs.type_at(0))] } else { let variant = &adt.variants[self.variant_index_for_adt(cx, adt)]; - let is_non_exhaustive = cx.is_foreign_non_exhaustive_variant(ty, variant); variant .fields .iter() .map(|field| { - let ty = field.ty(cx.tcx, substs); - let is_visible = adt.is_enum() - || field.vis.is_accessible_from(cx.module, cx.tcx); - let is_inhabited = !cx.is_uninhabited(ty); - // Treat all uninhabited non-visible fields as `TyErr`. They can't - // appear in any other pattern from this match (because they are - // private), so their type does not matter - but we don't want - // to know they are uninhabited. - // Also treat all uninhabited types in non-exhaustive variants as - // `TyErr`. - let allowed_to_inspect = - is_inhabited || (is_visible && !is_non_exhaustive); - - if allowed_to_inspect { - Pat::wildcard_from_ty(ty) + // Treat hidden fields as TyErr so we don't know they are + // uninhabited. + if cx.hide_uninhabited_field(ty, variant, field) { + cx.tcx.types.err } else { - Pat::wildcard_from_ty(cx.tcx.types.err) + field.ty(cx.tcx, substs) } }) + .map(Pat::wildcard_from_ty) .collect() } } From 75e732d7fea0381002b2509dcd8d63ec6129ad32 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 13 Apr 2020 18:45:27 +0100 Subject: [PATCH 6/8] fixup! Small tweaks --- src/librustc_mir_build/hair/pattern/_match.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index cf8126b458665..a3857769a7cec 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -2359,7 +2359,7 @@ fn specialize_one_pattern<'p, 'tcx>( PatKind::Variant { adt_def, variant_index, ref subpatterns, .. } => { let variant = &adt_def.variants[variant_index]; - if constructor == Variant(variant.def_id) { + if constructor == &Variant(variant.def_id) { let is_non_exhaustive = cx.is_foreign_non_exhaustive_variant(pat.ty, variant); Some(patterns_for_variant( cx, From 81e321baae6ecc1239b917fe96335d9126a2eb6d Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 13 Apr 2020 18:45:45 +0100 Subject: [PATCH 7/8] fixup! Factor out the condition for hiding fields --- src/librustc_mir_build/hair/pattern/_match.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index a3857769a7cec..8d7397a0027de 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -242,7 +242,7 @@ use rustc_hir::{HirId, RangeEnd}; use rustc_middle::mir::interpret::{truncate, AllocId, ConstValue, Pointer, Scalar}; use rustc_middle::mir::Field; use rustc_middle::ty::layout::IntegerExt; -use rustc_middle::ty::{self, Const, Ty, TyCtxt, TypeFoldable, VariantDef}; +use rustc_middle::ty::{self, Const, FieldDef, Ty, TyCtxt, TypeFoldable, VariantDef}; use rustc_session::lint; use rustc_span::{Span, DUMMY_SP}; use rustc_target::abi::{Integer, Size, VariantIdx}; @@ -630,7 +630,7 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { field: &FieldDef, ) -> bool { match adt_ty.kind { - ty::Adt(def, substs) => { + ty::Adt(adt, substs) => { let is_non_exhaustive = self.is_foreign_non_exhaustive_variant(adt_ty, variant); let field_ty = field.ty(self.tcx, substs); let is_visible = From 1cb8f7b9511c625a072ab95485bdf706d847bb91 Mon Sep 17 00:00:00 2001 From: mark Date: Sun, 12 Apr 2020 15:04:21 -0500 Subject: [PATCH 8/8] de-abuse TyKind::Error: use dummy self type --- src/librustc_typeck/check/closure.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/closure.rs b/src/librustc_typeck/check/closure.rs index 1acbcc038891d..035e5880dc522 100644 --- a/src/librustc_typeck/check/closure.rs +++ b/src/librustc_typeck/check/closure.rs @@ -178,7 +178,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { match expected_ty.kind { ty::Dynamic(ref object_type, ..) => { let sig = object_type.projection_bounds().find_map(|pb| { - let pb = pb.with_self_ty(self.tcx, self.tcx.types.err); + let pb = pb.with_self_ty(self.tcx, self.tcx.types.trait_object_dummy_self); self.deduce_sig_from_projection(None, &pb) }); let kind = object_type