From 9d13520a6b00fe239c3990e487cd7449722c0c74 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Wed, 8 Apr 2020 04:26:48 +0300 Subject: [PATCH 01/13] Replace "rc"/"arc" lang items with Rc/Arc diagnostic items. --- src/liballoc/rc.rs | 3 +- src/liballoc/sync.rs | 3 +- src/librustc_error_codes/error_codes/E0152.md | 4 +-- src/librustc_error_codes/error_codes/E0718.md | 2 +- src/librustc_hir/lang_items.rs | 3 -- src/librustc_middle/ty/context.rs | 6 ++++ src/librustc_middle/ty/mod.rs | 23 +------------- src/librustc_middle/ty/sty.rs | 18 ----------- .../borrow_check/diagnostics/mod.rs | 30 +++++++++---------- .../borrow_check/diagnostics/move_errors.rs | 5 +++- .../diagnostics/mutability_errors.rs | 2 +- src/librustc_span/symbol.rs | 2 ++ src/librustc_typeck/check/expr.rs | 12 ++++---- src/test/ui/error-codes/E0152.rs | 2 +- src/test/ui/error-codes/E0152.stderr | 2 +- src/test/ui/error-codes/E0718.rs | 4 +-- src/test/ui/error-codes/E0718.stderr | 6 ++-- 17 files changed, 49 insertions(+), 78 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 9db997e864170..abc4056cf5695 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -279,7 +279,8 @@ struct RcBox { /// type `T`. /// /// [get_mut]: #method.get_mut -#[cfg_attr(not(test), lang = "rc")] +#[cfg_attr(all(bootstrap, not(test)), lang = "rc")] +#[cfg_attr(not(test), rustc_diagnostic_item = "Rc")] #[stable(feature = "rust1", since = "1.0.0")] pub struct Rc { ptr: NonNull>, diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 1cfb26eb35a11..b1b22e46a7c2f 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -207,7 +207,8 @@ macro_rules! acquire { /// counting in general. /// /// [rc_examples]: ../../std/rc/index.html#examples -#[cfg_attr(not(test), lang = "arc")] +#[cfg_attr(all(bootstrap, not(test)), lang = "arc")] +#[cfg_attr(not(test), rustc_diagnostic_item = "Arc")] #[stable(feature = "rust1", since = "1.0.0")] pub struct Arc { ptr: NonNull>, diff --git a/src/librustc_error_codes/error_codes/E0152.md b/src/librustc_error_codes/error_codes/E0152.md index 602dcb6e2c5b7..120c96b421104 100644 --- a/src/librustc_error_codes/error_codes/E0152.md +++ b/src/librustc_error_codes/error_codes/E0152.md @@ -5,8 +5,8 @@ Erroneous code example: ```compile_fail,E0152 #![feature(lang_items)] -#[lang = "arc"] -struct Foo; // error: duplicate lang item found: `arc` +#[lang = "owned_box"] +struct Foo; // error: duplicate lang item found: `owned_box` ``` Lang items are already implemented in the standard library. Unless you are diff --git a/src/librustc_error_codes/error_codes/E0718.md b/src/librustc_error_codes/error_codes/E0718.md index 8548ff0c15011..e7ae51ca58835 100644 --- a/src/librustc_error_codes/error_codes/E0718.md +++ b/src/librustc_error_codes/error_codes/E0718.md @@ -6,6 +6,6 @@ Examples of erroneous code: ```compile_fail,E0718 #![feature(lang_items)] -#[lang = "arc"] +#[lang = "owned_box"] static X: u32 = 42; ``` diff --git a/src/librustc_hir/lang_items.rs b/src/librustc_hir/lang_items.rs index 89457009a8bfa..5a3a9cabeb450 100644 --- a/src/librustc_hir/lang_items.rs +++ b/src/librustc_hir/lang_items.rs @@ -254,7 +254,4 @@ language_item_table! { AlignOffsetLangItem, "align_offset", align_offset_fn, Target::Fn; TerminationTraitLangItem, "termination", termination, Target::Trait; - - Arc, "arc", arc, Target::Struct; - Rc, "rc", rc, Target::Struct; } diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index 95d0c758d08de..076134b8e9462 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -2213,6 +2213,12 @@ impl<'tcx> TyCtxt<'tcx> { Some(self.mk_generic_adt(def_id, ty)) } + #[inline] + pub fn mk_diagnostic_item(self, ty: Ty<'tcx>, name: Symbol) -> Option> { + let def_id = self.get_diagnostic_item(name)?; + Some(self.mk_generic_adt(def_id, ty)) + } + #[inline] pub fn mk_maybe_uninit(self, ty: Ty<'tcx>) -> Ty<'tcx> { let def_id = self.require_lang_item(lang_items::MaybeUninitLangItem, None); diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 1870856150f50..3683ea3288feb 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -1856,14 +1856,9 @@ bitflags! { const IS_BOX = 1 << 6; /// Indicates whether the type is `ManuallyDrop`. const IS_MANUALLY_DROP = 1 << 7; - // FIXME(matthewjasper) replace these with diagnostic items - /// Indicates whether the type is an `Arc`. - const IS_ARC = 1 << 8; - /// Indicates whether the type is an `Rc`. - const IS_RC = 1 << 9; /// 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 << 10; + const IS_VARIANT_LIST_NON_EXHAUSTIVE = 1 << 8; } } @@ -2248,12 +2243,6 @@ impl<'tcx> AdtDef { if Some(did) == tcx.lang_items().manually_drop() { flags |= AdtFlags::IS_MANUALLY_DROP; } - if Some(did) == tcx.lang_items().arc() { - flags |= AdtFlags::IS_ARC; - } - if Some(did) == tcx.lang_items().rc() { - flags |= AdtFlags::IS_RC; - } AdtDef { did, variants, flags, repr } } @@ -2332,16 +2321,6 @@ impl<'tcx> AdtDef { self.flags.contains(AdtFlags::IS_PHANTOM_DATA) } - /// Returns `true` if this is `Arc`. - pub fn is_arc(&self) -> bool { - self.flags.contains(AdtFlags::IS_ARC) - } - - /// Returns `true` if this is `Rc`. - pub fn is_rc(&self) -> bool { - self.flags.contains(AdtFlags::IS_RC) - } - /// Returns `true` if this is Box. #[inline] pub fn is_box(&self) -> bool { diff --git a/src/librustc_middle/ty/sty.rs b/src/librustc_middle/ty/sty.rs index f09327886872c..fbd5049b0fbcb 100644 --- a/src/librustc_middle/ty/sty.rs +++ b/src/librustc_middle/ty/sty.rs @@ -1865,24 +1865,6 @@ impl<'tcx> TyS<'tcx> { self.is_region_ptr() || self.is_unsafe_ptr() || self.is_fn_ptr() } - /// Returns `true` if this type is an `Arc`. - #[inline] - pub fn is_arc(&self) -> bool { - match self.kind { - Adt(def, _) => def.is_arc(), - _ => false, - } - } - - /// Returns `true` if this type is an `Rc`. - #[inline] - pub fn is_rc(&self) -> bool { - match self.kind { - Adt(def, _) => def.is_rc(), - _ => false, - } - } - #[inline] pub fn is_box(&self) -> bool { match self.kind { diff --git a/src/librustc_mir/borrow_check/diagnostics/mod.rs b/src/librustc_mir/borrow_check/diagnostics/mod.rs index 619ae0ff8faa1..404cc0c74679f 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mod.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mod.rs @@ -11,7 +11,7 @@ use rustc_middle::mir::{ }; use rustc_middle::ty::print::Print; use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt}; -use rustc_span::Span; +use rustc_span::{symbol::sym, Span}; use rustc_target::abi::VariantIdx; use super::borrow_set::BorrowData; @@ -632,20 +632,20 @@ pub(super) enum BorrowedContentSource<'tcx> { } impl BorrowedContentSource<'tcx> { - pub(super) fn describe_for_unnamed_place(&self) -> String { + pub(super) fn describe_for_unnamed_place(&self, tcx: TyCtxt<'_>) -> String { match *self { BorrowedContentSource::DerefRawPointer => "a raw pointer".to_string(), BorrowedContentSource::DerefSharedRef => "a shared reference".to_string(), BorrowedContentSource::DerefMutableRef => "a mutable reference".to_string(), - BorrowedContentSource::OverloadedDeref(ty) => { - if ty.is_rc() { + BorrowedContentSource::OverloadedDeref(ty) => match ty.kind { + ty::Adt(def, _) if tcx.is_diagnostic_item(sym::Rc, def.did) => { "an `Rc`".to_string() - } else if ty.is_arc() { + } + ty::Adt(def, _) if tcx.is_diagnostic_item(sym::Arc, def.did) => { "an `Arc`".to_string() - } else { - format!("dereference of `{}`", ty) } - } + _ => format!("dereference of `{}`", ty), + }, BorrowedContentSource::OverloadedIndex(ty) => format!("index of `{}`", ty), } } @@ -662,22 +662,22 @@ impl BorrowedContentSource<'tcx> { } } - pub(super) fn describe_for_immutable_place(&self) -> String { + pub(super) fn describe_for_immutable_place(&self, tcx: TyCtxt<'_>) -> String { match *self { BorrowedContentSource::DerefRawPointer => "a `*const` pointer".to_string(), BorrowedContentSource::DerefSharedRef => "a `&` reference".to_string(), BorrowedContentSource::DerefMutableRef => { bug!("describe_for_immutable_place: DerefMutableRef isn't immutable") } - BorrowedContentSource::OverloadedDeref(ty) => { - if ty.is_rc() { + BorrowedContentSource::OverloadedDeref(ty) => match ty.kind { + ty::Adt(def, _) if tcx.is_diagnostic_item(sym::Rc, def.did) => { "an `Rc`".to_string() - } else if ty.is_arc() { + } + ty::Adt(def, _) if tcx.is_diagnostic_item(sym::Arc, def.did) => { "an `Arc`".to_string() - } else { - format!("a dereference of `{}`", ty) } - } + _ => format!("a dereference of `{}`", ty), + }, BorrowedContentSource::OverloadedIndex(ty) => format!("an index of `{}`", ty), } } diff --git a/src/librustc_mir/borrow_check/diagnostics/move_errors.rs b/src/librustc_mir/borrow_check/diagnostics/move_errors.rs index d32533b6ce9a1..1fbecb75dff53 100644 --- a/src/librustc_mir/borrow_check/diagnostics/move_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/move_errors.rs @@ -377,7 +377,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { span, &format!("`{}` which is behind a {}", place_desc, source_desc), ), - (_, _) => self.cannot_move_out_of(span, &source.describe_for_unnamed_place()), + (_, _) => self.cannot_move_out_of( + span, + &source.describe_for_unnamed_place(self.infcx.tcx), + ), } } }; diff --git a/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs b/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs index 35edd3d803db9..635c299cf8136 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mutability_errors.rs @@ -120,7 +120,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { local: the_place_err.local, projection: proj_base, }); - let pointer_type = source.describe_for_immutable_place(); + let pointer_type = source.describe_for_immutable_place(self.infcx.tcx); opt_source = Some(source); if let Some(desc) = access_place_desc { item_msg = format!("`{}`", desc); diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 54b404e1161b9..6845cb3b9a352 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -144,6 +144,7 @@ symbols! { any, arbitrary_enum_discriminant, arbitrary_self_types, + Arc, Arguments, ArgumentV1, arm_target_feature, @@ -582,6 +583,7 @@ symbols! { raw_dylib, raw_identifiers, raw_ref_op, + Rc, Ready, reason, recursion_limit, diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 9c57ffaf055ac..57e2349bb2e80 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -902,8 +902,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { error: MethodError<'tcx>, ) { let rcvr = &args[0]; - let try_alt_rcvr = |err: &mut DiagnosticBuilder<'_>, rcvr_t, lang_item| { - if let Some(new_rcvr_t) = self.tcx.mk_lang_item(rcvr_t, lang_item) { + let try_alt_rcvr = |err: &mut DiagnosticBuilder<'_>, new_rcvr_t| { + if let Some(new_rcvr_t) = new_rcvr_t { if let Ok(pick) = self.lookup_probe( span, segment.ident, @@ -931,10 +931,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Try alternative arbitrary self types that could fulfill this call. // FIXME: probe for all types that *could* be arbitrary self-types, not // just this whitelist. - try_alt_rcvr(&mut err, rcvr_t, lang_items::OwnedBoxLangItem); - try_alt_rcvr(&mut err, rcvr_t, lang_items::PinTypeLangItem); - try_alt_rcvr(&mut err, rcvr_t, lang_items::Arc); - try_alt_rcvr(&mut err, rcvr_t, lang_items::Rc); + try_alt_rcvr(&mut err, self.tcx.mk_lang_item(rcvr_t, lang_items::OwnedBoxLangItem)); + try_alt_rcvr(&mut err, self.tcx.mk_lang_item(rcvr_t, lang_items::PinTypeLangItem)); + try_alt_rcvr(&mut err, self.tcx.mk_diagnostic_item(rcvr_t, sym::Arc)); + try_alt_rcvr(&mut err, self.tcx.mk_diagnostic_item(rcvr_t, sym::Rc)); } err.emit(); } diff --git a/src/test/ui/error-codes/E0152.rs b/src/test/ui/error-codes/E0152.rs index dcaf920883543..94467b9bddeb0 100644 --- a/src/test/ui/error-codes/E0152.rs +++ b/src/test/ui/error-codes/E0152.rs @@ -1,6 +1,6 @@ #![feature(lang_items)] -#[lang = "arc"] +#[lang = "owned_box"] struct Foo; //~ ERROR E0152 fn main() { diff --git a/src/test/ui/error-codes/E0152.stderr b/src/test/ui/error-codes/E0152.stderr index 29981991ee02b..fbaa276ce1093 100644 --- a/src/test/ui/error-codes/E0152.stderr +++ b/src/test/ui/error-codes/E0152.stderr @@ -1,4 +1,4 @@ -error[E0152]: found duplicate lang item `arc` +error[E0152]: found duplicate lang item `owned_box` --> $DIR/E0152.rs:4:1 | LL | struct Foo; diff --git a/src/test/ui/error-codes/E0718.rs b/src/test/ui/error-codes/E0718.rs index 82ab2d4af1ba8..909cae0ba25a2 100644 --- a/src/test/ui/error-codes/E0718.rs +++ b/src/test/ui/error-codes/E0718.rs @@ -1,7 +1,7 @@ #![feature(lang_items)] -// Arc is expected to be a struct, so this will error. -#[lang = "arc"] //~ ERROR language item must be applied to a struct +// Box is expected to be a struct, so this will error. +#[lang = "owned_box"] //~ ERROR language item must be applied to a struct static X: u32 = 42; fn main() {} diff --git a/src/test/ui/error-codes/E0718.stderr b/src/test/ui/error-codes/E0718.stderr index 412c856ce065a..30378dd167457 100644 --- a/src/test/ui/error-codes/E0718.stderr +++ b/src/test/ui/error-codes/E0718.stderr @@ -1,8 +1,8 @@ -error[E0718]: `arc` language item must be applied to a struct +error[E0718]: `owned_box` language item must be applied to a struct --> $DIR/E0718.rs:4:1 | -LL | #[lang = "arc"] - | ^^^^^^^^^^^^^^^ attribute should be applied to a struct, not a static item +LL | #[lang = "owned_box"] + | ^^^^^^^^^^^^^^^^^^^^^ attribute should be applied to a struct, not a static item error: aborting due to previous error From f9a691faac43f838a16aa872101b0277dbf2e25f Mon Sep 17 00:00:00 2001 From: mark Date: Wed, 8 Apr 2020 12:43:43 -0500 Subject: [PATCH 02/13] de-abuse TyKind::Error: handle empty slices in array patterns --- src/librustc_typeck/check/pat.rs | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/librustc_typeck/check/pat.rs b/src/librustc_typeck/check/pat.rs index b3cace8298a92..5dcb6d8fdc787 100644 --- a/src/librustc_typeck/check/pat.rs +++ b/src/librustc_typeck/check/pat.rs @@ -1353,23 +1353,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { def_bm: BindingMode, ti: TopInfo<'tcx>, ) -> Ty<'tcx> { - let err = self.tcx.types.err; let expected = self.structurally_resolved_type(span, expected); - let (element_ty, slice_ty, inferred) = match expected.kind { + let (element_ty, opt_slice_ty, inferred) = match expected.kind { // An array, so we might have something like `let [a, b, c] = [0, 1, 2];`. ty::Array(element_ty, len) => { let min = before.len() as u64 + after.len() as u64; - let (slice_ty, expected) = + let (opt_slice_ty, expected) = self.check_array_pat_len(span, element_ty, expected, slice, len, min); - (element_ty, slice_ty, expected) + // we can get opt_slice_ty == None in cases that are not an error, e.g. if the + // slice covers 0 elements or if slice is None. + (element_ty, opt_slice_ty, expected) } - ty::Slice(element_ty) => (element_ty, expected, expected), + ty::Slice(element_ty) => (element_ty, Some(expected), expected), // The expected type must be an array or slice, but was neither, so error. _ => { if !expected.references_error() { self.error_expected_array_or_slice(span, expected); } - (err, err, err) + let err = self.tcx.types.err; + (err, None, err) } }; @@ -1377,8 +1379,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { for elt in before { self.check_pat(&elt, element_ty, def_bm, ti); } - // Type check the `slice`, if present, against its expected type. - if let Some(slice) = slice { + // Type check the `slice`, if present, against its expected type, if there is one. + if let (Some(slice), Some(slice_ty)) = (slice, opt_slice_ty) { self.check_pat(&slice, slice_ty, def_bm, ti); } // Type check the elements after `slice`, if present. @@ -1390,9 +1392,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Type check the length of an array pattern. /// - /// Returns both the type of the variable length pattern - /// (or `tcx.err` in case there is none), - /// and the potentially inferred array type. + /// Returns both the type of the variable length pattern (or `None`), and the potentially + /// inferred array type. fn check_array_pat_len( &self, span: Span, @@ -1401,7 +1402,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { slice: Option<&'tcx Pat<'tcx>>, len: &ty::Const<'tcx>, min_len: u64, - ) -> (Ty<'tcx>, Ty<'tcx>) { + ) -> (Option>, Ty<'tcx>) { if let Some(len) = len.try_eval_usize(self.tcx, self.param_env) { // Now we know the length... if slice.is_none() { @@ -1414,7 +1415,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } else if let Some(pat_len) = len.checked_sub(min_len) { // The variable-length pattern was there, // so it has an array type with the remaining elements left as its size... - return (self.tcx.mk_array(element_ty, pat_len), arr_ty); + return (Some(self.tcx.mk_array(element_ty, pat_len)), arr_ty); } else { // ...however, in this case, there were no remaining elements. // That is, the slice pattern requires more than the array type offers. @@ -1425,14 +1426,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // which we can use to infer the length of the array. let updated_arr_ty = self.tcx.mk_array(element_ty, min_len); self.demand_eqtype(span, updated_arr_ty, arr_ty); - return (self.tcx.types.err, updated_arr_ty); + return (None, updated_arr_ty); } else { // We have a variable-length pattern and don't know the array length. // This happens if we have e.g., // `let [a, b, ..] = arr` where `arr: [T; N]` where `const N: usize`. self.error_scrutinee_unfixed_length(span); } - (self.tcx.types.err, arr_ty) + (None, arr_ty) } fn error_scrutinee_inconsistent_length(&self, span: Span, min_len: u64, size: u64) { From e1c838d737ffd30172e394eaf64dd087529a2b6f Mon Sep 17 00:00:00 2001 From: mark Date: Wed, 8 Apr 2020 16:02:31 -0500 Subject: [PATCH 03/13] de-abuse TyKind::Error: ice on missing slice type --- src/librustc_typeck/check/pat.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/librustc_typeck/check/pat.rs b/src/librustc_typeck/check/pat.rs index 5dcb6d8fdc787..37f0efd4064c0 100644 --- a/src/librustc_typeck/check/pat.rs +++ b/src/librustc_typeck/check/pat.rs @@ -1360,8 +1360,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let min = before.len() as u64 + after.len() as u64; let (opt_slice_ty, expected) = self.check_array_pat_len(span, element_ty, expected, slice, len, min); - // we can get opt_slice_ty == None in cases that are not an error, e.g. if the - // slice covers 0 elements or if slice is None. + // opt_slice_ty.is_none() => slice.is_none() + // Note, though, that opt_slice_ty could be Some(error_ty). + assert!(opt_slice_ty.is_some() || slice.is_none()); (element_ty, opt_slice_ty, expected) } ty::Slice(element_ty) => (element_ty, Some(expected), expected), @@ -1371,7 +1372,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.error_expected_array_or_slice(span, expected); } let err = self.tcx.types.err; - (err, None, err) + (err, Some(err), err) } }; @@ -1379,9 +1380,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { for elt in before { self.check_pat(&elt, element_ty, def_bm, ti); } - // Type check the `slice`, if present, against its expected type, if there is one. - if let (Some(slice), Some(slice_ty)) = (slice, opt_slice_ty) { - self.check_pat(&slice, slice_ty, def_bm, ti); + // Type check the `slice`, if present, against its expected type. + if let Some(slice) = slice { + self.check_pat(&slice, opt_slice_ty.unwrap(), def_bm, ti); } // Type check the elements after `slice`, if present. for elt in after { @@ -1393,7 +1394,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Type check the length of an array pattern. /// /// Returns both the type of the variable length pattern (or `None`), and the potentially - /// inferred array type. + /// inferred array type. We should only return `None` for the slice type if `slice.is_none()`. fn check_array_pat_len( &self, span: Span, @@ -1409,9 +1410,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // ...and since there is no variable-length pattern, // we require an exact match between the number of elements // in the array pattern and as provided by the matched type. - if min_len != len { - self.error_scrutinee_inconsistent_length(span, min_len, len); + if min_len == len { + return (None, arr_ty); } + + self.error_scrutinee_inconsistent_length(span, min_len, len); } else if let Some(pat_len) = len.checked_sub(min_len) { // The variable-length pattern was there, // so it has an array type with the remaining elements left as its size... @@ -1433,7 +1436,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // `let [a, b, ..] = arr` where `arr: [T; N]` where `const N: usize`. self.error_scrutinee_unfixed_length(span); } - (None, arr_ty) + + // If we get here, we must have emitted an error. + (Some(self.tcx.types.err), arr_ty) } fn error_scrutinee_inconsistent_length(&self, span: Span, min_len: u64, size: u64) { From 15f8d89901849e55f2577c4a8582c499a094fb88 Mon Sep 17 00:00:00 2001 From: Ana-Maria Mihalache Date: Tue, 31 Mar 2020 18:18:01 +0000 Subject: [PATCH 04/13] rustc_target::abi: add Primitive variant to FieldsShape. --- src/librustc_codegen_llvm/type_of.rs | 4 +-- src/librustc_data_structures/stable_hasher.rs | 6 ++++ src/librustc_middle/ty/layout.rs | 15 +++++++--- src/librustc_mir/interpret/validity.rs | 10 ++++--- src/librustc_mir/interpret/visitor.rs | 5 +++- src/librustc_target/abi/call/mips64.rs | 11 ++++--- src/librustc_target/abi/call/mod.rs | 5 +++- src/librustc_target/abi/call/riscv.rs | 3 ++ src/librustc_target/abi/mod.rs | 29 ++++++++++++++----- src/test/ui/layout/debug.stderr | 4 +-- 10 files changed, 63 insertions(+), 29 deletions(-) diff --git a/src/librustc_codegen_llvm/type_of.rs b/src/librustc_codegen_llvm/type_of.rs index f475ea741c893..77009aca6d32e 100644 --- a/src/librustc_codegen_llvm/type_of.rs +++ b/src/librustc_codegen_llvm/type_of.rs @@ -81,7 +81,7 @@ fn uncached_llvm_type<'a, 'tcx>( }; match layout.fields { - FieldsShape::Union(_) => { + FieldsShape::Primitive | FieldsShape::Union(_) => { let fill = cx.type_padding_filler(layout.size, layout.align.abi); let packed = false; match name { @@ -368,7 +368,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyAndLayout<'tcx> { _ => {} } match self.fields { - FieldsShape::Union(_) => { + FieldsShape::Primitive | FieldsShape::Union(_) => { bug!("TyAndLayout::llvm_field_index({:?}): not applicable", self) } diff --git a/src/librustc_data_structures/stable_hasher.rs b/src/librustc_data_structures/stable_hasher.rs index a98e77cebd88a..97b02eaef35d0 100644 --- a/src/librustc_data_structures/stable_hasher.rs +++ b/src/librustc_data_structures/stable_hasher.rs @@ -210,6 +210,12 @@ impl HashStable for ::std::num::NonZeroU32 { } } +impl HashStable for ::std::num::NonZeroUsize { + fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) { + self.get().hash_stable(ctx, hasher) + } +} + impl HashStable for f32 { fn hash_stable(&self, ctx: &mut CTX, hasher: &mut StableHasher) { let val: u32 = unsafe { ::std::mem::transmute(*self) }; diff --git a/src/librustc_middle/ty/layout.rs b/src/librustc_middle/ty/layout.rs index 1bb338d43ad0a..5a210547f13c0 100644 --- a/src/librustc_middle/ty/layout.rs +++ b/src/librustc_middle/ty/layout.rs @@ -22,6 +22,7 @@ use std::cmp; use std::fmt; use std::iter; use std::mem; +use std::num::NonZeroUsize; use std::ops::Bound; pub trait IntegerExt { @@ -518,7 +519,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { // The never type. ty::Never => tcx.intern_layout(Layout { variants: Variants::Single { index: VariantIdx::new(0) }, - fields: FieldsShape::Union(0), + fields: FieldsShape::Primitive, abi: Abi::Uninhabited, largest_niche: None, align: dl.i8_align, @@ -744,7 +745,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { return Ok(tcx.intern_layout(Layout { variants: Variants::Single { index }, - fields: FieldsShape::Union(variants[index].len()), + fields: FieldsShape::Union( + NonZeroUsize::new(variants[index].len()) + .ok_or(LayoutError::Unknown(ty))?, + ), abi, largest_niche: None, align, @@ -1988,7 +1992,7 @@ where if index == variant_index && // Don't confuse variants of uninhabited enums with the enum itself. // For more details see https://github.com/rust-lang/rust/issues/69763. - this.fields != FieldsShape::Union(0) => + this.fields != FieldsShape::Primitive => { this.layout } @@ -2006,7 +2010,10 @@ where let tcx = cx.tcx(); tcx.intern_layout(Layout { variants: Variants::Single { index: variant_index }, - fields: FieldsShape::Union(fields), + fields: match NonZeroUsize::new(fields) { + Some(fields) => FieldsShape::Union(fields), + None => FieldsShape::Arbitrary { offsets: vec![], memory_index: vec![] }, + }, abi: Abi::Uninhabited, largest_niche: None, align: tcx.data_layout.i8_align, diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 701e394415bbd..ff270b471fbf6 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -6,6 +6,7 @@ use std::convert::TryFrom; use std::fmt::Write; +use std::num::NonZeroUsize; use std::ops::RangeInclusive; use rustc_data_structures::fx::FxHashSet; @@ -642,10 +643,11 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> } #[inline(always)] - fn visit_union(&mut self, op: OpTy<'tcx, M::PointerTag>, fields: usize) -> InterpResult<'tcx> { - // Empty unions are not accepted by rustc. But uninhabited enums - // claim to be unions, so allow them, too. - assert!(op.layout.abi.is_uninhabited() || fields > 0); + fn visit_union( + &mut self, + _op: OpTy<'tcx, M::PointerTag>, + _fields: NonZeroUsize, + ) -> InterpResult<'tcx> { Ok(()) } diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index e03984f4d0be5..00d39452c49d5 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -6,6 +6,8 @@ use rustc_middle::ty; use rustc_middle::ty::layout::TyAndLayout; use rustc_target::abi::{FieldsShape, VariantIdx, Variants}; +use std::num::NonZeroUsize; + use super::{InterpCx, MPlaceTy, Machine, OpTy}; // A thing that we can project into, and that has a layout. @@ -130,7 +132,7 @@ macro_rules! make_value_visitor { } /// Visits the given value as a union. No automatic recursion can happen here. #[inline(always)] - fn visit_union(&mut self, _v: Self::V, _fields: usize) -> InterpResult<'tcx> + fn visit_union(&mut self, _v: Self::V, _fields: NonZeroUsize) -> InterpResult<'tcx> { Ok(()) } @@ -208,6 +210,7 @@ macro_rules! make_value_visitor { // Visit the fields of this value. match v.layout().fields { + FieldsShape::Primitive => {}, FieldsShape::Union(fields) => { self.visit_union(v, fields)?; }, diff --git a/src/librustc_target/abi/call/mips64.rs b/src/librustc_target/abi/call/mips64.rs index 81de630678890..6b8336fb4e44d 100644 --- a/src/librustc_target/abi/call/mips64.rs +++ b/src/librustc_target/abi/call/mips64.rs @@ -77,17 +77,15 @@ where Ty: TyAndLayoutMethods<'a, C> + Copy, C: LayoutOf> + HasDataLayout, { - if !arg.layout.is_aggregate() { - extend_integer_width_mips(arg, 64); - return; - } - - let dl = cx.data_layout(); let size = arg.layout.size; let mut prefix = [None; 8]; let mut prefix_index = 0; match arg.layout.fields { + abi::FieldsShape::Primitive => { + extend_integer_width_mips(arg, 64); + return; + } abi::FieldsShape::Array { .. } => { // Arrays are passed indirectly arg.make_indirect(); @@ -100,6 +98,7 @@ where // Structures are split up into a series of 64-bit integer chunks, but any aligned // doubles not part of another aggregate are passed as floats. let mut last_offset = Size::ZERO; + let dl = cx.data_layout(); for i in 0..arg.layout.fields.count() { let field = arg.layout.field(cx, i); diff --git a/src/librustc_target/abi/call/mod.rs b/src/librustc_target/abi/call/mod.rs index b6bfa70005b5e..0303312d057fe 100644 --- a/src/librustc_target/abi/call/mod.rs +++ b/src/librustc_target/abi/call/mod.rs @@ -308,13 +308,16 @@ impl<'a, Ty> TyAndLayout<'a, Ty> { } Abi::ScalarPair(..) | Abi::Aggregate { .. } => { - // Helper for computing `homogenous_aggregate`, allowing a custom + // Helper for computing `homogeneous_aggregate`, allowing a custom // starting offset (used below for handling variants). let from_fields_at = |layout: Self, start: Size| -> Result<(HomogeneousAggregate, Size), Heterogeneous> { let is_union = match layout.fields { + FieldsShape::Primitive => { + unreachable!("aggregates can't have `FieldsShape::Primitive`") + } FieldsShape::Array { count, .. } => { assert_eq!(start, Size::ZERO); diff --git a/src/librustc_target/abi/call/riscv.rs b/src/librustc_target/abi/call/riscv.rs index 0eb8816e43461..2e10bed3bd451 100644 --- a/src/librustc_target/abi/call/riscv.rs +++ b/src/librustc_target/abi/call/riscv.rs @@ -87,6 +87,9 @@ where }, Abi::Vector { .. } | Abi::Uninhabited => return Err(CannotUseFpConv), Abi::ScalarPair(..) | Abi::Aggregate { .. } => match arg_layout.fields { + FieldsShape::Primitive => { + unreachable!("aggregates can't have `FieldsShape::Primitive`") + } FieldsShape::Union(_) => { if !arg_layout.is_zst() { return Err(CannotUseFpConv); diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 4c25363a6575f..dcf181cb59f4a 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -4,6 +4,7 @@ pub use Primitive::*; use crate::spec::Target; use std::convert::{TryFrom, TryInto}; +use std::num::NonZeroUsize; use std::ops::{Add, AddAssign, Deref, Mul, Range, RangeInclusive, Sub}; use rustc_index::vec::{Idx, IndexVec}; @@ -619,10 +620,11 @@ impl Scalar { /// Describes how the fields of a type are located in memory. #[derive(PartialEq, Eq, Hash, Debug, HashStable_Generic)] pub enum FieldsShape { + /// Scalar primitives and `!`, which never have fields. + Primitive, + /// All fields start at no offset. The `usize` is the field count. - /// - /// In the case of primitives the number of fields is `0`. - Union(usize), + Union(NonZeroUsize), /// Array/vector-like placement, with all fields of identical types. Array { stride: Size, count: u64 }, @@ -660,7 +662,8 @@ pub enum FieldsShape { impl FieldsShape { pub fn count(&self) -> usize { match *self { - FieldsShape::Union(count) => count, + FieldsShape::Primitive => 0, + FieldsShape::Union(count) => count.get(), FieldsShape::Array { count, .. } => { let usize_count = count as usize; assert_eq!(usize_count as u64, count); @@ -672,8 +675,16 @@ impl FieldsShape { pub fn offset(&self, i: usize) -> Size { match *self { + FieldsShape::Primitive => { + unreachable!("FieldsShape::offset: `Primitive`s have no fields") + } FieldsShape::Union(count) => { - assert!(i < count, "tried to access field {} of union with {} fields", i, count); + assert!( + i < count.get(), + "tried to access field {} of union with {} fields", + i, + count + ); Size::ZERO } FieldsShape::Array { stride, count } => { @@ -687,6 +698,9 @@ impl FieldsShape { pub fn memory_index(&self, i: usize) -> usize { match *self { + FieldsShape::Primitive => { + unreachable!("FieldsShape::memory_index: `Primitive`s have no fields") + } FieldsShape::Union(_) | FieldsShape::Array { .. } => i, FieldsShape::Arbitrary { ref memory_index, .. } => { let r = memory_index[i]; @@ -718,7 +732,7 @@ impl FieldsShape { } (0..self.count()).map(move |i| match *self { - FieldsShape::Union(_) | FieldsShape::Array { .. } => i, + FieldsShape::Primitive | FieldsShape::Union(_) | FieldsShape::Array { .. } => i, FieldsShape::Arbitrary { .. } => { if use_small { inverse_small[i] as usize @@ -887,7 +901,6 @@ impl Niche { #[derive(PartialEq, Eq, Hash, Debug, HashStable_Generic)] pub struct Layout { /// Says where the fields are located within the layout. - /// Primitives and uninhabited enums appear as unions without fields. pub fields: FieldsShape, /// Encodes information about multi-variant layouts. @@ -923,7 +936,7 @@ impl Layout { let align = scalar.value.align(cx); Layout { variants: Variants::Single { index: VariantIdx::new(0) }, - fields: FieldsShape::Union(0), + fields: FieldsShape::Primitive, abi: Abi::Scalar(scalar), largest_niche, size, diff --git a/src/test/ui/layout/debug.stderr b/src/test/ui/layout/debug.stderr index 153dec594d32e..cd8ebdffb730b 100644 --- a/src/test/ui/layout/debug.stderr +++ b/src/test/ui/layout/debug.stderr @@ -316,9 +316,7 @@ LL | type Test = Result; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: layout_of(i32) = Layout { - fields: Union( - 0, - ), + fields: Primitive, variants: Single { index: 0, }, From 44daa4521a6952fba012fa80c28845caf1b99ae3 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 9 Apr 2020 13:48:11 +0200 Subject: [PATCH 05/13] Clean up E0511 explanation --- src/librustc_error_codes/error_codes/E0511.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0511.md b/src/librustc_error_codes/error_codes/E0511.md index 4f6644f358291..5351a685eb512 100644 --- a/src/librustc_error_codes/error_codes/E0511.md +++ b/src/librustc_error_codes/error_codes/E0511.md @@ -1,5 +1,6 @@ -Invalid monomorphization of an intrinsic function was used. Erroneous code -example: +Invalid monomorphization of an intrinsic function was used. + +Erroneous code example: ```compile_fail,E0511 #![feature(platform_intrinsics)] From f2e4709f2252103a2461991dba209f8ab4d76aca Mon Sep 17 00:00:00 2001 From: mark Date: Thu, 9 Apr 2020 09:31:52 -0500 Subject: [PATCH 06/13] improve comments --- src/librustc_typeck/check/pat.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_typeck/check/pat.rs b/src/librustc_typeck/check/pat.rs index 37f0efd4064c0..0335aba914460 100644 --- a/src/librustc_typeck/check/pat.rs +++ b/src/librustc_typeck/check/pat.rs @@ -1360,8 +1360,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let min = before.len() as u64 + after.len() as u64; let (opt_slice_ty, expected) = self.check_array_pat_len(span, element_ty, expected, slice, len, min); - // opt_slice_ty.is_none() => slice.is_none() - // Note, though, that opt_slice_ty could be Some(error_ty). + // `opt_slice_ty.is_none()` => `slice.is_none()`. + // Note, though, that opt_slice_ty could be `Some(error_ty)`. assert!(opt_slice_ty.is_some() || slice.is_none()); (element_ty, opt_slice_ty, expected) } @@ -1394,7 +1394,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Type check the length of an array pattern. /// /// Returns both the type of the variable length pattern (or `None`), and the potentially - /// inferred array type. We should only return `None` for the slice type if `slice.is_none()`. + /// inferred array type. We only return `None` for the slice type if `slice.is_none()`. fn check_array_pat_len( &self, span: Span, From 444ad6255b3324a7dd3c4d852808336d39a09ab3 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 29 Mar 2020 14:15:41 -0700 Subject: [PATCH 07/13] Add utility to find locals that don't use `Storage*` annotations --- src/librustc_mir/util/mod.rs | 1 + src/librustc_mir/util/storage.rs | 42 ++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 src/librustc_mir/util/storage.rs diff --git a/src/librustc_mir/util/mod.rs b/src/librustc_mir/util/mod.rs index bb9d168d9193a..3e501193e8dc4 100644 --- a/src/librustc_mir/util/mod.rs +++ b/src/librustc_mir/util/mod.rs @@ -3,6 +3,7 @@ pub mod borrowck_errors; pub mod def_use; pub mod elaborate_drops; pub mod patch; +pub mod storage; mod alignment; pub mod collect_writes; diff --git a/src/librustc_mir/util/storage.rs b/src/librustc_mir/util/storage.rs new file mode 100644 index 0000000000000..2ce9bed794d96 --- /dev/null +++ b/src/librustc_mir/util/storage.rs @@ -0,0 +1,42 @@ +use rustc_index::bit_set::BitSet; +use rustc_middle::mir::{self, Local}; + +/// The set of locals in a MIR body that do not have `StorageLive`/`StorageDead` annotations. +/// +/// These locals have fixed storage for the duration of the body. +// +// FIXME: Currently, we need to traverse the entire MIR to compute this. We should instead store it +// as a field in the `LocalDecl` for each `Local`. +#[derive(Debug, Clone)] +pub struct AlwaysLiveLocals(BitSet); + +impl AlwaysLiveLocals { + pub fn new(body: &mir::Body<'tcx>) -> Self { + let mut locals = BitSet::new_filled(body.local_decls.len()); + + // FIXME: Use a visitor for this when `visit_body` can take a plain `Body`. + for block in body.basic_blocks().iter() { + for stmt in &block.statements { + if let mir::StatementKind::StorageLive(l) | mir::StatementKind::StorageDead(l) = + stmt.kind + { + locals.remove(l); + } + } + } + + AlwaysLiveLocals(locals) + } + + pub fn into_inner(self) -> BitSet { + self.0 + } +} + +impl std::ops::Deref for AlwaysLiveLocals { + type Target = BitSet; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} From fcd1f5bc0a1090f295667dc0fdb7f238672b0302 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 29 Mar 2020 14:16:29 -0700 Subject: [PATCH 08/13] Make `MaybeStorageLive` correct for all kinds of MIR bodies Before, it ignored the first argument and marked all variables without `Storage*` annotations as dead. --- .../dataflow/impls/storage_liveness.rs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs index 3dfcfe16fb514..cc63f5f39d53c 100644 --- a/src/librustc_mir/dataflow/impls/storage_liveness.rs +++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs @@ -2,12 +2,21 @@ pub use super::*; use crate::dataflow::BottomValue; use crate::dataflow::{self, GenKill, Results, ResultsRefCursor}; +use crate::util::storage::AlwaysLiveLocals; use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::*; use std::cell::RefCell; -#[derive(Copy, Clone)] -pub struct MaybeStorageLive; +#[derive(Clone)] +pub struct MaybeStorageLive { + always_live_locals: AlwaysLiveLocals, +} + +impl MaybeStorageLive { + pub fn new(always_live_locals: AlwaysLiveLocals) -> Self { + MaybeStorageLive { always_live_locals } + } +} impl dataflow::AnalysisDomain<'tcx> for MaybeStorageLive { type Idx = Local; @@ -19,9 +28,12 @@ impl dataflow::AnalysisDomain<'tcx> for MaybeStorageLive { } fn initialize_start_block(&self, body: &mir::Body<'tcx>, on_entry: &mut BitSet) { - // The resume argument is live on function entry (we don't care about - // the `self` argument) - for arg in body.args_iter().skip(1) { + assert_eq!(body.local_decls.len(), self.always_live_locals.domain_size()); + for local in self.always_live_locals.iter() { + on_entry.insert(local); + } + + for arg in body.args_iter() { on_entry.insert(arg); } } From 335fd6b456ac75f5a5bd34d71855dc892bdd8e2f Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 29 Mar 2020 14:17:27 -0700 Subject: [PATCH 09/13] Use new utility in `eval_context` --- src/librustc_mir/interpret/eval_context.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 72d20644fe8b2..e0b5f634bf3df 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -24,6 +24,7 @@ use super::{ Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy, ScalarMaybeUndef, StackPopJump, }; +use crate::util::storage::AlwaysLiveLocals; pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> { /// Stores the `Machine` instance. @@ -610,17 +611,17 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Now mark those locals as dead that we do not want to initialize match self.tcx.def_kind(instance.def_id()) { // statics and constants don't have `Storage*` statements, no need to look for them + // + // FIXME: The above is likely untrue. See + // . Is it + // okay to ignore `StorageDead`/`StorageLive` annotations during CTFE? Some(DefKind::Static) | Some(DefKind::Const) | Some(DefKind::AssocConst) => {} _ => { - for block in body.basic_blocks() { - for stmt in block.statements.iter() { - use rustc_middle::mir::StatementKind::{StorageDead, StorageLive}; - match stmt.kind { - StorageLive(local) | StorageDead(local) => { - locals[local].value = LocalValue::Dead; - } - _ => {} - } + // Mark locals that use `Storage*` annotations as dead on function entry. + let always_live = AlwaysLiveLocals::new(self.body()); + for local in locals.indices() { + if !always_live.contains(local) { + locals[local].value = LocalValue::Dead; } } } From 02c65e1e11ecdbed6e0573b271bf5e865e1ad995 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 29 Mar 2020 14:17:53 -0700 Subject: [PATCH 10/13] Use new utility in `transform/generator.rs` --- src/librustc_mir/transform/generator.rs | 74 +++++++++++++------------ 1 file changed, 40 insertions(+), 34 deletions(-) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 53eec1f6dc3de..25e879b01e298 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -56,12 +56,13 @@ use crate::transform::simplify; use crate::transform::{MirPass, MirSource}; use crate::util::dump_mir; use crate::util::liveness; +use crate::util::storage; use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_index::bit_set::{BitMatrix, BitSet}; use rustc_index::vec::{Idx, IndexVec}; -use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor}; +use rustc_middle::mir::visit::{MutVisitor, PlaceContext}; use rustc_middle::mir::*; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::GeneratorSubsts; @@ -222,6 +223,9 @@ struct TransformVisitor<'tcx> { // A list of suspension points, generated during the transform suspension_points: Vec>, + // The set of locals that have no `StorageLive`/`StorageDead` annotations. + always_live_locals: storage::AlwaysLiveLocals, + // The original RETURN_PLACE local new_ret_local: Local, } @@ -416,19 +420,6 @@ fn replace_local<'tcx>( new_local } -struct StorageIgnored(liveness::LiveVarSet); - -impl<'tcx> Visitor<'tcx> for StorageIgnored { - fn visit_statement(&mut self, statement: &Statement<'tcx>, _location: Location) { - match statement.kind { - StatementKind::StorageLive(l) | StatementKind::StorageDead(l) => { - self.0.remove(l); - } - _ => (), - } - } -} - struct LivenessInfo { /// Which locals are live across any suspension point. /// @@ -454,6 +445,7 @@ fn locals_live_across_suspend_points( tcx: TyCtxt<'tcx>, body: ReadOnlyBodyAndCache<'_, 'tcx>, source: MirSource<'tcx>, + always_live_locals: &storage::AlwaysLiveLocals, movable: bool, ) -> LivenessInfo { let def_id = source.def_id(); @@ -461,16 +453,11 @@ fn locals_live_across_suspend_points( // Calculate when MIR locals have live storage. This gives us an upper bound of their // lifetimes. - let mut storage_live = MaybeStorageLive + let mut storage_live = MaybeStorageLive::new(always_live_locals.clone()) .into_engine(tcx, body_ref, def_id) .iterate_to_fixpoint() .into_results_cursor(body_ref); - // Find the MIR locals which do not use StorageLive/StorageDead statements. - // The storage of these locals are always live. - let mut ignored = StorageIgnored(BitSet::new_filled(body.local_decls.len())); - ignored.visit_body(&body); - // Calculate the MIR locals which have been previously // borrowed (even if they are still active). let borrowed_locals_results = @@ -515,11 +502,13 @@ fn locals_live_across_suspend_points( } storage_live.seek_before(loc); - let storage_liveness = storage_live.get(); + let mut storage_liveness = storage_live.get().clone(); + + storage_liveness.remove(SELF_ARG); // Store the storage liveness for later use so we can restore the state // after a suspension point - storage_liveness_map.insert(block, storage_liveness.clone()); + storage_liveness_map.insert(block, storage_liveness); requires_storage_cursor.seek_before(loc); let storage_required = requires_storage_cursor.get().clone(); @@ -551,8 +540,12 @@ fn locals_live_across_suspend_points( .map(|live_here| renumber_bitset(&live_here, &live_locals)) .collect(); - let storage_conflicts = - compute_storage_conflicts(body_ref, &live_locals, &ignored, requires_storage_results); + let storage_conflicts = compute_storage_conflicts( + body_ref, + &live_locals, + always_live_locals.clone(), + requires_storage_results, + ); LivenessInfo { live_locals, @@ -590,18 +583,18 @@ fn renumber_bitset( fn compute_storage_conflicts( body: &'mir Body<'tcx>, stored_locals: &liveness::LiveVarSet, - ignored: &StorageIgnored, + always_live_locals: storage::AlwaysLiveLocals, requires_storage: dataflow::Results<'tcx, MaybeRequiresStorage<'mir, 'tcx>>, ) -> BitMatrix { - assert_eq!(body.local_decls.len(), ignored.0.domain_size()); assert_eq!(body.local_decls.len(), stored_locals.domain_size()); + debug!("compute_storage_conflicts({:?})", body.span); - debug!("ignored = {:?}", ignored.0); + debug!("always_live = {:?}", always_live_locals); - // Storage ignored locals are not eligible for overlap, since their storage - // is always live. - let mut ineligible_locals = ignored.0.clone(); - ineligible_locals.intersect(&stored_locals); + // Locals that are always live or ones that need to be stored across + // suspension points are not eligible for overlap. + let mut ineligible_locals = always_live_locals.into_inner(); + ineligible_locals.intersect(stored_locals); // Compute the storage conflicts for all eligible locals. let mut visitor = StorageConflictVisitor { @@ -697,6 +690,7 @@ fn compute_layout<'tcx>( source: MirSource<'tcx>, upvars: &Vec>, interior: Ty<'tcx>, + always_live_locals: &storage::AlwaysLiveLocals, movable: bool, body: &mut BodyAndCache<'tcx>, ) -> ( @@ -710,7 +704,13 @@ fn compute_layout<'tcx>( live_locals_at_suspension_points, storage_conflicts, storage_liveness, - } = locals_live_across_suspend_points(tcx, read_only!(body), source, movable); + } = locals_live_across_suspend_points( + tcx, + read_only!(body), + source, + always_live_locals, + movable, + ); // Erase regions from the types passed in from typeck so we can compare them with // MIR types @@ -1180,7 +1180,10 @@ fn create_cases<'tcx>( } let l = Local::new(i); - if point.storage_liveness.contains(l) && !transform.remap.contains_key(&l) { + let needs_storage_live = point.storage_liveness.contains(l) + && !transform.remap.contains_key(&l) + && !transform.always_live_locals.contains(l); + if needs_storage_live { statements .push(Statement { source_info, kind: StatementKind::StorageLive(l) }); } @@ -1276,11 +1279,13 @@ impl<'tcx> MirPass<'tcx> for StateTransform { }, ); + let always_live_locals = storage::AlwaysLiveLocals::new(&body); + // Extract locals which are live across suspension point into `layout` // `remap` gives a mapping from local indices onto generator struct indices // `storage_liveness` tells us which locals have live storage at suspension points let (remap, layout, storage_liveness) = - compute_layout(tcx, source, &upvars, interior, movable, body); + compute_layout(tcx, source, &upvars, interior, &always_live_locals, movable, body); let can_return = can_return(tcx, body); @@ -1294,6 +1299,7 @@ impl<'tcx> MirPass<'tcx> for StateTransform { state_substs, remap, storage_liveness, + always_live_locals, suspension_points: Vec::new(), new_ret_local, discr_ty, From 715486067e1b5a51e8b5b47a21ea4ce4159ad791 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 9 Apr 2020 13:01:59 -0700 Subject: [PATCH 11/13] Explain why we remove `self` from storage live locals --- src/librustc_mir/transform/generator.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 25e879b01e298..d25fd8ebb0c2b 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -504,6 +504,7 @@ fn locals_live_across_suspend_points( storage_live.seek_before(loc); let mut storage_liveness = storage_live.get().clone(); + // Later passes handle the generator's `self` argument separately. storage_liveness.remove(SELF_ARG); // Store the storage liveness for later use so we can restore the state From 209087b8fa26f28c847a5a98afa0bc52ed4f769b Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 9 Apr 2020 13:02:23 -0700 Subject: [PATCH 12/13] Use `Visitor` for `AlwaysLiveLocals` --- src/librustc_mir/util/storage.rs | 33 ++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/librustc_mir/util/storage.rs b/src/librustc_mir/util/storage.rs index 2ce9bed794d96..0b7b1c29537f5 100644 --- a/src/librustc_mir/util/storage.rs +++ b/src/librustc_mir/util/storage.rs @@ -1,5 +1,6 @@ use rustc_index::bit_set::BitSet; -use rustc_middle::mir::{self, Local}; +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::{self, Local, Location}; /// The set of locals in a MIR body that do not have `StorageLive`/`StorageDead` annotations. /// @@ -12,20 +13,12 @@ pub struct AlwaysLiveLocals(BitSet); impl AlwaysLiveLocals { pub fn new(body: &mir::Body<'tcx>) -> Self { - let mut locals = BitSet::new_filled(body.local_decls.len()); - - // FIXME: Use a visitor for this when `visit_body` can take a plain `Body`. - for block in body.basic_blocks().iter() { - for stmt in &block.statements { - if let mir::StatementKind::StorageLive(l) | mir::StatementKind::StorageDead(l) = - stmt.kind - { - locals.remove(l); - } - } - } + let mut ret = AlwaysLiveLocals(BitSet::new_filled(body.local_decls.len())); + + let mut vis = StorageAnnotationVisitor(&mut ret); + vis.visit_body(body); - AlwaysLiveLocals(locals) + ret } pub fn into_inner(self) -> BitSet { @@ -40,3 +33,15 @@ impl std::ops::Deref for AlwaysLiveLocals { &self.0 } } + +/// Removes locals that have `Storage*` annotations from `AlwaysLiveLocals`. +struct StorageAnnotationVisitor<'a>(&'a mut AlwaysLiveLocals); + +impl Visitor<'tcx> for StorageAnnotationVisitor<'_> { + fn visit_statement(&mut self, statement: &mir::Statement<'tcx>, _location: Location) { + use mir::StatementKind::{StorageDead, StorageLive}; + if let StorageLive(l) | StorageDead(l) = statement.kind { + (self.0).0.remove(l); + } + } +} From 1761a65ebadd3c4125fa80c0f5570b24429bafff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 9 Apr 2020 23:02:13 +0200 Subject: [PATCH 13/13] mark a temporary hack as such --- src/librustc_session/config.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_session/config.rs b/src/librustc_session/config.rs index 4e2423bc3b114..aaf30c583e263 100644 --- a/src/librustc_session/config.rs +++ b/src/librustc_session/config.rs @@ -1019,7 +1019,9 @@ pub fn get_cmd_lint_options( for &level in &[lint::Allow, lint::Warn, lint::Deny, lint::Forbid] { for (passed_arg_pos, lint_name) in matches.opt_strs_pos(level.as_str()) { let arg_pos = if let lint::Forbid = level { - // forbid is always specified last, so it can't be overridden + // HACK: forbid is always specified last, so it can't be overridden. + // FIXME: remove this once is + // fixed and `forbid` works as expected. usize::max_value() } else { passed_arg_pos