From 91cf0e741186a9fa3bf31b07a65dc89324c10296 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Jan 2020 20:24:44 +0000 Subject: [PATCH 01/32] Don't requery the param_env of a union Union fields have the ParamEnv of the union. --- src/librustc_typeck/check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 0a917a1853eb5..8d2cfd9a335d1 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1545,11 +1545,11 @@ fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: DefId) -> bool { if let ty::Adt(def, substs) = item_type.kind { assert!(def.is_union()); let fields = &def.non_enum_variant().fields; + let param_env = tcx.param_env(item_def_id); for field in fields { let field_ty = field.ty(tcx, substs); // We are currently checking the type this field came from, so it must be local. let field_span = tcx.hir().span_if_local(field.did).unwrap(); - let param_env = tcx.param_env(field.did); if field_ty.needs_drop(tcx, param_env) { struct_span_err!( tcx.sess, From 570c1613c1225d5777af5603dcf526da9cf57e19 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Jan 2020 20:25:39 +0000 Subject: [PATCH 02/32] Remove unnecessary features in rustc_ty --- src/librustc_ty/lib.rs | 2 -- src/librustc_ty/ty.rs | 6 +++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc_ty/lib.rs b/src/librustc_ty/lib.rs index e5ec98743e0ae..e970faef02f28 100644 --- a/src/librustc_ty/lib.rs +++ b/src/librustc_ty/lib.rs @@ -6,9 +6,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/")] #![feature(bool_to_option)] -#![feature(in_band_lifetimes)] #![feature(nll)] -#![cfg_attr(bootstrap, feature(slice_patterns))] #![recursion_limit = "256"] #[macro_use] diff --git a/src/librustc_ty/ty.rs b/src/librustc_ty/ty.rs index 8f882be1a090e..a131b11b07ba3 100644 --- a/src/librustc_ty/ty.rs +++ b/src/librustc_ty/ty.rs @@ -9,7 +9,11 @@ use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc_span::symbol::Symbol; use rustc_span::Span; -fn sized_constraint_for_ty(tcx: TyCtxt<'tcx>, adtdef: &ty::AdtDef, ty: Ty<'tcx>) -> Vec> { +fn sized_constraint_for_ty<'tcx>( + tcx: TyCtxt<'tcx>, + adtdef: &ty::AdtDef, + ty: Ty<'tcx>, +) -> Vec> { use ty::TyKind::*; let result = match ty.kind { From 39733223fc817efba52a4204dd697192bf5da185 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Jan 2020 20:26:36 +0000 Subject: [PATCH 03/32] Add IS_MANUALLY_DROP to AdtFlags --- src/librustc/ty/mod.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index f417b907a3811..2b272d7fe08af 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1792,19 +1792,22 @@ bitflags! { const IS_STRUCT = 1 << 2; /// Indicates whether the ADT is a struct and has a constructor. const HAS_CTOR = 1 << 3; - /// Indicates whether the type is a `PhantomData`. + /// Indicates whether the type is `PhantomData`. const IS_PHANTOM_DATA = 1 << 4; /// Indicates whether the type has a `#[fundamental]` attribute. const IS_FUNDAMENTAL = 1 << 5; - /// Indicates whether the type is a `Box`. + /// Indicates whether the type is `Box`. 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 << 7; + const IS_ARC = 1 << 8; /// Indicates whether the type is an `Rc`. - const IS_RC = 1 << 8; + 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 << 9; + const IS_VARIANT_LIST_NON_EXHAUSTIVE = 1 << 10; } } @@ -2180,6 +2183,9 @@ impl<'tcx> AdtDef { if Some(did) == tcx.lang_items().owned_box() { flags |= AdtFlags::IS_BOX; } + if Some(did) == tcx.lang_items().manually_drop() { + flags |= AdtFlags::IS_MANUALLY_DROP; + } if Some(did) == tcx.lang_items().arc() { flags |= AdtFlags::IS_ARC; } @@ -2280,6 +2286,12 @@ impl<'tcx> AdtDef { self.flags.contains(AdtFlags::IS_BOX) } + /// Returns `true` if this is ManuallyDrop. + #[inline] + pub fn is_manually_drop(&self) -> bool { + self.flags.contains(AdtFlags::IS_MANUALLY_DROP) + } + /// Returns `true` if this type has a destructor. pub fn has_dtor(&self, tcx: TyCtxt<'tcx>) -> bool { self.destructor(tcx).is_some() From d1965216a34dc2831cf44d2e15ad9d78403d10cc Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Jan 2020 20:28:16 +0000 Subject: [PATCH 04/32] Improve needs_drop query * Handle cycles in `needs_drop` correctly * Normalize types when computing `needs_drop` * Move queries from rustc to rustc_ty --- src/librustc/query/mod.rs | 17 +- src/librustc/traits/misc.rs | 132 -------------- src/librustc/traits/mod.rs | 1 - src/librustc/ty/query/mod.rs | 2 +- src/librustc/ty/query/values.rs | 7 - src/librustc/ty/util.rs | 81 ++++++++- src/librustc_ty/common_traits.rs | 40 ++++ src/librustc_ty/lib.rs | 4 + src/librustc_ty/needs_drop.rs | 171 ++++++++++++++++++ .../ui/type-alias-impl-trait/issue-65918.rs | 2 + 10 files changed, 304 insertions(+), 153 deletions(-) create mode 100644 src/librustc_ty/common_traits.rs create mode 100644 src/librustc_ty/needs_drop.rs diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index 37d5e23535b81..c705956f4540a 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -651,26 +651,27 @@ rustc_queries! { no_force desc { "computing whether `{}` is `Copy`", env.value } } + /// Query backing `TyS::is_sized`. query is_sized_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { no_force desc { "computing whether `{}` is `Sized`", env.value } } + /// Query backing `TyS::is_freeze`. query is_freeze_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { no_force desc { "computing whether `{}` is freeze", env.value } } - - // The cycle error here should be reported as an error by `check_representable`. - // We consider the type as not needing drop in the meanwhile to avoid - // further errors (done in impl Value for NeedsDrop). - // Use `cycle_delay_bug` to delay the cycle error here to be emitted later - // in case we accidentally otherwise don't emit an error. - query needs_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> NeedsDrop { - cycle_delay_bug + /// Query backing `TyS::needs_drop`. + query needs_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { no_force desc { "computing whether `{}` needs drop", env.value } } + /// A list of types where the ADT requires drop if and only if any of + /// those types require drop. If the ADT is known to always need drop + /// then `Err(AlwaysRequiresDrop)` is returned. + query adt_drop_tys(_: DefId) -> Result<&'tcx ty::List>, AlwaysRequiresDrop> {} + query layout_raw( env: ty::ParamEnvAnd<'tcx, Ty<'tcx>> ) -> Result<&'tcx ty::layout::LayoutDetails, ty::layout::LayoutError<'tcx>> { diff --git a/src/librustc/traits/misc.rs b/src/librustc/traits/misc.rs index 08c3a77bf3aca..3fd0d12c626aa 100644 --- a/src/librustc/traits/misc.rs +++ b/src/librustc/traits/misc.rs @@ -1,12 +1,9 @@ //! Miscellaneous type-system utilities that are too small to deserve their own modules. -use crate::middle::lang_items; use crate::traits::{self, ObligationCause}; -use crate::ty::util::NeedsDrop; use crate::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc_hir as hir; -use rustc_span::DUMMY_SP; #[derive(Clone)] pub enum CopyImplementationError<'tcx> { @@ -71,132 +68,3 @@ pub fn can_type_implement_copy( Ok(()) }) } - -fn is_copy_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { - is_item_raw(tcx, query, lang_items::CopyTraitLangItem) -} - -fn is_sized_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { - is_item_raw(tcx, query, lang_items::SizedTraitLangItem) -} - -fn is_freeze_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { - is_item_raw(tcx, query, lang_items::FreezeTraitLangItem) -} - -fn is_item_raw<'tcx>( - tcx: TyCtxt<'tcx>, - query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>, - item: lang_items::LangItem, -) -> bool { - let (param_env, ty) = query.into_parts(); - let trait_def_id = tcx.require_lang_item(item, None); - tcx.infer_ctxt().enter(|infcx| { - traits::type_known_to_meet_bound_modulo_regions( - &infcx, - param_env, - ty, - trait_def_id, - DUMMY_SP, - ) - }) -} - -fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> NeedsDrop { - let (param_env, ty) = query.into_parts(); - - let needs_drop = |ty: Ty<'tcx>| -> bool { tcx.needs_drop_raw(param_env.and(ty)).0 }; - - assert!(!ty.needs_infer()); - - NeedsDrop(match ty.kind { - // Fast-path for primitive types - ty::Infer(ty::FreshIntTy(_)) - | ty::Infer(ty::FreshFloatTy(_)) - | ty::Bool - | ty::Int(_) - | ty::Uint(_) - | ty::Float(_) - | ty::Never - | ty::FnDef(..) - | ty::FnPtr(_) - | ty::Char - | ty::GeneratorWitness(..) - | ty::RawPtr(_) - | ty::Ref(..) - | ty::Str => false, - - // Foreign types can never have destructors - ty::Foreign(..) => false, - - // `ManuallyDrop` doesn't have a destructor regardless of field types. - ty::Adt(def, _) if Some(def.did) == tcx.lang_items().manually_drop() => false, - - // Issue #22536: We first query `is_copy_modulo_regions`. It sees a - // normalized version of the type, and therefore will definitely - // know whether the type implements Copy (and thus needs no - // cleanup/drop/zeroing) ... - _ if ty.is_copy_modulo_regions(tcx, param_env, DUMMY_SP) => false, - - // ... (issue #22536 continued) but as an optimization, still use - // prior logic of asking for the structural "may drop". - - // FIXME(#22815): Note that this is a conservative heuristic; - // it may report that the type "may drop" when actual type does - // not actually have a destructor associated with it. But since - // the type absolutely did not have the `Copy` bound attached - // (see above), it is sound to treat it as having a destructor. - - // User destructors are the only way to have concrete drop types. - ty::Adt(def, _) if def.has_dtor(tcx) => true, - - // Can refer to a type which may drop. - // FIXME(eddyb) check this against a ParamEnv. - ty::Dynamic(..) - | ty::Projection(..) - | ty::Param(_) - | ty::Bound(..) - | ty::Placeholder(..) - | ty::Opaque(..) - | ty::Infer(_) - | ty::Error => true, - - ty::UnnormalizedProjection(..) => bug!("only used with chalk-engine"), - - // Zero-length arrays never contain anything to drop. - ty::Array(_, len) if len.try_eval_usize(tcx, param_env) == Some(0) => false, - - // Structural recursion. - ty::Array(ty, _) | ty::Slice(ty) => needs_drop(ty), - - ty::Closure(def_id, ref substs) => { - substs.as_closure().upvar_tys(def_id, tcx).any(needs_drop) - } - - // Pessimistically assume that all generators will require destructors - // as we don't know if a destructor is a noop or not until after the MIR - // state transformation pass - ty::Generator(..) => true, - - ty::Tuple(..) => ty.tuple_fields().any(needs_drop), - - // unions don't have destructors because of the child types, - // only if they manually implement `Drop` (handled above). - ty::Adt(def, _) if def.is_union() => false, - - ty::Adt(def, substs) => def - .variants - .iter() - .any(|variant| variant.fields.iter().any(|field| needs_drop(field.ty(tcx, substs)))), - }) -} - -pub fn provide(providers: &mut ty::query::Providers<'_>) { - *providers = ty::query::Providers { - is_copy_raw, - is_sized_raw, - is_freeze_raw, - needs_drop_raw, - ..*providers - }; -} diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index daaba95cf6b13..f6c939c1ce5a2 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -1255,7 +1255,6 @@ impl<'tcx> TraitObligation<'tcx> { } pub fn provide(providers: &mut ty::query::Providers<'_>) { - misc::provide(providers); *providers = ty::query::Providers { is_object_safe: object_safety::is_object_safe_provider, specialization_graph_of: specialize::specialization_graph_provider, diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 973cd81014616..4126b161a821b 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -33,7 +33,7 @@ use crate::traits::Clauses; use crate::traits::{self, Vtable}; use crate::ty::steal::Steal; use crate::ty::subst::SubstsRef; -use crate::ty::util::NeedsDrop; +use crate::ty::util::AlwaysRequiresDrop; use crate::ty::{self, AdtSizedConstraint, CrateInherentImpls, ParamEnvAnd, Ty, TyCtxt}; use crate::util::common::ErrorReported; use rustc_data_structures::fingerprint::Fingerprint; diff --git a/src/librustc/ty/query/values.rs b/src/librustc/ty/query/values.rs index 900a91fe5cf59..b01d15c29b2db 100644 --- a/src/librustc/ty/query/values.rs +++ b/src/librustc/ty/query/values.rs @@ -1,4 +1,3 @@ -use crate::ty::util::NeedsDrop; use crate::ty::{self, AdtSizedConstraint, Ty, TyCtxt}; use rustc_span::symbol::Symbol; @@ -26,12 +25,6 @@ impl<'tcx> Value<'tcx> for ty::SymbolName { } } -impl<'tcx> Value<'tcx> for NeedsDrop { - fn from_cycle_error(_: TyCtxt<'tcx>) -> Self { - NeedsDrop(false) - } -} - impl<'tcx> Value<'tcx> for AdtSizedConstraint<'tcx> { fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self { AdtSizedConstraint(tcx.intern_type_list(&[tcx.types.err])) diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 4dfff85d53147..eaa7a43b09174 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -18,6 +18,7 @@ use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_macros::HashStable; use rustc_span::Span; +use smallvec::SmallVec; use std::{cmp, fmt}; use syntax::ast; @@ -724,7 +725,23 @@ impl<'tcx> ty::TyS<'tcx> { /// Note that this method is used to check eligible types in unions. #[inline] pub fn needs_drop(&'tcx self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool { - tcx.needs_drop_raw(param_env.and(self)).0 + // Avoid querying in simple cases. + match needs_drop_components(self) { + Err(AlwaysRequiresDrop) => true, + Ok(components) => { + let query_ty = match *components { + [] => return false, + // If we've got a single component, call the query with that + // to increase the chance that we hit the query cache. + [component_ty] => component_ty, + _ => self, + }; + // This doesn't depend on regions, so try to minimize distinct. + // query keys used. + let erased = tcx.normalize_erasing_regions(param_env, query_ty); + tcx.needs_drop_raw(param_env.and(erased)) + } + } } pub fn same_type(a: Ty<'tcx>, b: Ty<'tcx>) -> bool { @@ -923,9 +940,6 @@ impl<'tcx> ty::TyS<'tcx> { } } -#[derive(Clone, HashStable)] -pub struct NeedsDrop(pub bool); - pub enum ExplicitSelf<'tcx> { ByValue, ByReference(ty::Region<'tcx>, hir::Mutability), @@ -974,3 +988,62 @@ impl<'tcx> ExplicitSelf<'tcx> { } } } + +/// Returns a list of types such that the given type needs drop if and only if +/// *any* of the returned types need drop. Returns `Err(AlwaysRequiresDrop)` if +/// this type always needs drop. +pub fn needs_drop_components(ty: Ty<'tcx>) -> Result; 4]>, AlwaysRequiresDrop> { + match ty.kind { + ty::Infer(ty::FreshIntTy(_)) + | ty::Infer(ty::FreshFloatTy(_)) + | ty::Bool + | ty::Int(_) + | ty::Uint(_) + | ty::Float(_) + | ty::Never + | ty::FnDef(..) + | ty::FnPtr(_) + | ty::Char + | ty::GeneratorWitness(..) + | ty::RawPtr(_) + | ty::Ref(..) + | ty::Str => Ok(SmallVec::new()), + + // Foreign types can never have destructors + ty::Foreign(..) => Ok(SmallVec::new()), + + // Pessimistically assume that all generators will require destructors + // as we don't know if a destructor is a noop or not until after the MIR + // state transformation pass + ty::Generator(..) | ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop), + + ty::Slice(ty) => needs_drop_components(ty), + ty::Array(elem_ty, ..) => { + match needs_drop_components(elem_ty) { + Ok(v) if v.is_empty() => Ok(v), + // Arrays of size zero don't need drop, even if their element + // type does. + _ => Ok(smallvec![ty]), + } + } + // If any field needs drop, then the whole tuple does. + ty::Tuple(..) => ty.tuple_fields().try_fold(SmallVec::new(), |mut acc, elem| { + acc.extend(needs_drop_components(elem)?); + Ok(acc) + }), + + // These require checking for `Copy` bounds or `Adt` destructors. + ty::Adt(..) + | ty::Projection(..) + | ty::UnnormalizedProjection(..) + | ty::Param(_) + | ty::Bound(..) + | ty::Placeholder(..) + | ty::Opaque(..) + | ty::Infer(_) + | ty::Closure(..) => Ok(smallvec![ty]), + } +} + +#[derive(Copy, Clone, Debug, HashStable)] +pub struct AlwaysRequiresDrop; diff --git a/src/librustc_ty/common_traits.rs b/src/librustc_ty/common_traits.rs new file mode 100644 index 0000000000000..9fe8a19311fb6 --- /dev/null +++ b/src/librustc_ty/common_traits.rs @@ -0,0 +1,40 @@ +//! Queries for checking whether a type implements one of a few common traits. + +use rustc::middle::lang_items; +use rustc::traits; +use rustc::ty::{self, Ty, TyCtxt}; +use rustc_span::DUMMY_SP; + +fn is_copy_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { + is_item_raw(tcx, query, lang_items::CopyTraitLangItem) +} + +fn is_sized_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { + is_item_raw(tcx, query, lang_items::SizedTraitLangItem) +} + +fn is_freeze_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { + is_item_raw(tcx, query, lang_items::FreezeTraitLangItem) +} + +fn is_item_raw<'tcx>( + tcx: TyCtxt<'tcx>, + query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>, + item: lang_items::LangItem, +) -> bool { + let (param_env, ty) = query.into_parts(); + let trait_def_id = tcx.require_lang_item(item, None); + tcx.infer_ctxt().enter(|infcx| { + traits::type_known_to_meet_bound_modulo_regions( + &infcx, + param_env, + ty, + trait_def_id, + DUMMY_SP, + ) + }) +} + +pub(crate) fn provide(providers: &mut ty::query::Providers<'_>) { + *providers = ty::query::Providers { is_copy_raw, is_sized_raw, is_freeze_raw, ..*providers }; +} diff --git a/src/librustc_ty/lib.rs b/src/librustc_ty/lib.rs index e970faef02f28..7eef19b94e401 100644 --- a/src/librustc_ty/lib.rs +++ b/src/librustc_ty/lib.rs @@ -16,8 +16,12 @@ extern crate log; use rustc::ty::query::Providers; +mod common_traits; +mod needs_drop; mod ty; pub fn provide(providers: &mut Providers<'_>) { + common_traits::provide(providers); + needs_drop::provide(providers); ty::provide(providers); } diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs new file mode 100644 index 0000000000000..1a65acb1f984b --- /dev/null +++ b/src/librustc_ty/needs_drop.rs @@ -0,0 +1,171 @@ +//! Check whether a type has (potentially) non-trivial drop glue. + +use rustc::ty::subst::Subst; +use rustc::ty::util::{needs_drop_components, AlwaysRequiresDrop}; +use rustc::ty::{self, Ty, TyCtxt}; +use rustc_data_structures::fx::FxHashSet; +use rustc_hir::def_id::DefId; +use rustc_span::DUMMY_SP; + +type NeedsDropResult = Result; + +fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { + let adt_fields = + move |adt_def: &ty::AdtDef| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter().copied()); + // If we don't know a type doesn't need drop, say it's a type parameter + // without a `Copy` bound, then we conservatively return that it needs + // drop. + let res = NeedsDropTypes::new(tcx, query.param_env, query.value, adt_fields).next().is_some(); + debug!("needs_drop_raw({:?}) = {:?}", query, res); + res +} + +struct NeedsDropTypes<'tcx, F> { + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + query_ty: Ty<'tcx>, + seen_tys: FxHashSet>, + /// A stack of types left to process. Each round, we pop something from the + /// stack and check if it needs drop. If the result depends on whether some + /// other types need drop we push them onto the stack. + unchecked_tys: Vec<(Ty<'tcx>, usize)>, + recursion_limit: usize, + adt_components: F, +} + +impl<'tcx, F> NeedsDropTypes<'tcx, F> { + fn new( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + ty: Ty<'tcx>, + adt_components: F, + ) -> Self { + let mut seen_tys = FxHashSet::default(); + seen_tys.insert(ty); + let recursion_limit = *tcx.sess.recursion_limit.get(); + Self { + tcx, + param_env, + seen_tys, + query_ty: ty, + unchecked_tys: vec![(ty, 0)], + recursion_limit, + adt_components, + } + } +} + +impl<'tcx, F, I> Iterator for NeedsDropTypes<'tcx, F> +where + F: Fn(&ty::AdtDef) -> NeedsDropResult, + I: Iterator>, +{ + type Item = NeedsDropResult>; + + fn next(&mut self) -> Option>> { + let tcx = self.tcx; + + while let Some((ty, level)) = self.unchecked_tys.pop() { + if level > self.recursion_limit { + // Not having a `Span` isn't great. But there's hopefully some other + // recursion limit error as well. + tcx.sess.span_err( + DUMMY_SP, + &format!("overflow while checking whether `{}` requires drop", self.query_ty), + ); + return Some(Err(AlwaysRequiresDrop)); + } + + let components = match needs_drop_components(ty) { + Err(e) => return Some(Err(e)), + Ok(components) => components, + }; + debug!("needs_drop_components({:?}) = {:?}", ty, components); + + for component in components { + match component.kind { + _ if component.is_copy_modulo_regions(tcx, self.param_env, DUMMY_SP) => (), + + ty::Array(elem_ty, len) => { + // Zero-length arrays never contain anything to drop. + if len.try_eval_usize(tcx, self.param_env) != Some(0) { + if self.seen_tys.insert(elem_ty) { + self.unchecked_tys.push((elem_ty, level + 1)); + } + } + } + + ty::Closure(def_id, substs) => { + for upvar_ty in substs.as_closure().upvar_tys(def_id, tcx) { + if self.seen_tys.insert(upvar_ty) { + self.unchecked_tys.push((upvar_ty, level + 1)); + } + } + } + + // Check for a `Drop` impl and whether this is a union or + // `ManuallyDrop`. If it's a struct or enum without a `Drop` + // impl then check whether the field types need `Drop`. + ty::Adt(adt_def, substs) => { + let tys = match (self.adt_components)(adt_def) { + Err(e) => return Some(Err(e)), + Ok(tys) => tys, + }; + for required_ty in tys { + let subst_ty = tcx.normalize_erasing_regions( + self.param_env, + required_ty.subst(tcx, substs), + ); + if self.seen_tys.insert(subst_ty) { + self.unchecked_tys.push((subst_ty, level + 1)); + } + } + } + ty::Opaque(..) | ty::Projection(..) | ty::Param(_) => { + if ty == component { + // Return the type to the caller so they can decide + // what to do with it. + return Some(Ok(component)); + } else if self.seen_tys.insert(component) { + // Store the type for later. We can't return here + // because we would then lose any other components + // of the type. + self.unchecked_tys.push((component, level + 1)); + } + } + _ => return Some(Err(AlwaysRequiresDrop)), + } + } + } + + return None; + } +} + +fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List>, AlwaysRequiresDrop> { + let adt_components = move |adt_def: &ty::AdtDef| { + if adt_def.is_manually_drop() { + debug!("adt_drop_tys: `{:?}` is manually drop", adt_def); + return Ok(Vec::new().into_iter()); + } else if adt_def.destructor(tcx).is_some() { + debug!("adt_drop_tys: `{:?}` implements `Drop`", adt_def); + return Err(AlwaysRequiresDrop); + } else if adt_def.is_union() { + debug!("adt_drop_tys: `{:?}` is a union", adt_def); + return Ok(Vec::new().into_iter()); + } + Ok(adt_def.all_fields().map(|field| tcx.type_of(field.did)).collect::>().into_iter()) + }; + + let adt_ty = tcx.type_of(def_id); + let param_env = tcx.param_env(def_id); + let res: Result, _> = + NeedsDropTypes::new(tcx, param_env, adt_ty, adt_components).collect(); + + debug!("adt_drop_tys(`{}`) = `{:?}`", tcx.def_path_str(def_id), res); + res.map(|components| tcx.intern_type_list(&components)) +} + +pub(crate) fn provide(providers: &mut ty::query::Providers<'_>) { + *providers = ty::query::Providers { needs_drop_raw, adt_drop_tys, ..*providers }; +} diff --git a/src/test/ui/type-alias-impl-trait/issue-65918.rs b/src/test/ui/type-alias-impl-trait/issue-65918.rs index 97efb85ef64c7..4ba778d53ac08 100644 --- a/src/test/ui/type-alias-impl-trait/issue-65918.rs +++ b/src/test/ui/type-alias-impl-trait/issue-65918.rs @@ -1,3 +1,5 @@ +// ignore-test: This now ICEs again. + // build-pass #![feature(type_alias_impl_trait)] From d20646b2d8033f31423b5bda3e56776df115e144 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 31 Jan 2020 22:22:30 +0000 Subject: [PATCH 05/32] Address review comments * Handle arrays with const-generic lengths * Use closure for repeated code. --- src/librustc/ty/util.rs | 33 ++++++++++++++++++++++----------- src/librustc_ty/needs_drop.rs | 35 ++++++++++++++--------------------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index eaa7a43b09174..6191d304719cb 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -18,6 +18,7 @@ use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_macros::HashStable; use rustc_span::Span; +use rustc_target::abi::TargetDataLayout; use smallvec::SmallVec; use std::{cmp, fmt}; use syntax::ast; @@ -726,7 +727,7 @@ impl<'tcx> ty::TyS<'tcx> { #[inline] pub fn needs_drop(&'tcx self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool { // Avoid querying in simple cases. - match needs_drop_components(self) { + match needs_drop_components(self, &tcx.data_layout) { Err(AlwaysRequiresDrop) => true, Ok(components) => { let query_ty = match *components { @@ -736,7 +737,7 @@ impl<'tcx> ty::TyS<'tcx> { [component_ty] => component_ty, _ => self, }; - // This doesn't depend on regions, so try to minimize distinct. + // This doesn't depend on regions, so try to minimize distinct // query keys used. let erased = tcx.normalize_erasing_regions(param_env, query_ty); tcx.needs_drop_raw(param_env.and(erased)) @@ -992,7 +993,10 @@ impl<'tcx> ExplicitSelf<'tcx> { /// Returns a list of types such that the given type needs drop if and only if /// *any* of the returned types need drop. Returns `Err(AlwaysRequiresDrop)` if /// this type always needs drop. -pub fn needs_drop_components(ty: Ty<'tcx>) -> Result; 4]>, AlwaysRequiresDrop> { +pub fn needs_drop_components( + ty: Ty<'tcx>, + target_layout: &TargetDataLayout, +) -> Result; 2]>, AlwaysRequiresDrop> { match ty.kind { ty::Infer(ty::FreshIntTy(_)) | ty::Infer(ty::FreshFloatTy(_)) @@ -1017,18 +1021,25 @@ pub fn needs_drop_components(ty: Ty<'tcx>) -> Result; 4]>, Al // state transformation pass ty::Generator(..) | ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop), - ty::Slice(ty) => needs_drop_components(ty), - ty::Array(elem_ty, ..) => { - match needs_drop_components(elem_ty) { + ty::Slice(ty) => needs_drop_components(ty, target_layout), + ty::Array(elem_ty, size) => { + match needs_drop_components(elem_ty, target_layout) { Ok(v) if v.is_empty() => Ok(v), - // Arrays of size zero don't need drop, even if their element - // type does. - _ => Ok(smallvec![ty]), + res => match size.val.try_to_bits(target_layout.pointer_size) { + // Arrays of size zero don't need drop, even if their element + // type does. + Some(0) => Ok(SmallVec::new()), + Some(_) => res, + // We don't know which of the cases above we are in, so + // return the whole type and let the caller decide what to + // do. + None => Ok(smallvec![ty]), + }, } } // If any field needs drop, then the whole tuple does. - ty::Tuple(..) => ty.tuple_fields().try_fold(SmallVec::new(), |mut acc, elem| { - acc.extend(needs_drop_components(elem)?); + ty::Tuple(..) => ty.tuple_fields().try_fold(SmallVec::new(), move |mut acc, elem| { + acc.extend(needs_drop_components(elem, target_layout)?); Ok(acc) }), diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs index 1a65acb1f984b..c01b3e384aec5 100644 --- a/src/librustc_ty/needs_drop.rs +++ b/src/librustc_ty/needs_drop.rs @@ -76,30 +76,25 @@ where return Some(Err(AlwaysRequiresDrop)); } - let components = match needs_drop_components(ty) { + let components = match needs_drop_components(ty, &tcx.data_layout) { Err(e) => return Some(Err(e)), Ok(components) => components, }; debug!("needs_drop_components({:?}) = {:?}", ty, components); + let queue_type = move |this: &mut Self, component: Ty<'tcx>| { + if this.seen_tys.insert(component) { + this.unchecked_tys.push((component, level + 1)); + } + }; + for component in components { match component.kind { _ if component.is_copy_modulo_regions(tcx, self.param_env, DUMMY_SP) => (), - ty::Array(elem_ty, len) => { - // Zero-length arrays never contain anything to drop. - if len.try_eval_usize(tcx, self.param_env) != Some(0) { - if self.seen_tys.insert(elem_ty) { - self.unchecked_tys.push((elem_ty, level + 1)); - } - } - } - ty::Closure(def_id, substs) => { for upvar_ty in substs.as_closure().upvar_tys(def_id, tcx) { - if self.seen_tys.insert(upvar_ty) { - self.unchecked_tys.push((upvar_ty, level + 1)); - } + queue_type(self, upvar_ty); } } @@ -116,21 +111,19 @@ where self.param_env, required_ty.subst(tcx, substs), ); - if self.seen_tys.insert(subst_ty) { - self.unchecked_tys.push((subst_ty, level + 1)); - } + queue_type(self, subst_ty); } } - ty::Opaque(..) | ty::Projection(..) | ty::Param(_) => { + ty::Array(..) | ty::Opaque(..) | ty::Projection(..) | ty::Param(_) => { if ty == component { - // Return the type to the caller so they can decide - // what to do with it. + // Return the type to the caller: they may be able + // to normalize further than we can. return Some(Ok(component)); - } else if self.seen_tys.insert(component) { + } else { // Store the type for later. We can't return here // because we would then lose any other components // of the type. - self.unchecked_tys.push((component, level + 1)); + queue_type(self, component); } } _ => return Some(Err(AlwaysRequiresDrop)), From 465b86253ce828e215d564fde53adf8742f0e3f6 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 1 Feb 2020 11:41:58 +0000 Subject: [PATCH 06/32] Use correct `ParamEnv` in `Instance::resolve` --- src/librustc/ty/instance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index 51a18f8eae274..f73e069df36c7 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -285,7 +285,7 @@ impl<'tcx> Instance<'tcx> { _ => { if Some(def_id) == tcx.lang_items().drop_in_place_fn() { let ty = substs.type_at(0); - if ty.needs_drop(tcx, ty::ParamEnv::reveal_all()) { + if ty.needs_drop(tcx, param_env.with_reveal_all()) { debug!(" => nontrivial drop glue"); ty::InstanceDef::DropGlue(def_id, Some(ty)) } else { From 6bf2cc2229768faa8e86e0e8a9f5bd8ebfc817a2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 5 Feb 2020 09:44:03 +1100 Subject: [PATCH 07/32] Avoid instantiating many `Parser` structs in `generic_extension`. Currently, every iteration of the main loop in `generic_extension` instantiates a `Parser`, which is expensive because `Parser` is a large type. Many of those instantiations are only used immutably, particularly for simple-but-repetitive macros of the sort seen in `html5ever` and PR 68836. This commit initializes a single "base" parser outside the loop, and then uses `Cow` to avoid cloning it except for the mutating iterations. This speeds up `html5ever` runs by up to 15%. --- src/librustc_expand/lib.rs | 1 + src/librustc_expand/mbe/macro_parser.rs | 38 ++++----------- src/librustc_expand/mbe/macro_rules.rs | 62 ++++++++++++++++++------- 3 files changed, 56 insertions(+), 45 deletions(-) diff --git a/src/librustc_expand/lib.rs b/src/librustc_expand/lib.rs index 4fe7c268c4f0b..f119c956ced04 100644 --- a/src/librustc_expand/lib.rs +++ b/src/librustc_expand/lib.rs @@ -1,3 +1,4 @@ +#![feature(cow_is_borrowed)] #![feature(crate_visibility_modifier)] #![feature(decl_macro)] #![feature(proc_macro_diagnostic)] diff --git a/src/librustc_expand/mbe/macro_parser.rs b/src/librustc_expand/mbe/macro_parser.rs index b14725fd731b1..78f22f3e443b1 100644 --- a/src/librustc_expand/mbe/macro_parser.rs +++ b/src/librustc_expand/mbe/macro_parser.rs @@ -78,13 +78,11 @@ use crate::mbe::{self, TokenTree}; use rustc_ast_pretty::pprust; use rustc_parse::parser::{FollowedByType, Parser, PathStyle}; -use rustc_parse::Directory; use rustc_session::parse::ParseSess; use rustc_span::symbol::{kw, sym, Symbol}; use syntax::ast::{Ident, Name}; use syntax::ptr::P; use syntax::token::{self, DocComment, Nonterminal, Token}; -use syntax::tokenstream::TokenStream; use rustc_errors::{FatalError, PResult}; use rustc_span::Span; @@ -92,6 +90,7 @@ use smallvec::{smallvec, SmallVec}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::Lrc; +use std::borrow::Cow; use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::mem; use std::ops::{Deref, DerefMut}; @@ -613,28 +612,9 @@ fn inner_parse_loop<'root, 'tt>( Success(()) } -/// Use the given sequence of token trees (`ms`) as a matcher. Match the given token stream `tts` -/// against it and return the match. -/// -/// # Parameters -/// -/// - `sess`: The session into which errors are emitted -/// - `tts`: The tokenstream we are matching against the pattern `ms` -/// - `ms`: A sequence of token trees representing a pattern against which we are matching -/// - `directory`: Information about the file locations (needed for the black-box parser) -/// - `recurse_into_modules`: Whether or not to recurse into modules (needed for the black-box -/// parser) -pub(super) fn parse( - sess: &ParseSess, - tts: TokenStream, - ms: &[TokenTree], - directory: Option>, - recurse_into_modules: bool, -) -> NamedParseResult { - // Create a parser that can be used for the "black box" parts. - let mut parser = - Parser::new(sess, tts, directory, recurse_into_modules, true, rustc_parse::MACRO_ARGUMENTS); - +/// Use the given sequence of token trees (`ms`) as a matcher. Match the token +/// stream from the given `parser` against it and return the match. +pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> NamedParseResult { // A queue of possible matcher positions. We initialize it with the matcher position in which // the "dot" is before the first token of the first token tree in `ms`. `inner_parse_loop` then // processes all of these possible matcher positions and produces possible next positions into @@ -659,7 +639,7 @@ pub(super) fn parse( // parsing from the black-box parser done. The result is that `next_items` will contain a // bunch of possible next matcher positions in `next_items`. match inner_parse_loop( - sess, + parser.sess, &mut cur_items, &mut next_items, &mut eof_items, @@ -684,7 +664,7 @@ pub(super) fn parse( if eof_items.len() == 1 { let matches = eof_items[0].matches.iter_mut().map(|dv| Lrc::make_mut(dv).pop().unwrap()); - return nameize(sess, ms, matches); + return nameize(parser.sess, ms, matches); } else if eof_items.len() > 1 { return Error( parser.token.span, @@ -736,13 +716,13 @@ pub(super) fn parse( // If there are no possible next positions AND we aren't waiting for the black-box parser, // then there is a syntax error. else if bb_items.is_empty() && next_items.is_empty() { - return Failure(parser.token.take(), "no rules expected this token in macro call"); + return Failure(parser.token.clone(), "no rules expected this token in macro call"); } // Dump all possible `next_items` into `cur_items` for the next iteration. else if !next_items.is_empty() { // Now process the next token cur_items.extend(next_items.drain(..)); - parser.bump(); + parser.to_mut().bump(); } // Finally, we have the case where we need to call the black-box parser to get some // nonterminal. @@ -754,7 +734,7 @@ pub(super) fn parse( let match_cur = item.match_cur; item.push_match( match_cur, - MatchedNonterminal(Lrc::new(parse_nt(&mut parser, span, ident.name))), + MatchedNonterminal(Lrc::new(parse_nt(parser.to_mut(), span, ident.name))), ); item.idx += 1; item.match_cur += 1; diff --git a/src/librustc_expand/mbe/macro_rules.rs b/src/librustc_expand/mbe/macro_rules.rs index 29d41543fbf8c..9432790e78ced 100644 --- a/src/librustc_expand/mbe/macro_rules.rs +++ b/src/librustc_expand/mbe/macro_rules.rs @@ -1,11 +1,11 @@ -use crate::base::{DummyResult, ExtCtxt, MacResult, TTMacroExpander}; +use crate::base::{DummyResult, ExpansionData, ExtCtxt, MacResult, TTMacroExpander}; use crate::base::{SyntaxExtension, SyntaxExtensionKind}; use crate::expand::{ensure_complete_parse, parse_ast_fragment, AstFragment, AstFragmentKind}; use crate::mbe; use crate::mbe::macro_check; -use crate::mbe::macro_parser::parse; +use crate::mbe::macro_parser::parse_tt; use crate::mbe::macro_parser::{Error, Failure, Success}; -use crate::mbe::macro_parser::{MatchedNonterminal, MatchedSeq, NamedParseResult}; +use crate::mbe::macro_parser::{MatchedNonterminal, MatchedSeq}; use crate::mbe::transcribe::transcribe; use rustc_ast_pretty::pprust; @@ -166,9 +166,9 @@ impl TTMacroExpander for MacroRulesMacroExpander { } } -fn trace_macros_note(cx: &mut ExtCtxt<'_>, sp: Span, message: String) { +fn trace_macros_note(cx_expansions: &mut FxHashMap>, sp: Span, message: String) { let sp = sp.macro_backtrace().last().map(|trace| trace.call_site).unwrap_or(sp); - cx.expansions.entry(sp).or_default().push(message); + cx_expansions.entry(sp).or_default().push(message); } /// Given `lhses` and `rhses`, this is the new macro we create @@ -184,12 +184,36 @@ fn generic_extension<'cx>( ) -> Box { if cx.trace_macros() { let msg = format!("expanding `{}! {{ {} }}`", name, pprust::tts_to_string(arg.clone())); - trace_macros_note(cx, sp, msg); + trace_macros_note(&mut cx.expansions, sp, msg); } // Which arm's failure should we report? (the one furthest along) let mut best_failure: Option<(Token, &str)> = None; + + // We create a base parser that can be used for the "black box" parts. + // Every iteration needs a fresh copy of that base parser. However, the + // parser is not mutated on many of the iterations, particularly when + // dealing with macros like this: + // + // macro_rules! foo { + // ("a") => (A); + // ("b") => (B); + // ("c") => (C); + // // ... etc. (maybe hundreds more) + // } + // + // as seen in the `html5ever` benchmark. We use a `Cow` so that the base + // parser is only cloned when necessary (upon mutation). Furthermore, we + // reinitialize the `Cow` with the base parser at the start of every + // iteration, so that any mutated parsers are not reused. This is all quite + // hacky, but speeds up the `html5ever` benchmark significantly. (Issue + // 68836 suggests a more comprehensive but more complex change to deal with + // this situation.) + let base_parser = base_parser_from_cx(&cx.current_expansion, &cx.parse_sess, arg.clone()); + for (i, lhs) in lhses.iter().enumerate() { + let mut parser = Cow::Borrowed(&base_parser); + // try each arm's matchers let lhs_tt = match *lhs { mbe::TokenTree::Delimited(_, ref delim) => &delim.tts[..], @@ -202,7 +226,7 @@ fn generic_extension<'cx>( // are not recorded. On the first `Success(..)`ful matcher, the spans are merged. let mut gated_spans_snaphot = mem::take(&mut *cx.parse_sess.gated_spans.spans.borrow_mut()); - match parse_tt(cx, lhs_tt, arg.clone()) { + match parse_tt(&mut parser, lhs_tt) { Success(named_matches) => { // The matcher was `Success(..)`ful. // Merge the gated spans from parsing the matcher with the pre-existing ones. @@ -232,7 +256,7 @@ fn generic_extension<'cx>( if cx.trace_macros() { let msg = format!("to `{}`", pprust::tts_to_string(tts.clone())); - trace_macros_note(cx, sp, msg); + trace_macros_note(&mut cx.expansions, sp, msg); } let directory = Directory { @@ -269,6 +293,7 @@ fn generic_extension<'cx>( // Restore to the state before snapshotting and maybe try again. mem::swap(&mut gated_spans_snaphot, &mut cx.parse_sess.gated_spans.spans.borrow_mut()); } + drop(base_parser); let (token, label) = best_failure.expect("ran no matchers"); let span = token.span.substitute_dummy(sp); @@ -286,7 +311,9 @@ fn generic_extension<'cx>( mbe::TokenTree::Delimited(_, ref delim) => &delim.tts[..], _ => continue, }; - match parse_tt(cx, lhs_tt, arg.clone()) { + let base_parser = + base_parser_from_cx(&cx.current_expansion, &cx.parse_sess, arg.clone()); + match parse_tt(&mut Cow::Borrowed(&base_parser), lhs_tt) { Success(_) => { if comma_span.is_dummy() { err.note("you might be missing a comma"); @@ -368,7 +395,8 @@ pub fn compile_declarative_macro( ), ]; - let argument_map = match parse(sess, body, &argument_gram, None, true) { + let base_parser = Parser::new(sess, body, None, true, true, rustc_parse::MACRO_ARGUMENTS); + let argument_map = match parse_tt(&mut Cow::Borrowed(&base_parser), &argument_gram) { Success(m) => m, Failure(token, msg) => { let s = parse_failure_msg(&token); @@ -1184,14 +1212,16 @@ fn quoted_tt_to_string(tt: &mbe::TokenTree) -> String { } } -/// Use this token tree as a matcher to parse given tts. -fn parse_tt(cx: &ExtCtxt<'_>, mtch: &[mbe::TokenTree], tts: TokenStream) -> NamedParseResult { - // `None` is because we're not interpolating +fn base_parser_from_cx<'cx>( + current_expansion: &'cx ExpansionData, + sess: &'cx ParseSess, + tts: TokenStream, +) -> Parser<'cx> { let directory = Directory { - path: Cow::from(cx.current_expansion.module.directory.as_path()), - ownership: cx.current_expansion.directory_ownership, + path: Cow::from(current_expansion.module.directory.as_path()), + ownership: current_expansion.directory_ownership, }; - parse(cx.parse_sess(), tts, mtch, Some(directory), true) + Parser::new(sess, tts, Some(directory), true, true, rustc_parse::MACRO_ARGUMENTS) } /// Generates an appropriate parsing failure message. For EOF, this is "unexpected end...". For From f840a955bd449810e75d8320b4c46482d6dbdec1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 5 Feb 2020 14:33:08 +1100 Subject: [PATCH 08/32] Remove the `Cow` from `Directory`. The previous commit wrapped `Parser` within a `Cow` for the hot macro parsing path. As a result, there's no need for the `Cow` within `Directory`, because it lies within `Parser`. --- src/librustc_expand/mbe/macro_rules.rs | 4 ++-- src/librustc_parse/lib.rs | 9 ++++----- src/librustc_parse/parser/mod.rs | 9 ++++----- src/librustc_parse/parser/module.rs | 6 +++--- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/librustc_expand/mbe/macro_rules.rs b/src/librustc_expand/mbe/macro_rules.rs index 9432790e78ced..9e6edee265c98 100644 --- a/src/librustc_expand/mbe/macro_rules.rs +++ b/src/librustc_expand/mbe/macro_rules.rs @@ -260,7 +260,7 @@ fn generic_extension<'cx>( } let directory = Directory { - path: Cow::from(cx.current_expansion.module.directory.as_path()), + path: cx.current_expansion.module.directory.clone(), ownership: cx.current_expansion.directory_ownership, }; let mut p = Parser::new(cx.parse_sess(), tts, Some(directory), true, false, None); @@ -1218,7 +1218,7 @@ fn base_parser_from_cx<'cx>( tts: TokenStream, ) -> Parser<'cx> { let directory = Directory { - path: Cow::from(current_expansion.module.directory.as_path()), + path: current_expansion.module.directory.clone(), ownership: current_expansion.directory_ownership, }; Parser::new(sess, tts, Some(directory), true, true, rustc_parse::MACRO_ARGUMENTS) diff --git a/src/librustc_parse/lib.rs b/src/librustc_parse/lib.rs index cd674e3c5ebef..4aad2c0f68a29 100644 --- a/src/librustc_parse/lib.rs +++ b/src/librustc_parse/lib.rs @@ -12,8 +12,7 @@ use syntax::ast; use syntax::token::{self, Nonterminal}; use syntax::tokenstream::{self, TokenStream, TokenTree}; -use std::borrow::Cow; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::str; use log::info; @@ -29,8 +28,8 @@ pub mod validate_attr; pub mod config; #[derive(Clone)] -pub struct Directory<'a> { - pub path: Cow<'a, Path>, +pub struct Directory { + pub path: PathBuf, pub ownership: DirectoryOwnership, } @@ -274,7 +273,7 @@ pub fn stream_to_parser<'a>( pub fn stream_to_parser_with_base_dir<'a>( sess: &'a ParseSess, stream: TokenStream, - base_dir: Directory<'a>, + base_dir: Directory, ) -> Parser<'a> { Parser::new(sess, stream, Some(base_dir), true, false, None) } diff --git a/src/librustc_parse/parser/mod.rs b/src/librustc_parse/parser/mod.rs index 8c1839da1cb8f..cb95750d984e9 100644 --- a/src/librustc_parse/parser/mod.rs +++ b/src/librustc_parse/parser/mod.rs @@ -29,7 +29,6 @@ use syntax::token::{self, DelimToken, Token, TokenKind}; use syntax::tokenstream::{self, DelimSpan, TokenStream, TokenTree, TreeAndJoint}; use syntax::util::comments::{doc_comment_style, strip_doc_comment_decoration}; -use std::borrow::Cow; use std::path::PathBuf; use std::{cmp, mem, slice}; @@ -114,7 +113,7 @@ pub struct Parser<'a> { prev_token_kind: PrevTokenKind, restrictions: Restrictions, /// Used to determine the path to externally loaded source files. - pub(super) directory: Directory<'a>, + pub(super) directory: Directory, /// `true` to parse sub-modules in other files. // Public for rustfmt usage. pub recurse_into_file_modules: bool, @@ -376,7 +375,7 @@ impl<'a> Parser<'a> { pub fn new( sess: &'a ParseSess, tokens: TokenStream, - directory: Option>, + directory: Option, recurse_into_file_modules: bool, desugar_doc_comments: bool, subparser_name: Option<&'static str>, @@ -390,7 +389,7 @@ impl<'a> Parser<'a> { restrictions: Restrictions::empty(), recurse_into_file_modules, directory: Directory { - path: Cow::from(PathBuf::new()), + path: PathBuf::new(), ownership: DirectoryOwnership::Owned { relative: None }, }, root_module_name: None, @@ -418,7 +417,7 @@ impl<'a> Parser<'a> { &sess.source_map().lookup_char_pos(parser.token.span.lo()).file.unmapped_path { if let Some(directory_path) = path.parent() { - parser.directory.path = Cow::from(directory_path.to_path_buf()); + parser.directory.path = directory_path.to_path_buf(); } } } diff --git a/src/librustc_parse/parser/module.rs b/src/librustc_parse/parser/module.rs index 6ce94d3c6793c..0c8fad03d8690 100644 --- a/src/librustc_parse/parser/module.rs +++ b/src/librustc_parse/parser/module.rs @@ -285,7 +285,7 @@ impl<'a> Parser<'a> { fn push_directory(&mut self, id: Ident, attrs: &[Attribute]) { if let Some(path) = attr::first_attr_value_str_by_name(attrs, sym::path) { - self.directory.path.to_mut().push(&*path.as_str()); + self.directory.path.push(&*path.as_str()); self.directory.ownership = DirectoryOwnership::Owned { relative: None }; } else { // We have to push on the current module name in the case of relative @@ -297,10 +297,10 @@ impl<'a> Parser<'a> { if let DirectoryOwnership::Owned { relative } = &mut self.directory.ownership { if let Some(ident) = relative.take() { // remove the relative offset - self.directory.path.to_mut().push(&*ident.as_str()); + self.directory.path.push(&*ident.as_str()); } } - self.directory.path.to_mut().push(&*id.as_str()); + self.directory.path.push(&*id.as_str()); } } } From 2a13b24d369b8619f0197993cd5dc60f7217ed72 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 5 Feb 2020 15:09:24 +1100 Subject: [PATCH 09/32] Change condition ordering in `parse_tt`. This is a small win, because `Failure` is much more common than `Success`. --- src/librustc_expand/mbe/macro_parser.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_expand/mbe/macro_parser.rs b/src/librustc_expand/mbe/macro_parser.rs index 78f22f3e443b1..5bf7602ea6e8f 100644 --- a/src/librustc_expand/mbe/macro_parser.rs +++ b/src/librustc_expand/mbe/macro_parser.rs @@ -689,9 +689,14 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na // unnecessary implicit clone later in Rc::make_mut. drop(eof_items); + // If there are no possible next positions AND we aren't waiting for the black-box parser, + // then there is a syntax error. + if bb_items.is_empty() && next_items.is_empty() { + return Failure(parser.token.clone(), "no rules expected this token in macro call"); + } // Another possibility is that we need to call out to parse some rust nonterminal // (black-box) parser. However, if there is not EXACTLY ONE of these, something is wrong. - if (!bb_items.is_empty() && !next_items.is_empty()) || bb_items.len() > 1 { + else if (!bb_items.is_empty() && !next_items.is_empty()) || bb_items.len() > 1 { let nts = bb_items .iter() .map(|item| match item.top_elts.get_tt(item.idx) { @@ -713,11 +718,6 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na ), ); } - // If there are no possible next positions AND we aren't waiting for the black-box parser, - // then there is a syntax error. - else if bb_items.is_empty() && next_items.is_empty() { - return Failure(parser.token.clone(), "no rules expected this token in macro call"); - } // Dump all possible `next_items` into `cur_items` for the next iteration. else if !next_items.is_empty() { // Now process the next token From 3eb524188451fcec6cd5ed7e3cba2404021b75eb Mon Sep 17 00:00:00 2001 From: matthewjasper <20113453+matthewjasper@users.noreply.github.com> Date: Sun, 9 Feb 2020 10:16:57 +0000 Subject: [PATCH 10/32] Apply suggestions from code review Co-Authored-By: varkor --- src/librustc/ty/mod.rs | 2 +- src/librustc/ty/util.rs | 4 ++-- src/librustc_ty/needs_drop.rs | 13 +++++++------ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 2b272d7fe08af..85b9cbf3de15a 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2286,7 +2286,7 @@ impl<'tcx> AdtDef { self.flags.contains(AdtFlags::IS_BOX) } - /// Returns `true` if this is ManuallyDrop. + /// Returns `true` if this is `ManuallyDrop`. #[inline] pub fn is_manually_drop(&self) -> bool { self.flags.contains(AdtFlags::IS_MANUALLY_DROP) diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 6191d304719cb..4011c010c0355 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -1013,12 +1013,12 @@ pub fn needs_drop_components( | ty::Ref(..) | ty::Str => Ok(SmallVec::new()), - // Foreign types can never have destructors + // Foreign types can never have destructors. ty::Foreign(..) => Ok(SmallVec::new()), // Pessimistically assume that all generators will require destructors // as we don't know if a destructor is a noop or not until after the MIR - // state transformation pass + // state transformation pass. ty::Generator(..) | ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop), ty::Slice(ty) => needs_drop_components(ty, target_layout), diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs index c01b3e384aec5..0f71246c73759 100644 --- a/src/librustc_ty/needs_drop.rs +++ b/src/librustc_ty/needs_drop.rs @@ -12,9 +12,9 @@ type NeedsDropResult = Result; fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { let adt_fields = move |adt_def: &ty::AdtDef| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter().copied()); - // If we don't know a type doesn't need drop, say it's a type parameter - // without a `Copy` bound, then we conservatively return that it needs - // drop. + // If we don't know a type doesn't need drop, for example if it's a type + // parameter without a `Copy` bound, then we conservatively return that it + // needs drop. let res = NeedsDropTypes::new(tcx, query.param_env, query.value, adt_fields).next().is_some(); debug!("needs_drop_raw({:?}) = {:?}", query, res); res @@ -25,9 +25,10 @@ struct NeedsDropTypes<'tcx, F> { param_env: ty::ParamEnv<'tcx>, query_ty: Ty<'tcx>, seen_tys: FxHashSet>, - /// A stack of types left to process. Each round, we pop something from the - /// stack and check if it needs drop. If the result depends on whether some - /// other types need drop we push them onto the stack. + /// A stack of types left to process, and the recursion depth when we + /// pushed that type. Each round, we pop something from the stack and check + /// if it needs drop. If the result depends on whether some other types + /// need drop we push them onto the stack. unchecked_tys: Vec<(Ty<'tcx>, usize)>, recursion_limit: usize, adt_components: F, From 842938a82f673f0483e40328709c3887a2091534 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 9 Feb 2020 10:56:18 +0000 Subject: [PATCH 11/32] cache adt_drop_tys --- src/librustc/query/mod.rs | 4 +++- src/librustc/ty/util.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index c705956f4540a..39046df56e5b8 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -670,7 +670,9 @@ rustc_queries! { /// A list of types where the ADT requires drop if and only if any of /// those types require drop. If the ADT is known to always need drop /// then `Err(AlwaysRequiresDrop)` is returned. - query adt_drop_tys(_: DefId) -> Result<&'tcx ty::List>, AlwaysRequiresDrop> {} + query adt_drop_tys(_: DefId) -> Result<&'tcx ty::List>, AlwaysRequiresDrop> { + cache_on_disk_if { true } + } query layout_raw( env: ty::ParamEnvAnd<'tcx, Ty<'tcx>> diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 4011c010c0355..5a77bac8cf204 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -1056,5 +1056,5 @@ pub fn needs_drop_components( } } -#[derive(Copy, Clone, Debug, HashStable)] +#[derive(Copy, Clone, Debug, HashStable, RustcEncodable, RustcDecodable)] pub struct AlwaysRequiresDrop; From 9e78ce068bb649738d489bf3cc9d701aa9a47b1e Mon Sep 17 00:00:00 2001 From: Chris Simpkins Date: Mon, 10 Feb 2020 11:31:55 -0500 Subject: [PATCH 12/32] handle TerminatorKind::Yield by returning Err(Unpromotable) --- src/librustc_mir/transform/promote_consts.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 9a7f3f86a6fcd..a5d59860c3d16 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -463,6 +463,7 @@ impl<'tcx> Validator<'_, 'tcx> { let terminator = self.body[loc.block].terminator(); match &terminator.kind { TerminatorKind::Call { func, args, .. } => self.validate_call(func, args), + TerminatorKind::Yield { .. } => Err(Unpromotable), kind => { span_bug!(terminator.source_info.span, "{:?} not promotable", kind); } From fc3ecb22b9b7131ab0150672b4117ba9b53306fa Mon Sep 17 00:00:00 2001 From: Chris Simpkins Date: Mon, 10 Feb 2020 12:03:49 -0500 Subject: [PATCH 13/32] add issue 69017 test --- src/test/ui/generator/issue-69017.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 src/test/ui/generator/issue-69017.rs diff --git a/src/test/ui/generator/issue-69017.rs b/src/test/ui/generator/issue-69017.rs new file mode 100644 index 0000000000000..7b1d87efc9443 --- /dev/null +++ b/src/test/ui/generator/issue-69017.rs @@ -0,0 +1,16 @@ +// This issue reproduces an ICE on compile +// Fails on 2020-02-08 nightly +// regressed commit: https://github.com/rust-lang/rust/commit/f8fd4624474a68bd26694eff3536b9f3a127b2d3 +// +// check-pass + +#![feature(generator_trait)] +#![feature(generators)] + +use std::ops::Generator; + +fn gen() -> impl Generator { + |_: usize| { + println!("-> {}", yield); + } +} From 97d1f8d9bbb6ae25d22f5193006becf37a57d226 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Mon, 10 Feb 2020 17:00:59 +0100 Subject: [PATCH 14/32] Add missing `_zeroed` varants to `AllocRef` --- src/libcore/alloc.rs | 137 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/src/libcore/alloc.rs b/src/libcore/alloc.rs index 38df843d258d3..425deadeace0a 100644 --- a/src/libcore/alloc.rs +++ b/src/libcore/alloc.rs @@ -853,6 +853,59 @@ pub unsafe trait AllocRef { result } + /// Behaves like `realloc`, but also ensures that the new contents + /// are set to zero before being returned. + /// + /// # Safety + /// + /// This function is unsafe for the same reasons that `realloc` is. + /// + /// # Errors + /// + /// Returns `Err` only if the new layout + /// does not meet the allocator's size + /// and alignment constraints of the allocator, or if reallocation + /// otherwise fails. + /// + /// Implementations are encouraged to return `Err` on memory + /// exhaustion rather than panicking or aborting, but this is not + /// a strict requirement. (Specifically: it is *legal* to + /// implement this trait atop an underlying native allocation + /// library that aborts on memory exhaustion.) + /// + /// Clients wishing to abort computation in response to a + /// reallocation error are encouraged to call the [`handle_alloc_error`] function, + /// rather than directly invoking `panic!` or similar. + /// + /// [`handle_alloc_error`]: ../../alloc/alloc/fn.handle_alloc_error.html + unsafe fn realloc_zeroed( + &mut self, + ptr: NonNull, + layout: Layout, + new_size: usize, + ) -> Result, AllocErr> { + let old_size = layout.size(); + + if new_size >= old_size { + if let Ok(()) = self.grow_in_place_zeroed(ptr, layout, new_size) { + return Ok(ptr); + } + } else if new_size < old_size { + if let Ok(()) = self.shrink_in_place(ptr, layout, new_size) { + return Ok(ptr); + } + } + + // otherwise, fall back on alloc + copy + dealloc. + let new_layout = Layout::from_size_align_unchecked(new_size, layout.align()); + let result = self.alloc_zeroed(new_layout); + if let Ok(new_ptr) = result { + ptr::copy_nonoverlapping(ptr.as_ptr(), new_ptr.as_ptr(), cmp::min(old_size, new_size)); + self.dealloc(ptr, layout); + } + result + } + /// Behaves like `alloc`, but also ensures that the contents /// are set to zero before being returned. /// @@ -904,6 +957,31 @@ pub unsafe trait AllocRef { self.alloc(layout).map(|p| Excess(p, usable_size.1)) } + /// Behaves like `alloc`, but also returns the whole size of + /// the returned block. For some `layout` inputs, like arrays, this + /// may include extra storage usable for additional data. + /// Also it ensures that the contents are set to zero before being returned. + /// + /// # Safety + /// + /// This function is unsafe for the same reasons that `alloc` is. + /// + /// # Errors + /// + /// Returning `Err` indicates that either memory is exhausted or + /// `layout` does not meet allocator's size or alignment + /// constraints, just as in `alloc`. + /// + /// Clients wishing to abort computation in response to an + /// allocation error are encouraged to call the [`handle_alloc_error`] function, + /// rather than directly invoking `panic!` or similar. + /// + /// [`handle_alloc_error`]: ../../alloc/alloc/fn.handle_alloc_error.html + unsafe fn alloc_excess_zeroed(&mut self, layout: Layout) -> Result { + let usable_size = self.usable_size(&layout); + self.alloc_zeroed(layout).map(|p| Excess(p, usable_size.1)) + } + /// Behaves like `realloc`, but also returns the whole size of /// the returned block. For some `layout` inputs, like arrays, this /// may include extra storage usable for additional data. @@ -934,6 +1012,37 @@ pub unsafe trait AllocRef { self.realloc(ptr, layout, new_size).map(|p| Excess(p, usable_size.1)) } + /// Behaves like `realloc`, but also returns the whole size of + /// the returned block. For some `layout` inputs, like arrays, this + /// may include extra storage usable for additional data. + /// Also it ensures that the contents are set to zero before being returned. + /// + /// # Safety + /// + /// This function is unsafe for the same reasons that `realloc` is. + /// + /// # Errors + /// + /// Returning `Err` indicates that either memory is exhausted or + /// `layout` does not meet allocator's size or alignment + /// constraints, just as in `realloc`. + /// + /// Clients wishing to abort computation in response to a + /// reallocation error are encouraged to call the [`handle_alloc_error`] function, + /// rather than directly invoking `panic!` or similar. + /// + /// [`handle_alloc_error`]: ../../alloc/alloc/fn.handle_alloc_error.html + unsafe fn realloc_excess_zeroed( + &mut self, + ptr: NonNull, + layout: Layout, + new_size: usize, + ) -> Result { + let new_layout = Layout::from_size_align_unchecked(new_size, layout.align()); + let usable_size = self.usable_size(&new_layout); + self.realloc_zeroed(ptr, layout, new_size).map(|p| Excess(p, usable_size.1)) + } + /// Attempts to extend the allocation referenced by `ptr` to fit `new_size`. /// /// If this returns `Ok`, then the allocator has asserted that the @@ -983,6 +1092,34 @@ pub unsafe trait AllocRef { if new_size <= u { Ok(()) } else { Err(CannotReallocInPlace) } } + /// Behaves like `grow_in_place`, but also ensures that the new + /// contents are set to zero before being returned. + /// + /// # Safety + /// + /// This function is unsafe for the same reasons that `grow_in_place` is. + /// + /// # Errors + /// + /// Returns `Err(CannotReallocInPlace)` when the allocator is + /// unable to assert that the memory block referenced by `ptr` + /// could fit `layout`. + /// + /// Note that one cannot pass `CannotReallocInPlace` to the `handle_alloc_error` + /// function; clients are expected either to be able to recover from + /// `grow_in_place` failures without aborting, or to fall back on + /// another reallocation method before resorting to an abort. + unsafe fn grow_in_place_zeroed( + &mut self, + ptr: NonNull, + layout: Layout, + new_size: usize, + ) -> Result<(), CannotReallocInPlace> { + self.grow_in_place(ptr, layout, new_size)?; + ptr.as_ptr().add(layout.size()).write_bytes(0, new_size - layout.size()); + Ok(()) + } + /// Attempts to shrink the allocation referenced by `ptr` to fit `new_size`. /// /// If this returns `Ok`, then the allocator has asserted that the From 53b16fb5f2dad5571dc242c3671c1d0588183cb3 Mon Sep 17 00:00:00 2001 From: Chris Simpkins Date: Mon, 10 Feb 2020 13:47:52 -0500 Subject: [PATCH 15/32] add main function to issue-69017 test --- src/test/ui/generator/issue-69017.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/ui/generator/issue-69017.rs b/src/test/ui/generator/issue-69017.rs index 7b1d87efc9443..511deb60e4553 100644 --- a/src/test/ui/generator/issue-69017.rs +++ b/src/test/ui/generator/issue-69017.rs @@ -14,3 +14,5 @@ fn gen() -> impl Generator { println!("-> {}", yield); } } + +fn main() {} From e2b9d6ebd6808c2ac564cfde1a98793d9f00c9a3 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 20 Jan 2020 15:03:37 -0800 Subject: [PATCH 16/32] Add a `contains` method to `ResultsCursor` --- src/librustc_mir/dataflow/generic/cursor.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/librustc_mir/dataflow/generic/cursor.rs b/src/librustc_mir/dataflow/generic/cursor.rs index d2eff494ad701..8c0ab1505284a 100644 --- a/src/librustc_mir/dataflow/generic/cursor.rs +++ b/src/librustc_mir/dataflow/generic/cursor.rs @@ -65,6 +65,13 @@ where &self.state } + /// Returns `true` if the dataflow state at the current location contains the given element. + /// + /// Shorthand for `self.get().contains(elem)` + pub fn contains(&self, elem: A::Idx) -> bool { + self.state.contains(elem) + } + /// Resets the cursor to the start of the given basic block. pub fn seek_to_block_start(&mut self, block: BasicBlock) { self.state.overwrite(&self.results.borrow().entry_sets[block]); From 1ec99179c1ccb4305354d2d4ee503c40adcd714d Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 20 Jan 2020 15:05:26 -0800 Subject: [PATCH 17/32] Add an `into_engine` method to `Analysis` This makes it more ergonomic to create a dataflow engine and obviates the need to pick between `new_gen_kill` and `new_generic`. --- src/librustc_mir/dataflow/generic/mod.rs | 30 +++++++++++++++++++ .../transform/check_consts/validation.rs | 9 +++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/dataflow/generic/mod.rs b/src/librustc_mir/dataflow/generic/mod.rs index c9b4f875ebdf8..ed742143150f7 100644 --- a/src/librustc_mir/dataflow/generic/mod.rs +++ b/src/librustc_mir/dataflow/generic/mod.rs @@ -35,6 +35,8 @@ use std::io; use rustc::mir::{self, BasicBlock, Location}; +use rustc::ty::TyCtxt; +use rustc_hir::def_id::DefId; use rustc_index::bit_set::{BitSet, HybridBitSet}; use rustc_index::vec::{Idx, IndexVec}; @@ -166,6 +168,22 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { args: &[mir::Operand<'tcx>], return_place: &mir::Place<'tcx>, ); + + /// Creates an `Engine` to find the fixpoint for this dataflow problem. + /// + /// This is functionally equivalent to calling the appropriate `Engine` constructor. It should + /// not be overridden. Its purpose is to allow consumers of this API to use method-chaining. + fn into_engine( + self, + tcx: TyCtxt<'tcx>, + body: &'mir mir::Body<'tcx>, + def_id: DefId, + ) -> Engine<'mir, 'tcx, Self> + where + Self: Sized, + { + Engine::new_generic(tcx, body, def_id, self) + } } /// A gen/kill dataflow problem. @@ -272,6 +290,18 @@ where ) { self.call_return_effect(state, block, func, args, return_place); } + + fn into_engine( + self, + tcx: TyCtxt<'tcx>, + body: &'mir mir::Body<'tcx>, + def_id: DefId, + ) -> Engine<'mir, 'tcx, Self> + where + Self: Sized, + { + Engine::new_gen_kill(tcx, body, def_id, self) + } } /// The legal operations for a transfer function in a gen/kill problem. diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 35107a31aa122..aad471c785acc 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -22,6 +22,7 @@ use super::resolver::FlowSensitiveAnalysis; use super::{is_lang_panic_fn, ConstKind, Item, Qualif}; use crate::const_eval::{is_const_fn, is_unstable_const_fn}; use crate::dataflow::{self as old_dataflow, generic as dataflow}; +use dataflow::Analysis; pub type IndirectlyMutableResults<'mir, 'tcx> = old_dataflow::DataflowResultsCursor<'mir, 'tcx, IndirectlyMutableLocals<'mir, 'tcx>>; @@ -33,10 +34,10 @@ struct QualifCursor<'a, 'mir, 'tcx, Q: Qualif> { impl QualifCursor<'a, 'mir, 'tcx, Q> { pub fn new(q: Q, item: &'a Item<'mir, 'tcx>) -> Self { - let analysis = FlowSensitiveAnalysis::new(q, item); - let results = dataflow::Engine::new_generic(item.tcx, &item.body, item.def_id, analysis) - .iterate_to_fixpoint(); - let cursor = dataflow::ResultsCursor::new(*item.body, results); + let cursor = FlowSensitiveAnalysis::new(q, item) + .into_engine(item.tcx, &item.body, item.def_id) + .iterate_to_fixpoint() + .into_results_cursor(*item.body); let mut in_any_value_of_ty = BitSet::new_empty(item.body.local_decls.len()); for (local, decl) in item.body.local_decls.iter_enumerated() { From c64dfd76e8fc1e0d2b3d293cc134476e861cbccb Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 20 Jan 2020 15:06:08 -0800 Subject: [PATCH 18/32] Add a `Visitor` for dataflow results --- src/librustc_mir/dataflow/generic/mod.rs | 3 + src/librustc_mir/dataflow/generic/visitor.rs | 272 +++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 src/librustc_mir/dataflow/generic/visitor.rs diff --git a/src/librustc_mir/dataflow/generic/mod.rs b/src/librustc_mir/dataflow/generic/mod.rs index ed742143150f7..d961ba7fffa25 100644 --- a/src/librustc_mir/dataflow/generic/mod.rs +++ b/src/librustc_mir/dataflow/generic/mod.rs @@ -45,9 +45,12 @@ use crate::dataflow::BottomValue; mod cursor; mod engine; mod graphviz; +mod visitor; pub use self::cursor::{ResultsCursor, ResultsRefCursor}; pub use self::engine::Engine; +pub use self::visitor::{visit_results, ResultsVisitor}; +pub use self::visitor::{BorrowckFlowState, BorrowckResults}; /// A dataflow analysis that has converged to fixpoint. pub struct Results<'tcx, A> diff --git a/src/librustc_mir/dataflow/generic/visitor.rs b/src/librustc_mir/dataflow/generic/visitor.rs new file mode 100644 index 0000000000000..5bad8c61a0cd7 --- /dev/null +++ b/src/librustc_mir/dataflow/generic/visitor.rs @@ -0,0 +1,272 @@ +use rustc::mir::{self, BasicBlock, Location}; +use rustc_index::bit_set::BitSet; + +use super::{Analysis, Results}; +use crate::dataflow::impls::{borrows::Borrows, EverInitializedPlaces, MaybeUninitializedPlaces}; + +/// Calls the corresponding method in `ResultsVisitor` for every location in a `mir::Body` with the +/// dataflow state at that location. +pub fn visit_results( + body: &'mir mir::Body<'tcx>, + blocks: impl IntoIterator, + results: &impl ResultsVisitable<'tcx, FlowState = F>, + vis: &mut impl ResultsVisitor<'mir, 'tcx, FlowState = F>, +) { + let mut state = results.new_flow_state(body); + + for block in blocks { + let block_data = &body[block]; + results.reset_to_block_start(&mut state, block); + + for (statement_index, stmt) in block_data.statements.iter().enumerate() { + let loc = Location { block, statement_index }; + + results.reconstruct_before_statement_effect(&mut state, stmt, loc); + vis.visit_statement(&mut state, stmt, loc); + + results.reconstruct_statement_effect(&mut state, stmt, loc); + vis.visit_statement_exit(&mut state, stmt, loc); + } + + let loc = body.terminator_loc(block); + let term = block_data.terminator(); + + results.reconstruct_before_terminator_effect(&mut state, term, loc); + vis.visit_terminator(&mut state, term, loc); + + results.reconstruct_terminator_effect(&mut state, term, loc); + vis.visit_terminator_exit(&mut state, term, loc); + } +} + +pub trait ResultsVisitor<'mir, 'tcx> { + type FlowState; + + /// Called with the `before_statement_effect` of the given statement applied to `state` but not + /// its `statement_effect`. + fn visit_statement( + &mut self, + _state: &Self::FlowState, + _statement: &'mir mir::Statement<'tcx>, + _location: Location, + ) { + } + + /// Called with both the `before_statement_effect` and the `statement_effect` of the given + /// statement applied to `state`. + fn visit_statement_exit( + &mut self, + _state: &Self::FlowState, + _statement: &'mir mir::Statement<'tcx>, + _location: Location, + ) { + } + + /// Called with the `before_terminator_effect` of the given terminator applied to `state` but not + /// its `terminator_effect`. + fn visit_terminator( + &mut self, + _state: &Self::FlowState, + _terminator: &'mir mir::Terminator<'tcx>, + _location: Location, + ) { + } + + /// Called with both the `before_terminator_effect` and the `terminator_effect` of the given + /// terminator applied to `state`. + /// + /// The `call_return_effect` (if one exists) will *not* be applied to `state`. + fn visit_terminator_exit( + &mut self, + _state: &Self::FlowState, + _terminator: &'mir mir::Terminator<'tcx>, + _location: Location, + ) { + } +} + +/// Things that can be visited by a `ResultsVisitor`. +/// +/// This trait exists so that we can visit the results of multiple dataflow analyses simultaneously. +/// DO NOT IMPLEMENT MANUALLY. Instead, use the `impl_visitable` macro below. +pub trait ResultsVisitable<'tcx> { + type FlowState; + + /// Creates an empty `FlowState` to hold the transient state for these dataflow results. + /// + /// The value of the newly created `FlowState` will be overwritten by `reset_to_block_start` + /// before it can be observed by a `ResultsVisitor`. + fn new_flow_state(&self, body: &mir::Body<'tcx>) -> Self::FlowState; + + fn reset_to_block_start(&self, state: &mut Self::FlowState, block: BasicBlock); + + fn reconstruct_before_statement_effect( + &self, + state: &mut Self::FlowState, + statement: &mir::Statement<'tcx>, + location: Location, + ); + + fn reconstruct_statement_effect( + &self, + state: &mut Self::FlowState, + statement: &mir::Statement<'tcx>, + location: Location, + ); + + fn reconstruct_before_terminator_effect( + &self, + state: &mut Self::FlowState, + terminator: &mir::Terminator<'tcx>, + location: Location, + ); + + fn reconstruct_terminator_effect( + &self, + state: &mut Self::FlowState, + terminator: &mir::Terminator<'tcx>, + location: Location, + ); +} + +impl<'tcx, A> ResultsVisitable<'tcx> for Results<'tcx, A> +where + A: Analysis<'tcx>, +{ + type FlowState = BitSet; + + fn new_flow_state(&self, body: &mir::Body<'tcx>) -> Self::FlowState { + BitSet::new_empty(self.analysis.bits_per_block(body)) + } + + fn reset_to_block_start(&self, state: &mut Self::FlowState, block: BasicBlock) { + state.overwrite(&self.entry_set_for_block(block)); + } + + fn reconstruct_before_statement_effect( + &self, + state: &mut Self::FlowState, + stmt: &mir::Statement<'tcx>, + loc: Location, + ) { + self.analysis.apply_before_statement_effect(state, stmt, loc); + } + + fn reconstruct_statement_effect( + &self, + state: &mut Self::FlowState, + stmt: &mir::Statement<'tcx>, + loc: Location, + ) { + self.analysis.apply_statement_effect(state, stmt, loc); + } + + fn reconstruct_before_terminator_effect( + &self, + state: &mut Self::FlowState, + term: &mir::Terminator<'tcx>, + loc: Location, + ) { + self.analysis.apply_before_terminator_effect(state, term, loc); + } + + fn reconstruct_terminator_effect( + &self, + state: &mut Self::FlowState, + term: &mir::Terminator<'tcx>, + loc: Location, + ) { + self.analysis.apply_terminator_effect(state, term, loc); + } +} + +/// A tuple with named fields that can hold either the results or the transient state of the +/// dataflow analyses used by the borrow checker. +#[derive(Debug)] +pub struct BorrowckAnalyses { + pub borrows: B, + pub uninits: U, + pub ever_inits: E, +} + +/// The results of the dataflow analyses used by the borrow checker. +pub type BorrowckResults<'mir, 'tcx> = BorrowckAnalyses< + Results<'tcx, Borrows<'mir, 'tcx>>, + Results<'tcx, MaybeUninitializedPlaces<'mir, 'tcx>>, + Results<'tcx, EverInitializedPlaces<'mir, 'tcx>>, +>; + +/// The transient state of the dataflow analyses used by the borrow checker. +pub type BorrowckFlowState<'mir, 'tcx> = + as ResultsVisitable<'tcx>>::FlowState; + +macro_rules! impl_visitable { + ( $( + $T:ident { $( $field:ident : $A:ident ),* $(,)? } + )* ) => { $( + impl<'tcx, $($A),*> ResultsVisitable<'tcx> for $T<$( Results<'tcx, $A> ),*> + where + $( $A: Analysis<'tcx>, )* + { + type FlowState = $T<$( BitSet<$A::Idx> ),*>; + + fn new_flow_state(&self, body: &mir::Body<'tcx>) -> Self::FlowState { + $T { + $( $field: BitSet::new_empty(self.$field.analysis.bits_per_block(body)) ),* + } + } + + fn reset_to_block_start( + &self, + state: &mut Self::FlowState, + block: BasicBlock, + ) { + $( state.$field.overwrite(&self.$field.entry_sets[block]); )* + } + + fn reconstruct_before_statement_effect( + &self, + state: &mut Self::FlowState, + stmt: &mir::Statement<'tcx>, + loc: Location, + ) { + $( self.$field.analysis + .apply_before_statement_effect(&mut state.$field, stmt, loc); )* + } + + fn reconstruct_statement_effect( + &self, + state: &mut Self::FlowState, + stmt: &mir::Statement<'tcx>, + loc: Location, + ) { + $( self.$field.analysis + .apply_statement_effect(&mut state.$field, stmt, loc); )* + } + + fn reconstruct_before_terminator_effect( + &self, + state: &mut Self::FlowState, + term: &mir::Terminator<'tcx>, + loc: Location, + ) { + $( self.$field.analysis + .apply_before_terminator_effect(&mut state.$field, term, loc); )* + } + + fn reconstruct_terminator_effect( + &self, + state: &mut Self::FlowState, + term: &mir::Terminator<'tcx>, + loc: Location, + ) { + $( self.$field.analysis + .apply_terminator_effect(&mut state.$field, term, loc); )* + } + } + )* } +} + +impl_visitable! { + BorrowckAnalyses { borrows: B, uninits: U, ever_inits: E } +} From 702096da17254239d09e1095b060c083322b8ac2 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 20 Jan 2020 15:15:06 -0800 Subject: [PATCH 19/32] Implement a `find_descendant` method for `MovePath` --- src/librustc_mir/dataflow/at_location.rs | 41 ------------------ src/librustc_mir/dataflow/move_paths/mod.rs | 47 +++++++++++++++++++++ 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/src/librustc_mir/dataflow/at_location.rs b/src/librustc_mir/dataflow/at_location.rs index c6d29211d45a8..e4eb8506846c0 100644 --- a/src/librustc_mir/dataflow/at_location.rs +++ b/src/librustc_mir/dataflow/at_location.rs @@ -4,7 +4,6 @@ use rustc::mir::{BasicBlock, Location}; use rustc_index::bit_set::{BitIter, BitSet, HybridBitSet}; -use crate::dataflow::move_paths::{HasMoveData, MovePathIndex}; use crate::dataflow::{BitDenotation, DataflowResults, GenKillSet}; use std::borrow::Borrow; @@ -168,43 +167,3 @@ where self.stmt_trans.apply(&mut self.curr_state) } } - -impl<'tcx, T, DR> FlowAtLocation<'tcx, T, DR> -where - T: HasMoveData<'tcx> + BitDenotation<'tcx, Idx = MovePathIndex>, - DR: Borrow>, -{ - pub fn has_any_child_of(&self, mpi: T::Idx) -> Option { - // We process `mpi` before the loop below, for two reasons: - // - it's a little different from the loop case (we don't traverse its - // siblings); - // - ~99% of the time the loop isn't reached, and this code is hot, so - // we don't want to allocate `todo` unnecessarily. - if self.contains(mpi) { - return Some(mpi); - } - let move_data = self.operator().move_data(); - let move_path = &move_data.move_paths[mpi]; - let mut todo = if let Some(child) = move_path.first_child { - vec![child] - } else { - return None; - }; - - while let Some(mpi) = todo.pop() { - if self.contains(mpi) { - return Some(mpi); - } - let move_path = &move_data.move_paths[mpi]; - if let Some(child) = move_path.first_child { - todo.push(child); - } - // After we've processed the original `mpi`, we should always - // traverse the siblings of any of its children. - if let Some(sibling) = move_path.next_sibling { - todo.push(sibling); - } - } - return None; - } -} diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index 8d62b84bda8f0..614a276164334 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -72,6 +72,41 @@ impl<'tcx> MovePath<'tcx> { parents } + + /// Finds the closest descendant of `self` for which `f` returns `true` using a breadth-first + /// search. + /// + /// `f` will **not** be called on `self`. + pub fn find_descendant( + &self, + move_paths: &IndexVec>, + f: impl Fn(MovePathIndex) -> bool, + ) -> Option { + let mut todo = if let Some(child) = self.first_child { + vec![child] + } else { + return None; + }; + + while let Some(mpi) = todo.pop() { + if f(mpi) { + return Some(mpi); + } + + let move_path = &move_paths[mpi]; + if let Some(child) = move_path.first_child { + todo.push(child); + } + + // After we've processed the original `mpi`, we should always + // traverse the siblings of any of its children. + if let Some(sibling) = move_path.next_sibling { + todo.push(sibling); + } + } + + None + } } impl<'tcx> fmt::Debug for MovePath<'tcx> { @@ -333,4 +368,16 @@ impl<'tcx> MoveData<'tcx> { } } } + + pub fn find_in_move_path_or_its_descendants( + &self, + root: MovePathIndex, + pred: impl Fn(MovePathIndex) -> bool, + ) -> Option { + if pred(root) { + return Some(root); + } + + self.move_paths[root].find_descendant(&self.move_paths, pred) + } } From ce3f37b42ba25ab6fe2f401135be847a0351fbbe Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 20 Jan 2020 15:14:24 -0800 Subject: [PATCH 20/32] Use new dataflow interface for initialization/borrows analyses --- src/librustc_mir/dataflow/impls/borrows.rs | 88 +++++---- src/librustc_mir/dataflow/impls/mod.rs | 215 ++++++++++++++------- 2 files changed, 195 insertions(+), 108 deletions(-) diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index f94ee67f2bea7..151ae28bae255 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -10,7 +10,8 @@ use crate::borrow_check::{ places_conflict, BorrowData, BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext, ToRegionVid, }; -use crate::dataflow::{BitDenotation, BottomValue, GenKillSet}; +use crate::dataflow::generic::{self, GenKill}; +use crate::dataflow::BottomValue; use std::rc::Rc; @@ -172,7 +173,7 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { /// That means they went out of a nonlexical scope fn kill_loans_out_of_scope_at_location( &self, - trans: &mut GenKillSet, + trans: &mut impl GenKill, location: Location, ) { // NOTE: The state associated with a given `location` @@ -187,16 +188,21 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { // region, then setting that gen-bit will override any // potential kill introduced here. if let Some(indices) = self.borrows_out_of_scope_at_location.get(&location) { - trans.kill_all(indices); + trans.kill_all(indices.iter().copied()); } } /// Kill any borrows that conflict with `place`. - fn kill_borrows_on_place(&self, trans: &mut GenKillSet, place: &Place<'tcx>) { + fn kill_borrows_on_place(&self, trans: &mut impl GenKill, place: &Place<'tcx>) { debug!("kill_borrows_on_place: place={:?}", place); - let other_borrows_of_local = - self.borrow_set.local_map.get(&place.local).into_iter().flat_map(|bs| bs.into_iter()); + let other_borrows_of_local = self + .borrow_set + .local_map + .get(&place.local) + .into_iter() + .flat_map(|bs| bs.into_iter()) + .copied(); // If the borrowed place is a local with no projections, all other borrows of this // local must conflict. This is purely an optimization so we don't have to call @@ -212,7 +218,7 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { // pair of array indices are unequal, so that when `places_conflict` returns true, we // will be assured that two places being compared definitely denotes the same sets of // locations. - let definitely_conflicting_borrows = other_borrows_of_local.filter(|&&i| { + let definitely_conflicting_borrows = other_borrows_of_local.filter(|&i| { places_conflict( self.tcx, self.body, @@ -226,36 +232,41 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { } } -impl<'a, 'tcx> BitDenotation<'tcx> for Borrows<'a, 'tcx> { +impl<'tcx> generic::AnalysisDomain<'tcx> for Borrows<'_, 'tcx> { type Idx = BorrowIndex; - fn name() -> &'static str { - "borrows" - } - fn bits_per_block(&self) -> usize { + + const NAME: &'static str = "borrows"; + + fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { self.borrow_set.borrows.len() * 2 } - fn start_block_effect(&self, _entry_set: &mut BitSet) { + fn initialize_start_block(&self, _: &mir::Body<'tcx>, _: &mut BitSet) { // no borrows of code region_scopes have been taken prior to // function execution, so this method has no effect. } - fn before_statement_effect(&self, trans: &mut GenKillSet, location: Location) { - debug!("Borrows::before_statement_effect trans: {:?} location: {:?}", trans, location); - self.kill_loans_out_of_scope_at_location(trans, location); + fn pretty_print_idx(&self, w: &mut impl std::io::Write, idx: Self::Idx) -> std::io::Result<()> { + write!(w, "{:?}", self.location(idx)) } +} - fn statement_effect(&self, trans: &mut GenKillSet, location: Location) { - debug!("Borrows::statement_effect: trans={:?} location={:?}", trans, location); - - let block = &self.body.basic_blocks().get(location.block).unwrap_or_else(|| { - panic!("could not find block at location {:?}", location); - }); - let stmt = block.statements.get(location.statement_index).unwrap_or_else(|| { - panic!("could not find statement at location {:?}"); - }); +impl<'tcx> generic::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> { + fn before_statement_effect( + &self, + trans: &mut impl GenKill, + _statement: &mir::Statement<'tcx>, + location: Location, + ) { + self.kill_loans_out_of_scope_at_location(trans, location); + } - debug!("Borrows::statement_effect: stmt={:?}", stmt); + fn statement_effect( + &self, + trans: &mut impl GenKill, + stmt: &mir::Statement<'tcx>, + location: Location, + ) { match stmt.kind { mir::StatementKind::Assign(box (ref lhs, ref rhs)) => { if let mir::Rvalue::Ref(_, _, ref place) = *rhs { @@ -301,18 +312,29 @@ impl<'a, 'tcx> BitDenotation<'tcx> for Borrows<'a, 'tcx> { } } - fn before_terminator_effect(&self, trans: &mut GenKillSet, location: Location) { - debug!("Borrows::before_terminator_effect: trans={:?} location={:?}", trans, location); + fn before_terminator_effect( + &self, + trans: &mut impl GenKill, + _terminator: &mir::Terminator<'tcx>, + location: Location, + ) { self.kill_loans_out_of_scope_at_location(trans, location); } - fn terminator_effect(&self, _: &mut GenKillSet, _: Location) {} + fn terminator_effect( + &self, + _: &mut impl GenKill, + _: &mir::Terminator<'tcx>, + _: Location, + ) { + } - fn propagate_call_return( + fn call_return_effect( &self, - _in_out: &mut BitSet, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, + _trans: &mut impl GenKill, + _block: mir::BasicBlock, + _func: &mir::Operand<'tcx>, + _args: &[mir::Operand<'tcx>], _dest_place: &mir::Place<'tcx>, ) { } diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs index 6b66406338d1e..5b2264c2a6526 100644 --- a/src/librustc_mir/dataflow/impls/mod.rs +++ b/src/librustc_mir/dataflow/impls/mod.rs @@ -11,8 +11,9 @@ use super::MoveDataParamEnv; use crate::util::elaborate_drops::DropFlagState; +use super::generic::{AnalysisDomain, GenKill, GenKillAnalysis}; use super::move_paths::{HasMoveData, InitIndex, InitKind, MoveData, MovePathIndex}; -use super::{BitDenotation, BottomValue, GenKillSet}; +use super::{BottomValue, GenKillSet}; use super::drop_flag_effects_for_function_entry; use super::drop_flag_effects_for_location; @@ -216,6 +217,7 @@ impl<'a, 'tcx> HasMoveData<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { /// } /// ``` pub struct EverInitializedPlaces<'a, 'tcx> { + #[allow(dead_code)] tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, mdpe: &'a MoveDataParamEnv<'tcx>, @@ -235,7 +237,7 @@ impl<'a, 'tcx> HasMoveData<'tcx> for EverInitializedPlaces<'a, 'tcx> { impl<'a, 'tcx> MaybeInitializedPlaces<'a, 'tcx> { fn update_bits( - trans: &mut GenKillSet, + trans: &mut impl GenKill, path: MovePathIndex, state: DropFlagState, ) { @@ -248,7 +250,7 @@ impl<'a, 'tcx> MaybeInitializedPlaces<'a, 'tcx> { impl<'a, 'tcx> MaybeUninitializedPlaces<'a, 'tcx> { fn update_bits( - trans: &mut GenKillSet, + trans: &mut impl GenKill, path: MovePathIndex, state: DropFlagState, ) { @@ -261,7 +263,7 @@ impl<'a, 'tcx> MaybeUninitializedPlaces<'a, 'tcx> { impl<'a, 'tcx> DefinitelyInitializedPlaces<'a, 'tcx> { fn update_bits( - trans: &mut GenKillSet, + trans: &mut impl GenKill, path: MovePathIndex, state: DropFlagState, ) { @@ -272,39 +274,56 @@ impl<'a, 'tcx> DefinitelyInitializedPlaces<'a, 'tcx> { } } -impl<'a, 'tcx> BitDenotation<'tcx> for MaybeInitializedPlaces<'a, 'tcx> { +impl<'tcx> AnalysisDomain<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { type Idx = MovePathIndex; - fn name() -> &'static str { - "maybe_init" - } - fn bits_per_block(&self) -> usize { + + const NAME: &'static str = "maybe_init"; + + fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { self.move_data().move_paths.len() } - fn start_block_effect(&self, entry_set: &mut BitSet) { + fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut BitSet) { drop_flag_effects_for_function_entry(self.tcx, self.body, self.mdpe, |path, s| { assert!(s == DropFlagState::Present); - entry_set.insert(path); + state.insert(path); }); } - fn statement_effect(&self, trans: &mut GenKillSet, location: Location) { + fn pretty_print_idx(&self, w: &mut impl std::io::Write, mpi: Self::Idx) -> std::io::Result<()> { + write!(w, "{}", self.move_data().move_paths[mpi]) + } +} + +impl<'tcx> GenKillAnalysis<'tcx> for MaybeInitializedPlaces<'_, 'tcx> { + fn statement_effect( + &self, + trans: &mut impl GenKill, + _statement: &mir::Statement<'tcx>, + location: Location, + ) { drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| { Self::update_bits(trans, path, s) }) } - fn terminator_effect(&self, trans: &mut GenKillSet, location: Location) { + fn terminator_effect( + &self, + trans: &mut impl GenKill, + _terminator: &mir::Terminator<'tcx>, + location: Location, + ) { drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| { Self::update_bits(trans, path, s) }) } - fn propagate_call_return( + fn call_return_effect( &self, - in_out: &mut BitSet, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, + trans: &mut impl GenKill, + _block: mir::BasicBlock, + _func: &mir::Operand<'tcx>, + _args: &[mir::Operand<'tcx>], dest_place: &mir::Place<'tcx>, ) { // when a call returns successfully, that means we need to set @@ -315,50 +334,67 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeInitializedPlaces<'a, 'tcx> { self.move_data(), self.move_data().rev_lookup.find(dest_place.as_ref()), |mpi| { - in_out.insert(mpi); + trans.gen(mpi); }, ); } } -impl<'a, 'tcx> BitDenotation<'tcx> for MaybeUninitializedPlaces<'a, 'tcx> { +impl<'tcx> AnalysisDomain<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { type Idx = MovePathIndex; - fn name() -> &'static str { - "maybe_uninit" - } - fn bits_per_block(&self) -> usize { + + const NAME: &'static str = "maybe_uninit"; + + fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { self.move_data().move_paths.len() } // sets on_entry bits for Arg places - fn start_block_effect(&self, entry_set: &mut BitSet) { + fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut BitSet) { // set all bits to 1 (uninit) before gathering counterevidence - assert!(self.bits_per_block() == entry_set.domain_size()); - entry_set.insert_all(); + assert!(self.bits_per_block(body) == state.domain_size()); + state.insert_all(); drop_flag_effects_for_function_entry(self.tcx, self.body, self.mdpe, |path, s| { assert!(s == DropFlagState::Present); - entry_set.remove(path); + state.remove(path); }); } - fn statement_effect(&self, trans: &mut GenKillSet, location: Location) { + fn pretty_print_idx(&self, w: &mut impl std::io::Write, mpi: Self::Idx) -> std::io::Result<()> { + write!(w, "{}", self.move_data().move_paths[mpi]) + } +} + +impl<'tcx> GenKillAnalysis<'tcx> for MaybeUninitializedPlaces<'_, 'tcx> { + fn statement_effect( + &self, + trans: &mut impl GenKill, + _statement: &mir::Statement<'tcx>, + location: Location, + ) { drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| { Self::update_bits(trans, path, s) }) } - fn terminator_effect(&self, trans: &mut GenKillSet, location: Location) { + fn terminator_effect( + &self, + trans: &mut impl GenKill, + _terminator: &mir::Terminator<'tcx>, + location: Location, + ) { drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| { Self::update_bits(trans, path, s) }) } - fn propagate_call_return( + fn call_return_effect( &self, - in_out: &mut BitSet, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, + trans: &mut impl GenKill, + _block: mir::BasicBlock, + _func: &mir::Operand<'tcx>, + _args: &[mir::Operand<'tcx>], dest_place: &mir::Place<'tcx>, ) { // when a call returns successfully, that means we need to set @@ -369,48 +405,65 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeUninitializedPlaces<'a, 'tcx> { self.move_data(), self.move_data().rev_lookup.find(dest_place.as_ref()), |mpi| { - in_out.remove(mpi); + trans.kill(mpi); }, ); } } -impl<'a, 'tcx> BitDenotation<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { +impl<'a, 'tcx> AnalysisDomain<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { type Idx = MovePathIndex; - fn name() -> &'static str { - "definite_init" - } - fn bits_per_block(&self) -> usize { + + const NAME: &'static str = "definite_init"; + + fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { self.move_data().move_paths.len() } // sets on_entry bits for Arg places - fn start_block_effect(&self, entry_set: &mut BitSet) { - entry_set.clear(); + fn initialize_start_block(&self, _: &mir::Body<'tcx>, state: &mut BitSet) { + state.clear(); drop_flag_effects_for_function_entry(self.tcx, self.body, self.mdpe, |path, s| { assert!(s == DropFlagState::Present); - entry_set.insert(path); + state.insert(path); }); } - fn statement_effect(&self, trans: &mut GenKillSet, location: Location) { + fn pretty_print_idx(&self, w: &mut impl std::io::Write, mpi: Self::Idx) -> std::io::Result<()> { + write!(w, "{}", self.move_data().move_paths[mpi]) + } +} + +impl<'tcx> GenKillAnalysis<'tcx> for DefinitelyInitializedPlaces<'_, 'tcx> { + fn statement_effect( + &self, + trans: &mut impl GenKill, + _statement: &mir::Statement<'tcx>, + location: Location, + ) { drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| { Self::update_bits(trans, path, s) }) } - fn terminator_effect(&self, trans: &mut GenKillSet, location: Location) { + fn terminator_effect( + &self, + trans: &mut impl GenKill, + _terminator: &mir::Terminator<'tcx>, + location: Location, + ) { drop_flag_effects_for_location(self.tcx, self.body, self.mdpe, location, |path, s| { Self::update_bits(trans, path, s) }) } - fn propagate_call_return( + fn call_return_effect( &self, - in_out: &mut BitSet, - _call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, + trans: &mut impl GenKill, + _block: mir::BasicBlock, + _func: &mir::Operand<'tcx>, + _args: &[mir::Operand<'tcx>], dest_place: &mir::Place<'tcx>, ) { // when a call returns successfully, that means we need to set @@ -421,30 +474,36 @@ impl<'a, 'tcx> BitDenotation<'tcx> for DefinitelyInitializedPlaces<'a, 'tcx> { self.move_data(), self.move_data().rev_lookup.find(dest_place.as_ref()), |mpi| { - in_out.insert(mpi); + trans.gen(mpi); }, ); } } -impl<'a, 'tcx> BitDenotation<'tcx> for EverInitializedPlaces<'a, 'tcx> { +impl<'tcx> AnalysisDomain<'tcx> for EverInitializedPlaces<'_, 'tcx> { type Idx = InitIndex; - fn name() -> &'static str { - "ever_init" - } - fn bits_per_block(&self) -> usize { + + const NAME: &'static str = "ever_init"; + + fn bits_per_block(&self, _: &mir::Body<'tcx>) -> usize { self.move_data().inits.len() } - fn start_block_effect(&self, entry_set: &mut BitSet) { - for arg_init in 0..self.body.arg_count { - entry_set.insert(InitIndex::new(arg_init)); + fn initialize_start_block(&self, body: &mir::Body<'tcx>, state: &mut BitSet) { + for arg_init in 0..body.arg_count { + state.insert(InitIndex::new(arg_init)); } } +} - fn statement_effect(&self, trans: &mut GenKillSet, location: Location) { - let (_, body, move_data) = (self.tcx, self.body, self.move_data()); - let stmt = &body[location.block].statements[location.statement_index]; +impl<'tcx> GenKillAnalysis<'tcx> for EverInitializedPlaces<'_, 'tcx> { + fn statement_effect( + &self, + trans: &mut impl GenKill, + stmt: &mir::Statement<'tcx>, + location: Location, + ) { + let move_data = self.move_data(); let init_path_map = &move_data.init_path_map; let init_loc_map = &move_data.init_loc_map; let rev_lookup = &move_data.rev_lookup; @@ -453,7 +512,7 @@ impl<'a, 'tcx> BitDenotation<'tcx> for EverInitializedPlaces<'a, 'tcx> { "statement {:?} at loc {:?} initializes move_indexes {:?}", stmt, location, &init_loc_map[location] ); - trans.gen_all(&init_loc_map[location]); + trans.gen_all(init_loc_map[location].iter().copied()); match stmt.kind { mir::StatementKind::StorageDead(local) => { @@ -464,13 +523,18 @@ impl<'a, 'tcx> BitDenotation<'tcx> for EverInitializedPlaces<'a, 'tcx> { "stmt {:?} at loc {:?} clears the ever initialized status of {:?}", stmt, location, &init_path_map[move_path_index] ); - trans.kill_all(&init_path_map[move_path_index]); + trans.kill_all(init_path_map[move_path_index].iter().copied()); } _ => {} } } - fn terminator_effect(&self, trans: &mut GenKillSet, location: Location) { + fn terminator_effect( + &self, + trans: &mut impl GenKill, + _terminator: &mir::Terminator<'tcx>, + location: Location, + ) { let (body, move_data) = (self.body, self.move_data()); let term = body[location.block].terminator(); let init_loc_map = &move_data.init_loc_map; @@ -479,28 +543,29 @@ impl<'a, 'tcx> BitDenotation<'tcx> for EverInitializedPlaces<'a, 'tcx> { term, location, &init_loc_map[location] ); trans.gen_all( - init_loc_map[location].iter().filter(|init_index| { - move_data.inits[**init_index].kind != InitKind::NonPanicPathOnly - }), + init_loc_map[location] + .iter() + .filter(|init_index| { + move_data.inits[**init_index].kind != InitKind::NonPanicPathOnly + }) + .copied(), ); } - fn propagate_call_return( + fn call_return_effect( &self, - in_out: &mut BitSet, - call_bb: mir::BasicBlock, - _dest_bb: mir::BasicBlock, + trans: &mut impl GenKill, + block: mir::BasicBlock, + _func: &mir::Operand<'tcx>, + _args: &[mir::Operand<'tcx>], _dest_place: &mir::Place<'tcx>, ) { let move_data = self.move_data(); - let bits_per_block = self.bits_per_block(); let init_loc_map = &move_data.init_loc_map; - let call_loc = - Location { block: call_bb, statement_index: self.body[call_bb].statements.len() }; + let call_loc = self.body.terminator_loc(block); for init_index in &init_loc_map[call_loc] { - assert!(init_index.index() < bits_per_block); - in_out.insert(*init_index); + trans.gen(*init_index); } } } From 76aa29ff5e5a6bb355b017da4f6e476049b8dd76 Mon Sep 17 00:00:00 2001 From: Tim Diekmann Date: Tue, 11 Feb 2020 13:02:51 +0100 Subject: [PATCH 21/32] Preparation for allocator aware `Box` --- src/liballoc/alloc.rs | 18 ++++++++++++------ src/liballoc/boxed.rs | 31 ++++++++++++++++--------------- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/liballoc/alloc.rs b/src/liballoc/alloc.rs index 9fb0de63e6fc5..f41404bf8cab9 100644 --- a/src/liballoc/alloc.rs +++ b/src/liballoc/alloc.rs @@ -200,21 +200,27 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 { align as *mut u8 } else { let layout = Layout::from_size_align_unchecked(size, align); - let ptr = alloc(layout); - if !ptr.is_null() { ptr } else { handle_alloc_error(layout) } + match Global.alloc(layout) { + Ok(ptr) => ptr.as_ptr(), + Err(_) => handle_alloc_error(layout), + } } } #[cfg_attr(not(test), lang = "box_free")] #[inline] +// This signature has to be the same as `Box`, otherwise an ICE will happen. +// When an additional parameter to `Box` is added (like `A: AllocRef`), this has to be added here as +// well. +// For example if `Box` is changed to `struct Box(Unique, A)`, +// this function has to be changed to `fn box_free(Unique, A)` as well. pub(crate) unsafe fn box_free(ptr: Unique) { - let ptr = ptr.as_ptr(); - let size = size_of_val(&*ptr); - let align = min_align_of_val(&*ptr); + let size = size_of_val(ptr.as_ref()); + let align = min_align_of_val(ptr.as_ref()); // We do not allocate for Box when T is ZST, so deallocation is also not necessary. if size != 0 { let layout = Layout::from_size_align_unchecked(size, align); - dealloc(ptr as *mut u8, layout); + Global.dealloc(ptr.cast().into(), layout); } } diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index d65aee0923280..3ac4bd82a3a10 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -196,12 +196,14 @@ impl Box { #[unstable(feature = "new_uninit", issue = "63291")] pub fn new_uninit() -> Box> { let layout = alloc::Layout::new::>(); - if layout.size() == 0 { - return Box(NonNull::dangling().into()); + unsafe { + let ptr = if layout.size() == 0 { + NonNull::dangling() + } else { + Global.alloc(layout).unwrap_or_else(|_| alloc::handle_alloc_error(layout)).cast() + }; + Box::from_raw(ptr.as_ptr()) } - let ptr = - unsafe { Global.alloc(layout).unwrap_or_else(|_| alloc::handle_alloc_error(layout)) }; - Box(ptr.cast().into()) } /// Constructs a new `Box` with uninitialized contents, with the memory @@ -264,15 +266,14 @@ impl Box<[T]> { #[unstable(feature = "new_uninit", issue = "63291")] pub fn new_uninit_slice(len: usize) -> Box<[mem::MaybeUninit]> { let layout = alloc::Layout::array::>(len).unwrap(); - let ptr = if layout.size() == 0 { - NonNull::dangling() - } else { - unsafe { + unsafe { + let ptr = if layout.size() == 0 { + NonNull::dangling() + } else { Global.alloc(layout).unwrap_or_else(|_| alloc::handle_alloc_error(layout)).cast() - } - }; - let slice = unsafe { slice::from_raw_parts_mut(ptr.as_ptr(), len) }; - Box(Unique::from(slice)) + }; + Box::from_raw(slice::from_raw_parts_mut(ptr.as_ptr(), len)) + } } } @@ -308,7 +309,7 @@ impl Box> { #[unstable(feature = "new_uninit", issue = "63291")] #[inline] pub unsafe fn assume_init(self) -> Box { - Box(Box::into_unique(self).cast()) + Box::from_raw(Box::into_raw(self) as *mut T) } } @@ -346,7 +347,7 @@ impl Box<[mem::MaybeUninit]> { #[unstable(feature = "new_uninit", issue = "63291")] #[inline] pub unsafe fn assume_init(self) -> Box<[T]> { - Box(Unique::new_unchecked(Box::into_raw(self) as _)) + Box::from_raw(Box::into_raw(self) as *mut [T]) } } From 388431160db6b6c1c04364fcc826bc909a558c0b Mon Sep 17 00:00:00 2001 From: Jason Liquorish Date: Tue, 11 Feb 2020 18:33:00 +0000 Subject: [PATCH 22/32] Add self to .mailmap --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index 14797092475a2..e5aad52ef4b79 100644 --- a/.mailmap +++ b/.mailmap @@ -114,6 +114,7 @@ James Deng James Miller James Perry Jason Fager +Jason Liquorish Jason Orendorff Jason Orendorff Jason Toffaletti Jason Toffaletti From 30a8353f372f7cc719d1de6811996ce5215183a6 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 10 Feb 2020 22:31:19 +0000 Subject: [PATCH 23/32] Specify overflow checks behaviour in test --- src/test/ui/recursion/recursion.rs | 1 + src/test/ui/recursion/recursion.stderr | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/ui/recursion/recursion.rs b/src/test/ui/recursion/recursion.rs index 9d939e131823c..bf1eaef367d69 100644 --- a/src/test/ui/recursion/recursion.rs +++ b/src/test/ui/recursion/recursion.rs @@ -1,4 +1,5 @@ // build-fail +// compile-flags:-C overflow-checks=off enum Nil {NilValue} struct Cons {head:isize, tail:T} diff --git a/src/test/ui/recursion/recursion.stderr b/src/test/ui/recursion/recursion.stderr index 17293720a43ea..1a65b0e84f6a3 100644 --- a/src/test/ui/recursion/recursion.stderr +++ b/src/test/ui/recursion/recursion.stderr @@ -1,5 +1,5 @@ error: reached the recursion limit while instantiating `test::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` - --> $DIR/recursion.rs:14:1 + --> $DIR/recursion.rs:15:1 | LL | / fn test (n:isize, i:isize, first:T, second:T) ->isize { LL | | match n { 0 => {first.dot(second)} From f639607f63390ca582f92039b04f3ad710338796 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 20 Jan 2020 15:18:13 -0800 Subject: [PATCH 24/32] Use new dataflow framework for drop elaboration and borrow checking --- src/librustc_mir/borrow_check/flows.rs | 142 ------------ src/librustc_mir/borrow_check/mod.rs | 216 +++++++++--------- src/librustc_mir/borrow_check/nll.rs | 4 +- .../borrow_check/type_check/liveness/mod.rs | 6 +- .../borrow_check/type_check/liveness/trace.rs | 40 ++-- .../borrow_check/type_check/mod.rs | 6 +- src/librustc_mir/dataflow/impls/borrows.rs | 8 +- src/librustc_mir/transform/elaborate_drops.rs | 56 ++--- 8 files changed, 166 insertions(+), 312 deletions(-) delete mode 100644 src/librustc_mir/borrow_check/flows.rs diff --git a/src/librustc_mir/borrow_check/flows.rs b/src/librustc_mir/borrow_check/flows.rs deleted file mode 100644 index 57c544fda0c54..0000000000000 --- a/src/librustc_mir/borrow_check/flows.rs +++ /dev/null @@ -1,142 +0,0 @@ -//! Manages the dataflow bits required for borrowck. -//! -//! FIXME: this might be better as a "generic" fixed-point combinator, -//! but is not as ugly as it is right now. - -use rustc::mir::{BasicBlock, Location}; -use rustc_index::bit_set::BitIter; - -use crate::borrow_check::location::LocationIndex; - -use crate::borrow_check::nll::PoloniusOutput; - -use crate::dataflow::indexes::BorrowIndex; -use crate::dataflow::move_paths::HasMoveData; -use crate::dataflow::Borrows; -use crate::dataflow::EverInitializedPlaces; -use crate::dataflow::MaybeUninitializedPlaces; -use crate::dataflow::{FlowAtLocation, FlowsAtLocation}; -use either::Either; -use std::fmt; -use std::rc::Rc; - -crate struct Flows<'b, 'tcx> { - borrows: FlowAtLocation<'tcx, Borrows<'b, 'tcx>>, - pub uninits: FlowAtLocation<'tcx, MaybeUninitializedPlaces<'b, 'tcx>>, - pub ever_inits: FlowAtLocation<'tcx, EverInitializedPlaces<'b, 'tcx>>, - - /// Polonius Output - pub polonius_output: Option>, -} - -impl<'b, 'tcx> Flows<'b, 'tcx> { - crate fn new( - borrows: FlowAtLocation<'tcx, Borrows<'b, 'tcx>>, - uninits: FlowAtLocation<'tcx, MaybeUninitializedPlaces<'b, 'tcx>>, - ever_inits: FlowAtLocation<'tcx, EverInitializedPlaces<'b, 'tcx>>, - polonius_output: Option>, - ) -> Self { - Flows { borrows, uninits, ever_inits, polonius_output } - } - - crate fn borrows_in_scope( - &self, - location: LocationIndex, - ) -> impl Iterator + '_ { - if let Some(ref polonius) = self.polonius_output { - Either::Left(polonius.errors_at(location).iter().cloned()) - } else { - Either::Right(self.borrows.iter_incoming()) - } - } - - crate fn with_outgoing_borrows(&self, op: impl FnOnce(BitIter<'_, BorrowIndex>)) { - self.borrows.with_iter_outgoing(op) - } -} - -macro_rules! each_flow { - ($this:ident, $meth:ident($arg:ident)) => { - FlowAtLocation::$meth(&mut $this.borrows, $arg); - FlowAtLocation::$meth(&mut $this.uninits, $arg); - FlowAtLocation::$meth(&mut $this.ever_inits, $arg); - }; -} - -impl<'b, 'tcx> FlowsAtLocation for Flows<'b, 'tcx> { - fn reset_to_entry_of(&mut self, bb: BasicBlock) { - each_flow!(self, reset_to_entry_of(bb)); - } - - fn reset_to_exit_of(&mut self, bb: BasicBlock) { - each_flow!(self, reset_to_exit_of(bb)); - } - - fn reconstruct_statement_effect(&mut self, location: Location) { - each_flow!(self, reconstruct_statement_effect(location)); - } - - fn reconstruct_terminator_effect(&mut self, location: Location) { - each_flow!(self, reconstruct_terminator_effect(location)); - } - - fn apply_local_effect(&mut self, location: Location) { - each_flow!(self, apply_local_effect(location)); - } -} - -impl<'b, 'tcx> fmt::Display for Flows<'b, 'tcx> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut s = String::new(); - - s.push_str("borrows in effect: ["); - let mut saw_one = false; - self.borrows.each_state_bit(|borrow| { - if saw_one { - s.push_str(", "); - }; - saw_one = true; - let borrow_data = &self.borrows.operator().borrows()[borrow]; - s.push_str(&borrow_data.to_string()); - }); - s.push_str("] "); - - s.push_str("borrows generated: ["); - let mut saw_one = false; - self.borrows.each_gen_bit(|borrow| { - if saw_one { - s.push_str(", "); - }; - saw_one = true; - let borrow_data = &self.borrows.operator().borrows()[borrow]; - s.push_str(&borrow_data.to_string()); - }); - s.push_str("] "); - - s.push_str("uninits: ["); - let mut saw_one = false; - self.uninits.each_state_bit(|mpi_uninit| { - if saw_one { - s.push_str(", "); - }; - saw_one = true; - let move_path = &self.uninits.operator().move_data().move_paths[mpi_uninit]; - s.push_str(&move_path.to_string()); - }); - s.push_str("] "); - - s.push_str("ever_init: ["); - let mut saw_one = false; - self.ever_inits.each_state_bit(|mpi_ever_init| { - if saw_one { - s.push_str(", "); - }; - saw_one = true; - let ever_init = &self.ever_inits.operator().move_data().inits[mpi_ever_init]; - s.push_str(&format!("{:?}", ever_init)); - }); - s.push_str("]"); - - fmt::Display::fmt(&s, fmt) - } -} diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index e528159fcef17..95e21f3453377 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -4,8 +4,8 @@ use rustc::infer::InferCtxt; use rustc::lint::builtin::MUTABLE_BORROW_RESERVATION_CONFLICT; use rustc::lint::builtin::UNUSED_MUT; use rustc::mir::{ - read_only, Body, BodyAndCache, ClearCrossCrate, Local, Location, Mutability, Operand, Place, - PlaceElem, PlaceRef, ReadOnlyBodyAndCache, + read_only, traversal, Body, BodyAndCache, ClearCrossCrate, Local, Location, Mutability, + Operand, Place, PlaceElem, PlaceRef, ReadOnlyBodyAndCache, }; use rustc::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind}; use rustc::mir::{Field, ProjectionElem, Promoted, Rvalue, Statement, StatementKind}; @@ -21,6 +21,7 @@ use rustc_hir::{def_id::DefId, HirId, Node}; use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; +use either::Either; use smallvec::SmallVec; use std::cell::RefCell; use std::collections::BTreeMap; @@ -30,19 +31,17 @@ use std::rc::Rc; use rustc_span::{Span, DUMMY_SP}; use syntax::ast::Name; +use crate::dataflow; +use crate::dataflow::generic::{Analysis, BorrowckFlowState as Flows, BorrowckResults}; use crate::dataflow::indexes::{BorrowIndex, InitIndex, MoveOutIndex, MovePathIndex}; -use crate::dataflow::move_paths::{HasMoveData, InitLocation, LookupResult, MoveData, MoveError}; +use crate::dataflow::move_paths::{InitLocation, LookupResult, MoveData, MoveError}; use crate::dataflow::Borrows; -use crate::dataflow::DataflowResultsConsumer; use crate::dataflow::EverInitializedPlaces; -use crate::dataflow::FlowAtLocation; use crate::dataflow::MoveDataParamEnv; -use crate::dataflow::{do_dataflow, DebugFormatted}; use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; use crate::transform::MirSource; use self::diagnostics::{AccessKind, RegionName}; -use self::flows::Flows; use self::location::LocationTable; use self::prefixes::PrefixSet; use self::MutateMode::{JustWrite, WriteAndRead}; @@ -54,7 +53,6 @@ mod constraint_generation; mod constraints; mod diagnostics; mod facts; -mod flows; mod invalidation; mod location; mod member_constraints; @@ -70,7 +68,7 @@ mod universal_regions; mod used_muts; crate use borrow_set::{BorrowData, BorrowSet}; -crate use nll::ToRegionVid; +crate use nll::{PoloniusOutput, ToRegionVid}; crate use place_ext::PlaceExt; crate use places_conflict::{places_conflict, PlaceConflictBias}; crate use region_infer::RegionInferenceContext; @@ -115,7 +113,6 @@ fn do_mir_borrowck<'a, 'tcx>( debug!("do_mir_borrowck(def_id = {:?})", def_id); let tcx = infcx.tcx; - let attributes = tcx.get_attrs(def_id); let param_env = tcx.param_env(def_id); let id = tcx.hir().as_local_hir_id(def_id).expect("do_mir_borrowck: non-local DefId"); @@ -188,16 +185,10 @@ fn do_mir_borrowck<'a, 'tcx>( let mdpe = MoveDataParamEnv { move_data, param_env }; - let dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); - let mut flow_inits = FlowAtLocation::new(do_dataflow( - tcx, - &body, - def_id, - &attributes, - &dead_unwinds, - MaybeInitializedPlaces::new(tcx, &body, &mdpe), - |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i]), - )); + let mut flow_inits = MaybeInitializedPlaces::new(tcx, &body, &mdpe) + .into_engine(tcx, &body, def_id) + .iterate_to_fixpoint() + .into_results_cursor(&body); let locals_are_invalidated_at_exit = tcx.hir().body_owner_kind(id).is_fn_or_closure(); let borrow_set = @@ -233,33 +224,15 @@ fn do_mir_borrowck<'a, 'tcx>( let regioncx = Rc::new(regioncx); - let flow_borrows = FlowAtLocation::new(do_dataflow( - tcx, - &body, - def_id, - &attributes, - &dead_unwinds, - Borrows::new(tcx, &body, regioncx.clone(), &borrow_set), - |rs, i| DebugFormatted::new(&rs.location(i)), - )); - let flow_uninits = FlowAtLocation::new(do_dataflow( - tcx, - &body, - def_id, - &attributes, - &dead_unwinds, - MaybeUninitializedPlaces::new(tcx, &body, &mdpe), - |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i]), - )); - let flow_ever_inits = FlowAtLocation::new(do_dataflow( - tcx, - &body, - def_id, - &attributes, - &dead_unwinds, - EverInitializedPlaces::new(tcx, &body, &mdpe), - |bd, i| DebugFormatted::new(&bd.move_data().inits[i]), - )); + let flow_borrows = Borrows::new(tcx, &body, regioncx.clone(), &borrow_set) + .into_engine(tcx, &body, def_id) + .iterate_to_fixpoint(); + let flow_uninits = MaybeUninitializedPlaces::new(tcx, &body, &mdpe) + .into_engine(tcx, &body, def_id) + .iterate_to_fixpoint(); + let flow_ever_inits = EverInitializedPlaces::new(tcx, &body, &mdpe) + .into_engine(tcx, &body, def_id) + .iterate_to_fixpoint(); let movable_generator = match tcx.hir().get(id) { Node::Expr(&hir::Expr { @@ -294,17 +267,28 @@ fn do_mir_borrowck<'a, 'tcx>( local_names, region_names: RefCell::default(), next_region_name: RefCell::new(1), + polonius_output, }; // Compute and report region errors, if any. mbcx.report_region_errors(nll_errors); - let mut state = Flows::new(flow_borrows, flow_uninits, flow_ever_inits, polonius_output); + let results = BorrowckResults { + ever_inits: flow_ever_inits, + uninits: flow_uninits, + borrows: flow_borrows, + }; if let Some(errors) = move_errors { mbcx.report_move_errors(errors); } - mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer + + dataflow::generic::visit_results( + &*body, + traversal::reverse_postorder(&*body).map(|(bb, _)| bb), + &results, + &mut mbcx, + ); // Convert any reservation warnings into lints. let reservation_warnings = mem::take(&mut mbcx.reservation_warnings); @@ -500,6 +484,9 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> { /// The counter for generating new region names. next_region_name: RefCell, + + /// Results of Polonius analysis. + polonius_output: Option>, } // Check that: @@ -507,24 +494,16 @@ crate struct MirBorrowckCtxt<'cx, 'tcx> { // 2. loans made in overlapping scopes do not conflict // 3. assignments do not affect things loaned out as immutable // 4. moves do not affect things loaned out in any way -impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx> { +impl<'cx, 'tcx> dataflow::generic::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx> { type FlowState = Flows<'cx, 'tcx>; - fn body(&self) -> &'cx Body<'tcx> { - *self.body - } - - fn visit_block_entry(&mut self, bb: BasicBlock, flow_state: &Self::FlowState) { - debug!("MirBorrowckCtxt::process_block({:?}): {}", bb, flow_state); - } - - fn visit_statement_entry( + fn visit_statement( &mut self, - location: Location, + flow_state: &Flows<'cx, 'tcx>, stmt: &'cx Statement<'tcx>, - flow_state: &Self::FlowState, + location: Location, ) { - debug!("MirBorrowckCtxt::process_statement({:?}, {:?}): {}", location, stmt, flow_state); + debug!("MirBorrowckCtxt::process_statement({:?}, {:?}): {:?}", location, stmt, flow_state); let span = stmt.source_info.span; self.check_activations(location, span, flow_state); @@ -607,17 +586,16 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx } } - fn visit_terminator_entry( + fn visit_terminator( &mut self, - location: Location, + flow_state: &Flows<'cx, 'tcx>, term: &'cx Terminator<'tcx>, - flow_state: &Self::FlowState, + loc: Location, ) { - let loc = location; - debug!("MirBorrowckCtxt::process_terminator({:?}, {:?}): {}", location, term, flow_state); + debug!("MirBorrowckCtxt::process_terminator({:?}, {:?}): {:?}", loc, term, flow_state); let span = term.source_info.span; - self.check_activations(location, span, flow_state); + self.check_activations(loc, span, flow_state); match term.kind { TerminatorKind::SwitchInt { ref discr, switch_ty: _, values: _, targets: _ } => { @@ -686,19 +664,40 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx TerminatorKind::Yield { ref value, resume: _, ref resume_arg, drop: _ } => { self.consume_operand(loc, (value, span), flow_state); + self.mutate_place(loc, (resume_arg, span), Deep, JustWrite, flow_state); + } + + TerminatorKind::Goto { target: _ } + | TerminatorKind::Abort + | TerminatorKind::Unreachable + | TerminatorKind::Resume + | TerminatorKind::Return + | TerminatorKind::GeneratorDrop + | TerminatorKind::FalseEdges { real_target: _, imaginary_target: _ } + | TerminatorKind::FalseUnwind { real_target: _, unwind: _ } => { + // no data used, thus irrelevant to borrowck + } + } + } + + fn visit_terminator_exit( + &mut self, + flow_state: &Flows<'cx, 'tcx>, + term: &'cx Terminator<'tcx>, + loc: Location, + ) { + let span = term.source_info.span; + match term.kind { + TerminatorKind::Yield { value: _, resume: _, resume_arg: _, drop: _ } => { if self.movable_generator { // Look for any active borrows to locals let borrow_set = self.borrow_set.clone(); - flow_state.with_outgoing_borrows(|borrows| { - for i in borrows { - let borrow = &borrow_set[i]; - self.check_for_local_borrow(borrow, span); - } - }); + for i in flow_state.borrows.iter() { + let borrow = &borrow_set[i]; + self.check_for_local_borrow(borrow, span); + } } - - self.mutate_place(loc, (resume_arg, span), Deep, JustWrite, flow_state); } TerminatorKind::Resume | TerminatorKind::Return | TerminatorKind::GeneratorDrop => { @@ -707,20 +706,13 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx // StorageDead, but we don't always emit those (notably on unwind paths), // so this "extra check" serves as a kind of backup. let borrow_set = self.borrow_set.clone(); - flow_state.with_outgoing_borrows(|borrows| { - for i in borrows { - let borrow = &borrow_set[i]; - self.check_for_invalidation_at_exit(loc, borrow, span); - } - }); - } - TerminatorKind::Goto { target: _ } - | TerminatorKind::Abort - | TerminatorKind::Unreachable - | TerminatorKind::FalseEdges { real_target: _, imaginary_target: _ } - | TerminatorKind::FalseUnwind { real_target: _, unwind: _ } => { - // no data used, thus irrelevant to borrowck + for i in flow_state.borrows.iter() { + let borrow = &borrow_set[i]; + self.check_for_invalidation_at_exit(loc, borrow, span); + } } + + _ => {} } } } @@ -855,6 +847,10 @@ impl InitializationRequiringAction { } impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { + fn body(&self) -> &'cx Body<'tcx> { + *self.body + } + /// Checks an access to the given place to see if it is allowed. Examines the set of borrows /// that are in scope, as well as which paths have been initialized, to ensure that (a) the /// place is initialized and (b) it is not borrowed in some way that would prevent this @@ -934,8 +930,17 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let tcx = self.infcx.tcx; let body = self.body; let body: &Body<'_> = &body; - let location_table = self.location_table.start_index(location); let borrow_set = self.borrow_set.clone(); + + // Use polonius output if it has been enabled. + let polonius_output = self.polonius_output.clone(); + let borrows_in_scope = if let Some(polonius) = &polonius_output { + let location = self.location_table.start_index(location); + Either::Left(polonius.errors_at(location).iter().copied()) + } else { + Either::Right(flow_state.borrows.iter()) + }; + each_borrow_involving_path( self, tcx, @@ -943,7 +948,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { location, (sd, place_span.0), &borrow_set, - flow_state.borrows_in_scope(location_table), + borrows_in_scope, |this, borrow_index, borrow| match (rw, borrow.kind) { // Obviously an activation is compatible with its own // reservation (or even prior activating uses of same @@ -1473,9 +1478,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // initial reservation. } } -} -impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { fn check_if_reassignment_to_immutable_state( &mut self, location: Location, @@ -1565,21 +1568,26 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { location: Location, desired_action: InitializationRequiringAction, place_span: (PlaceRef<'cx, 'tcx>, Span), - maybe_uninits: &FlowAtLocation<'tcx, MaybeUninitializedPlaces<'cx, 'tcx>>, + maybe_uninits: &BitSet, from: u32, to: u32, ) { if let Some(mpi) = self.move_path_for_place(place_span.0) { - let mut child = self.move_data.move_paths[mpi].first_child; + let move_paths = &self.move_data.move_paths; + let mut child = move_paths[mpi].first_child; while let Some(child_mpi) = child { - let child_move_place = &self.move_data.move_paths[child_mpi]; - let child_place = &child_move_place.place; - let last_proj = child_place.projection.last().unwrap(); + let child_move_path = &move_paths[child_mpi]; + let last_proj = child_move_path.place.projection.last().unwrap(); if let ProjectionElem::ConstantIndex { offset, from_end, .. } = last_proj { debug_assert!(!from_end, "Array constant indexing shouldn't be `from_end`."); if (from..to).contains(offset) { - if let Some(uninit_child) = maybe_uninits.has_any_child_of(child_mpi) { + let uninit_child = + self.move_data.find_in_move_path_or_its_descendants(child_mpi, |mpi| { + maybe_uninits.contains(mpi) + }); + + if let Some(uninit_child) = uninit_child { self.report_use_of_moved_or_uninitialized( location, desired_action, @@ -1590,7 +1598,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } } - child = child_move_place.next_sibling; + child = child_move_path.next_sibling; } } } @@ -1651,12 +1659,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { debug!("check_if_path_or_subpath_is_moved place: {:?}", place_span.0); if let Some(mpi) = self.move_path_for_place(place_span.0) { - if let Some(child_mpi) = maybe_uninits.has_any_child_of(mpi) { + let uninit_mpi = self + .move_data + .find_in_move_path_or_its_descendants(mpi, |mpi| maybe_uninits.contains(mpi)); + + if let Some(uninit_mpi) = uninit_mpi { self.report_use_of_moved_or_uninitialized( location, desired_action, (place_span.0, place_span.0, place_span.1), - child_mpi, + uninit_mpi, ); return; // don't bother finding other problems. } diff --git a/src/librustc_mir/borrow_check/nll.rs b/src/librustc_mir/borrow_check/nll.rs index 73718d58346f1..a71dfc9a7780f 100644 --- a/src/librustc_mir/borrow_check/nll.rs +++ b/src/librustc_mir/borrow_check/nll.rs @@ -20,8 +20,8 @@ use std::str::FromStr; use self::mir_util::PassWhere; use polonius_engine::{Algorithm, Output}; +use crate::dataflow::generic::ResultsCursor; use crate::dataflow::move_paths::{InitKind, InitLocation, MoveData}; -use crate::dataflow::FlowAtLocation; use crate::dataflow::MaybeInitializedPlaces; use crate::transform::MirSource; use crate::util as mir_util; @@ -149,7 +149,7 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( promoted: &IndexVec>, location_table: &LocationTable, param_env: ty::ParamEnv<'tcx>, - flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'cx, 'tcx>>, + flow_inits: &mut ResultsCursor<'cx, 'tcx, MaybeInitializedPlaces<'cx, 'tcx>>, move_data: &MoveData<'tcx>, borrow_set: &BorrowSet<'tcx>, ) -> NllOutput<'tcx> { diff --git a/src/librustc_mir/borrow_check/type_check/liveness/mod.rs b/src/librustc_mir/borrow_check/type_check/liveness/mod.rs index e461ce739a7cd..cdf962ee31a6e 100644 --- a/src/librustc_mir/borrow_check/type_check/liveness/mod.rs +++ b/src/librustc_mir/borrow_check/type_check/liveness/mod.rs @@ -3,8 +3,8 @@ use rustc::ty::{RegionVid, TyCtxt}; use rustc_data_structures::fx::FxHashSet; use std::rc::Rc; +use crate::dataflow::generic::ResultsCursor; use crate::dataflow::move_paths::MoveData; -use crate::dataflow::FlowAtLocation; use crate::dataflow::MaybeInitializedPlaces; use crate::borrow_check::{ @@ -30,11 +30,11 @@ mod trace; /// /// N.B., this computation requires normalization; therefore, it must be /// performed before -pub(super) fn generate<'tcx>( +pub(super) fn generate<'mir, 'tcx>( typeck: &mut TypeChecker<'_, 'tcx>, body: ReadOnlyBodyAndCache<'_, 'tcx>, elements: &Rc, - flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'tcx>>, + flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, move_data: &MoveData<'tcx>, location_table: &LocationTable, ) { diff --git a/src/librustc_mir/borrow_check/type_check/liveness/trace.rs b/src/librustc_mir/borrow_check/type_check/liveness/trace.rs index ebb925ae0cb56..198f4b4b42e05 100644 --- a/src/librustc_mir/borrow_check/type_check/liveness/trace.rs +++ b/src/librustc_mir/borrow_check/type_check/liveness/trace.rs @@ -8,9 +8,10 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_index::bit_set::HybridBitSet; use std::rc::Rc; +use crate::dataflow::generic::ResultsCursor; use crate::dataflow::indexes::MovePathIndex; -use crate::dataflow::move_paths::MoveData; -use crate::dataflow::{FlowAtLocation, FlowsAtLocation, MaybeInitializedPlaces}; +use crate::dataflow::move_paths::{HasMoveData, MoveData}; +use crate::dataflow::MaybeInitializedPlaces; use crate::borrow_check::{ region_infer::values::{self, PointIndex, RegionValueElements}, @@ -38,7 +39,7 @@ pub(super) fn trace( typeck: &mut TypeChecker<'_, 'tcx>, body: ReadOnlyBodyAndCache<'_, 'tcx>, elements: &Rc, - flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'tcx>>, + flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, move_data: &MoveData<'tcx>, live_locals: Vec, polonius_drop_used: Option>, @@ -85,7 +86,7 @@ struct LivenessContext<'me, 'typeck, 'flow, 'tcx> { /// Results of dataflow tracking which variables (and paths) have been /// initialized. - flow_inits: &'me mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'flow, 'tcx>>, + flow_inits: &'me mut ResultsCursor<'flow, 'tcx, MaybeInitializedPlaces<'flow, 'tcx>>, /// Index indicating where each variable is assigned, used, or /// dropped. @@ -389,23 +390,26 @@ impl LivenessResults<'me, 'typeck, 'flow, 'tcx> { } impl LivenessContext<'_, '_, '_, 'tcx> { + /// Returns `true` if the local variable (or some part of it) is initialized at the current + /// cursor position. Callers should call one of the `seek` methods immediately before to point + /// the cursor to the desired location. + fn initialized_at_curr_loc(&self, mpi: MovePathIndex) -> bool { + let state = self.flow_inits.get(); + if state.contains(mpi) { + return true; + } + + let move_paths = &self.flow_inits.analysis().move_data().move_paths; + move_paths[mpi].find_descendant(&move_paths, |mpi| state.contains(mpi)).is_some() + } + /// Returns `true` if the local variable (or some part of it) is initialized in /// the terminator of `block`. We need to check this to determine if a /// DROP of some local variable will have an effect -- note that /// drops, as they may unwind, are always terminators. fn initialized_at_terminator(&mut self, block: BasicBlock, mpi: MovePathIndex) -> bool { - // Compute the set of initialized paths at terminator of block - // by resetting to the start of the block and then applying - // the effects of all statements. This is the only way to get - // "just ahead" of a terminator. - self.flow_inits.reset_to_entry_of(block); - for statement_index in 0..self.body[block].statements.len() { - let location = Location { block, statement_index }; - self.flow_inits.reconstruct_statement_effect(location); - self.flow_inits.apply_local_effect(location); - } - - self.flow_inits.has_any_child_of(mpi).is_some() + self.flow_inits.seek_before(self.body.terminator_loc(block)); + self.initialized_at_curr_loc(mpi) } /// Returns `true` if the path `mpi` (or some part of it) is initialized at @@ -414,8 +418,8 @@ impl LivenessContext<'_, '_, '_, 'tcx> { /// **Warning:** Does not account for the result of `Call` /// instructions. fn initialized_at_exit(&mut self, block: BasicBlock, mpi: MovePathIndex) -> bool { - self.flow_inits.reset_to_exit_of(block); - self.flow_inits.has_any_child_of(mpi).is_some() + self.flow_inits.seek_after(self.body.terminator_loc(block)); + self.initialized_at_curr_loc(mpi) } /// Stores the result that all regions in `value` are live for the diff --git a/src/librustc_mir/borrow_check/type_check/mod.rs b/src/librustc_mir/borrow_check/type_check/mod.rs index f645435cdf60f..100fd7dc48d0e 100644 --- a/src/librustc_mir/borrow_check/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/type_check/mod.rs @@ -34,8 +34,8 @@ use rustc_index::vec::{Idx, IndexVec}; use rustc_span::{Span, DUMMY_SP}; use syntax::ast; +use crate::dataflow::generic::ResultsCursor; use crate::dataflow::move_paths::MoveData; -use crate::dataflow::FlowAtLocation; use crate::dataflow::MaybeInitializedPlaces; use crate::transform::promote_consts::should_suggest_const_in_array_repeat_expressions_attribute; @@ -114,7 +114,7 @@ mod relate_tys; /// constraints for the regions in the types of variables /// - `flow_inits` -- results of a maybe-init dataflow analysis /// - `move_data` -- move-data constructed when performing the maybe-init dataflow analysis -pub(crate) fn type_check<'tcx>( +pub(crate) fn type_check<'mir, 'tcx>( infcx: &InferCtxt<'_, 'tcx>, param_env: ty::ParamEnv<'tcx>, body: ReadOnlyBodyAndCache<'_, 'tcx>, @@ -124,7 +124,7 @@ pub(crate) fn type_check<'tcx>( location_table: &LocationTable, borrow_set: &BorrowSet<'tcx>, all_facts: &mut Option, - flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'tcx>>, + flow_inits: &mut ResultsCursor<'mir, 'tcx, MaybeInitializedPlaces<'mir, 'tcx>>, move_data: &MoveData<'tcx>, elements: &Rc, ) -> MirTypeckResults<'tcx> { diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 151ae28bae255..74d1094f9645e 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -4,11 +4,9 @@ use rustc::ty::TyCtxt; use rustc_data_structures::fx::FxHashMap; use rustc_index::bit_set::BitSet; -use rustc_index::vec::IndexVec; use crate::borrow_check::{ - places_conflict, BorrowData, BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext, - ToRegionVid, + places_conflict, BorrowSet, PlaceConflictBias, PlaceExt, RegionInferenceContext, ToRegionVid, }; use crate::dataflow::generic::{self, GenKill}; use crate::dataflow::BottomValue; @@ -161,10 +159,6 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { } } - crate fn borrows(&self) -> &IndexVec> { - &self.borrow_set.borrows - } - pub fn location(&self, idx: BorrowIndex) -> &Location { &self.borrow_set.borrows[idx].reserve_location } diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index 1c0b1b8c13767..319b6f35f1127 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -1,7 +1,7 @@ -use crate::dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex}; -use crate::dataflow::DataflowResults; +use crate::dataflow; +use crate::dataflow::generic::{Analysis, Results}; +use crate::dataflow::move_paths::{LookupResult, MoveData, MovePathIndex}; use crate::dataflow::MoveDataParamEnv; -use crate::dataflow::{self, do_dataflow, DebugFormatted}; use crate::dataflow::{drop_flag_effects_for_location, on_lookup_result_bits}; use crate::dataflow::{on_all_children_bits, on_all_drop_children_bits}; use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; @@ -40,24 +40,16 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { let body = &*body; let env = MoveDataParamEnv { move_data, param_env }; let dead_unwinds = find_dead_unwinds(tcx, body, def_id, &env); - let flow_inits = do_dataflow( - tcx, - body, - def_id, - &[], - &dead_unwinds, - MaybeInitializedPlaces::new(tcx, body, &env), - |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p]), - ); - let flow_uninits = do_dataflow( - tcx, - body, - def_id, - &[], - &dead_unwinds, - MaybeUninitializedPlaces::new(tcx, body, &env), - |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p]), - ); + + let flow_inits = MaybeInitializedPlaces::new(tcx, body, &env) + .into_engine(tcx, body, def_id) + .dead_unwinds(&dead_unwinds) + .iterate_to_fixpoint(); + + let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &env) + .into_engine(tcx, body, def_id) + .dead_unwinds(&dead_unwinds) + .iterate_to_fixpoint(); ElaborateDropsCtxt { tcx, @@ -87,15 +79,9 @@ fn find_dead_unwinds<'tcx>( // We only need to do this pass once, because unwind edges can only // reach cleanup blocks, which can't have unwind edges themselves. let mut dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); - let flow_inits = do_dataflow( - tcx, - body, - def_id, - &[], - &dead_unwinds, - MaybeInitializedPlaces::new(tcx, body, &env), - |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p]), - ); + let flow_inits = MaybeInitializedPlaces::new(tcx, body, &env) + .into_engine(tcx, body, def_id) + .iterate_to_fixpoint(); for (bb, bb_data) in body.basic_blocks().iter_enumerated() { let location = match bb_data.terminator().kind { TerminatorKind::Drop { ref location, unwind: Some(_), .. } @@ -104,7 +90,7 @@ fn find_dead_unwinds<'tcx>( }; let mut init_data = InitializationData { - live: flow_inits.sets().entry_set_for(bb.index()).to_owned(), + live: flow_inits.entry_set_for_block(bb).clone(), dead: BitSet::new_empty(env.move_data.move_paths.len()), }; debug!("find_dead_unwinds @ {:?}: {:?}; init_data={:?}", bb, bb_data, init_data.live); @@ -283,8 +269,8 @@ struct ElaborateDropsCtxt<'a, 'tcx> { tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, env: &'a MoveDataParamEnv<'tcx>, - flow_inits: DataflowResults<'tcx, MaybeInitializedPlaces<'a, 'tcx>>, - flow_uninits: DataflowResults<'tcx, MaybeUninitializedPlaces<'a, 'tcx>>, + flow_inits: Results<'tcx, MaybeInitializedPlaces<'a, 'tcx>>, + flow_uninits: Results<'tcx, MaybeUninitializedPlaces<'a, 'tcx>>, drop_flags: FxHashMap, patch: MirPatch<'tcx>, } @@ -300,8 +286,8 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { fn initialization_data_at(&self, loc: Location) -> InitializationData { let mut data = InitializationData { - live: self.flow_inits.sets().entry_set_for(loc.block.index()).to_owned(), - dead: self.flow_uninits.sets().entry_set_for(loc.block.index()).to_owned(), + live: self.flow_inits.entry_set_for_block(loc.block).to_owned(), + dead: self.flow_uninits.entry_set_for_block(loc.block).to_owned(), }; for stmt in 0..loc.statement_index { data.apply_location( From 42d19a4e18d96bca6c56770a549eeda386d5b189 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 20 Jan 2020 15:16:24 -0800 Subject: [PATCH 25/32] Use new dataflow framework for `rustc_peek` tests --- src/librustc_mir/transform/rustc_peek.rs | 70 ++++++++----------- .../mir-dataflow/indirect-mutation-offset.rs | 2 + 2 files changed, 30 insertions(+), 42 deletions(-) diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index 7a90cb0390e52..7d8506eb28105 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -9,11 +9,9 @@ use rustc::ty::{self, Ty, TyCtxt}; use rustc_hir::def_id::DefId; use rustc_index::bit_set::BitSet; +use crate::dataflow::generic::{Analysis, Results, ResultsCursor}; use crate::dataflow::move_paths::{HasMoveData, MoveData}; use crate::dataflow::move_paths::{LookupResult, MovePathIndex}; -use crate::dataflow::BitDenotation; -use crate::dataflow::DataflowResults; -use crate::dataflow::DataflowResultsCursor; use crate::dataflow::IndirectlyMutableLocals; use crate::dataflow::MoveDataParamEnv; use crate::dataflow::{do_dataflow, DebugFormatted}; @@ -21,12 +19,12 @@ use crate::dataflow::{ DefinitelyInitializedPlaces, MaybeInitializedPlaces, MaybeUninitializedPlaces, }; -use crate::dataflow::has_rustc_mir_with; - pub struct SanityCheck; impl<'tcx> MirPass<'tcx> for SanityCheck { fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut BodyAndCache<'tcx>) { + use crate::dataflow::has_rustc_mir_with; + let def_id = src.def_id(); if !tcx.has_attr(def_id, sym::rustc_mir) { debug!("skipping rustc_peek::SanityCheck on {}", tcx.def_path_str(def_id)); @@ -40,34 +38,17 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { let move_data = MoveData::gather_moves(body, tcx, param_env).unwrap(); let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env }; let dead_unwinds = BitSet::new_empty(body.basic_blocks().len()); - let flow_inits = do_dataflow( - tcx, - body, - def_id, - &attributes, - &dead_unwinds, - MaybeInitializedPlaces::new(tcx, body, &mdpe), - |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i]), - ); - let flow_uninits = do_dataflow( - tcx, - body, - def_id, - &attributes, - &dead_unwinds, - MaybeUninitializedPlaces::new(tcx, body, &mdpe), - |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i]), - ); - let flow_def_inits = do_dataflow( - tcx, - body, - def_id, - &attributes, - &dead_unwinds, - DefinitelyInitializedPlaces::new(tcx, body, &mdpe), - |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i]), - ); - let flow_indirectly_mut = do_dataflow( + + let flow_inits = MaybeInitializedPlaces::new(tcx, body, &mdpe) + .into_engine(tcx, body, def_id) + .iterate_to_fixpoint(); + let flow_uninits = MaybeUninitializedPlaces::new(tcx, body, &mdpe) + .into_engine(tcx, body, def_id) + .iterate_to_fixpoint(); + let flow_def_inits = DefinitelyInitializedPlaces::new(tcx, body, &mdpe) + .into_engine(tcx, body, def_id) + .iterate_to_fixpoint(); + let _flow_indirectly_mut = do_dataflow( tcx, body, def_id, @@ -86,9 +67,12 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { if has_rustc_mir_with(&attributes, sym::rustc_peek_definite_init).is_some() { sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_def_inits); } + // FIXME: Uncomment these as analyses are migrated to the new framework + /* if has_rustc_mir_with(&attributes, sym::rustc_peek_indirectly_mutable).is_some() { sanity_check_via_rustc_peek(tcx, body, def_id, &attributes, &flow_indirectly_mut); } + */ if has_rustc_mir_with(&attributes, sym::stop_after_dataflow).is_some() { tcx.sess.fatal("stop_after_dataflow ended compilation"); } @@ -111,18 +95,18 @@ impl<'tcx> MirPass<'tcx> for SanityCheck { /// (If there are any calls to `rustc_peek` that do not match the /// expression form above, then that emits an error as well, but those /// errors are not intended to be used for unit tests.) -pub fn sanity_check_via_rustc_peek<'tcx, O>( +pub fn sanity_check_via_rustc_peek<'tcx, A>( tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, _attributes: &[ast::Attribute], - results: &DataflowResults<'tcx, O>, + results: &Results<'tcx, A>, ) where - O: RustcPeekAt<'tcx>, + A: RustcPeekAt<'tcx>, { debug!("sanity_check_via_rustc_peek def_id: {:?}", def_id); - let mut cursor = DataflowResultsCursor::new(results, body); + let mut cursor = ResultsCursor::new(body, results); let peek_calls = body.basic_blocks().iter_enumerated().filter_map(|(bb, block_data)| { PeekCall::from_terminator(tcx, block_data.terminator()).map(|call| (bb, block_data, call)) @@ -153,9 +137,9 @@ pub fn sanity_check_via_rustc_peek<'tcx, O>( | (PeekCallKind::ByVal, mir::Rvalue::Use(mir::Operand::Move(place))) | (PeekCallKind::ByVal, mir::Rvalue::Use(mir::Operand::Copy(place))) => { let loc = Location { block: bb, statement_index }; - cursor.seek(loc); + cursor.seek_before(loc); let state = cursor.get(); - results.operator().peek_at(tcx, place, state, call); + results.analysis.peek_at(tcx, place, state, call); } _ => { @@ -255,7 +239,7 @@ impl PeekCall { } } -pub trait RustcPeekAt<'tcx>: BitDenotation<'tcx> { +pub trait RustcPeekAt<'tcx>: Analysis<'tcx> { fn peek_at( &self, tcx: TyCtxt<'tcx>, @@ -265,9 +249,9 @@ pub trait RustcPeekAt<'tcx>: BitDenotation<'tcx> { ); } -impl<'tcx, O> RustcPeekAt<'tcx> for O +impl<'tcx, A> RustcPeekAt<'tcx> for A where - O: BitDenotation<'tcx, Idx = MovePathIndex> + HasMoveData<'tcx>, + A: Analysis<'tcx, Idx = MovePathIndex> + HasMoveData<'tcx>, { fn peek_at( &self, @@ -292,6 +276,7 @@ where } } +/* FIXME: Add this back once `IndirectlyMutableLocals` uses the new dataflow framework. impl<'tcx> RustcPeekAt<'tcx> for IndirectlyMutableLocals<'_, 'tcx> { fn peek_at( &self, @@ -313,3 +298,4 @@ impl<'tcx> RustcPeekAt<'tcx> for IndirectlyMutableLocals<'_, 'tcx> { } } } +*/ diff --git a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs index 8087a3e139915..884c83b66163c 100644 --- a/src/test/ui/mir-dataflow/indirect-mutation-offset.rs +++ b/src/test/ui/mir-dataflow/indirect-mutation-offset.rs @@ -1,5 +1,7 @@ // compile-flags: -Zunleash-the-miri-inside-of-you +// ignore-test Temporarily ignored while this analysis is migrated to the new framework. + #![feature(core_intrinsics, rustc_attrs, const_raw_ptr_deref)] use std::cell::UnsafeCell; From 5860e78ce9b5926de2d856d2dc8e4f3790584b81 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Mon, 20 Jan 2020 15:15:37 -0800 Subject: [PATCH 26/32] Skip caching block transfer functions for acyclic MIR --- src/librustc_mir/dataflow/generic/engine.rs | 24 ++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/dataflow/generic/engine.rs b/src/librustc_mir/dataflow/generic/engine.rs index 718c1e9ae2043..b81f0adc2015b 100644 --- a/src/librustc_mir/dataflow/generic/engine.rs +++ b/src/librustc_mir/dataflow/generic/engine.rs @@ -44,15 +44,20 @@ where def_id: DefId, analysis: A, ) -> Self { + // If there are no back-edges in the control-flow graph, we only ever need to apply the + // transfer function for each block exactly once (assuming that we process blocks in RPO). + // + // In this case, there's no need to compute the block transfer functions ahead of time. + if !body.is_cfg_cyclic() { + return Self::new(tcx, body, def_id, analysis, None); + } + + // Otherwise, compute and store the cumulative transfer function for each block. + let bits_per_block = analysis.bits_per_block(body); let mut trans_for_block = IndexVec::from_elem(GenKillSet::identity(bits_per_block), body.basic_blocks()); - // Compute cumulative block transfer functions. - // - // FIXME: we may want to skip this if the MIR is acyclic, since we will never access a - // block transfer function more than once. - for (block, block_data) in body.basic_blocks().iter_enumerated() { let trans = &mut trans_for_block[block]; @@ -62,11 +67,10 @@ where analysis.statement_effect(trans, statement, loc); } - if let Some(terminator) = &block_data.terminator { - let loc = Location { block, statement_index: block_data.statements.len() }; - analysis.before_terminator_effect(trans, terminator, loc); - analysis.terminator_effect(trans, terminator, loc); - } + let terminator = block_data.terminator(); + let loc = Location { block, statement_index: block_data.statements.len() }; + analysis.before_terminator_effect(trans, terminator, loc); + analysis.terminator_effect(trans, terminator, loc); } Self::new(tcx, body, def_id, analysis, Some(trans_for_block)) From 168ca9a325e9db2fa613cb6b442013c10a233267 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 21 Jan 2020 14:01:32 -0800 Subject: [PATCH 27/32] Add note about `elaborate_drops::InitializationData` --- src/librustc_mir/transform/elaborate_drops.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index 319b6f35f1127..99f13f68cbc79 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -284,6 +284,9 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { self.env.param_env } + // FIXME(ecstaticmorse): This duplicates `dataflow::ResultsCursor` but hardcodes the transfer + // function for `Maybe{Un,}InitializedPlaces` directly. It should be replaced by a a pair of + // `ResultsCursor`s. fn initialization_data_at(&self, loc: Location) -> InitializationData { let mut data = InitializationData { live: self.flow_inits.entry_set_for_block(loc.block).to_owned(), From 3ac920ffcd3a62fd0c1d5942c54629f430367d8f Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 11 Feb 2020 12:04:46 -0800 Subject: [PATCH 28/32] Use exhaustive matching --- src/librustc_mir/borrow_check/mod.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 95e21f3453377..5bb7a3785c796 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -712,7 +712,16 @@ impl<'cx, 'tcx> dataflow::generic::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt } } - _ => {} + TerminatorKind::Abort + | TerminatorKind::Assert { .. } + | TerminatorKind::Call { .. } + | TerminatorKind::Drop { .. } + | TerminatorKind::DropAndReplace { .. } + | TerminatorKind::FalseEdges { real_target: _, imaginary_target: _ } + | TerminatorKind::FalseUnwind { real_target: _, unwind: _ } + | TerminatorKind::Goto { .. } + | TerminatorKind::SwitchInt { .. } + | TerminatorKind::Unreachable => {} } } } From 5f40fe96a42d3590b804264c029fb02d0a4b065c Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Tue, 11 Feb 2020 12:13:03 -0800 Subject: [PATCH 29/32] Clarify why you shouldn't override `Analysis::into_engine` --- src/librustc_mir/dataflow/generic/mod.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/dataflow/generic/mod.rs b/src/librustc_mir/dataflow/generic/mod.rs index d961ba7fffa25..ea643042c5f86 100644 --- a/src/librustc_mir/dataflow/generic/mod.rs +++ b/src/librustc_mir/dataflow/generic/mod.rs @@ -172,10 +172,18 @@ pub trait Analysis<'tcx>: AnalysisDomain<'tcx> { return_place: &mir::Place<'tcx>, ); - /// Creates an `Engine` to find the fixpoint for this dataflow problem. + /// Calls the appropriate `Engine` constructor to find the fixpoint for this dataflow problem. /// - /// This is functionally equivalent to calling the appropriate `Engine` constructor. It should - /// not be overridden. Its purpose is to allow consumers of this API to use method-chaining. + /// You shouldn't need to override this outside this module, since the combination of the + /// default impl and the one for all `A: GenKillAnalysis` will do the right thing. + /// Its purpose is to enable method chaining like so: + /// + /// ```ignore(cross-crate-imports) + /// let results = MyAnalysis::new(tcx, body) + /// .into_engine(tcx, body, def_id) + /// .iterate_to_fixpoint() + /// .into_results_cursor(body); + /// ``` fn into_engine( self, tcx: TyCtxt<'tcx>, From 8e26ad0c2c5c755f927ac971a3c1ab51de45f64f Mon Sep 17 00:00:00 2001 From: Giles Cope Date: Tue, 11 Feb 2020 20:36:36 +0000 Subject: [PATCH 30/32] Keyword docs Co-Authored-By: Mazdak Farrokhzad Co-Authored-By: Tim Robinson Co-Authored-By: Peter Todd Co-Authored-By: Dylan DPC --- src/libstd/keyword_docs.rs | 49 ++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index 7901c8197b587..1d192cfddeb97 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -1100,10 +1100,28 @@ mod trait_keyword {} // /// A value of type [`bool`] representing logical **true**. /// -/// The documentation for this keyword is [not yet complete]. Pull requests welcome! +/// Logically `true` is not equal to [`false`]. +/// +/// ## Control structures that check for **true** +/// +/// Several of Rust's control structures will check for a `bool` condition evaluating to **true**. +/// +/// * The condition in an [`if`] expression must be of type `bool`. +/// Whenever that condition evaluates to **true**, the `if` expression takes +/// on the value of the first block. If however, the condition evaluates +/// to `false`, the expression takes on value of the `else` block if there is one. /// +/// * [`while`] is another control flow construct expecting a `bool`-typed condition. +/// As long as the condition evaluates to **true**, the `while` loop will continually +/// evaluate its associated block. +/// +/// * [`match`] arms can have guard clauses on them. +/// +/// [`if`]: keyword.if.html +/// [`while`]: keyword.while.html +/// [`match`]: ../reference/expressions/match-expr.html#match-guards +/// [`false`]: keyword.false.html /// [`bool`]: primitive.bool.html -/// [not yet complete]: https://github.com/rust-lang/rust/issues/34601 mod true_keyword {} #[doc(keyword = "type")] @@ -1167,12 +1185,33 @@ mod await_keyword {} #[doc(keyword = "dyn")] // -/// Name the type of a [trait object]. +/// `dyn` is a prefix of a [trait object]'s type. /// -/// The documentation for this keyword is [not yet complete]. Pull requests welcome! +/// The `dyn` keyword is used to highlight that calls to methods on the associated `Trait` +/// are dynamically dispatched. To use the trait this way, it must be 'object safe'. +/// +/// Unlike generic parameters or `impl Trait`, the compiler does not know the concrete type that +/// is being passed. That is, the type has been [erased]. +/// As such, a `dyn Trait` reference contains _two_ pointers. +/// One pointer goes to the data (e.g., an instance of a struct). +/// Another pointer goes to a map of method call names to function pointers +/// (known as a virtual method table or vtable). +/// +/// At run-time, when a method needs to be called on the `dyn Trait`, the vtable is consulted to get +/// the function pointer and then that function pointer is called. +/// +/// ## Trade-offs +/// +/// The above indirection is the additional runtime cost of calling a function on a `dyn Trait`. +/// Methods called by dynamic dispatch generally cannot be inlined by the compiler. +/// +/// However, `dyn Trait` is likely to produce smaller code than `impl Trait` / generic parameters as +/// the method won't be duplicated for each concrete type. +/// +/// Read more about `object safety` and [trait object]s. /// /// [trait object]: ../book/ch17-02-trait-objects.html -/// [not yet complete]: https://github.com/rust-lang/rust/issues/34601 +/// [erased]: https://en.wikipedia.org/wiki/Type_erasure mod dyn_keyword {} #[doc(keyword = "union")] From 683ebc2dec0a5b88eb3eaf146e6855ea299d17b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 5 Feb 2020 21:08:07 -0800 Subject: [PATCH 31/32] On mismatched argument count point at arguments --- src/librustc_typeck/check/mod.rs | 54 ++++++++++++++++--- src/test/ui/arg-count-mismatch.rs | 2 +- src/test/ui/arg-count-mismatch.stderr | 6 ++- src/test/ui/c-variadic/variadic-ffi-1.rs | 4 +- src/test/ui/c-variadic/variadic-ffi-1.stderr | 12 +++-- src/test/ui/error-codes/E0057.stderr | 12 +++-- src/test/ui/error-codes/E0060.rs | 2 +- src/test/ui/error-codes/E0060.stderr | 6 ++- src/test/ui/error-codes/E0061.rs | 4 +- src/test/ui/error-codes/E0061.stderr | 12 +++-- src/test/ui/hrtb/issue-58451.rs | 2 +- src/test/ui/hrtb/issue-58451.stderr | 6 ++- src/test/ui/issues/issue-16939.stderr | 6 ++- src/test/ui/issues/issue-18819.stderr | 6 ++- src/test/ui/issues/issue-26094.rs | 8 +-- src/test/ui/issues/issue-26094.stderr | 10 ++-- src/test/ui/issues/issue-3044.rs | 2 +- src/test/ui/issues/issue-3044.stderr | 10 ++-- src/test/ui/issues/issue-4935.rs | 2 +- src/test/ui/issues/issue-4935.stderr | 6 ++- src/test/ui/methods/method-call-err-msg.rs | 8 +-- .../ui/methods/method-call-err-msg.stderr | 39 ++++++++++---- .../mismatched_types/overloaded-calls-bad.rs | 4 +- .../overloaded-calls-bad.stderr | 12 +++-- src/test/ui/not-enough-arguments.rs | 2 +- src/test/ui/not-enough-arguments.stderr | 6 ++- .../ui/resolve/resolve-primitive-fallback.rs | 2 +- .../resolve/resolve-primitive-fallback.stderr | 6 ++- src/test/ui/span/E0057.stderr | 12 +++-- src/test/ui/span/issue-34264.stderr | 12 +++-- src/test/ui/span/missing-unit-argument.stderr | 28 +++++----- ...ant-priority-higher-than-other-inherent.rs | 2 +- ...priority-higher-than-other-inherent.stderr | 6 ++- .../type-ascription-instead-of-initializer.rs | 2 +- ...e-ascription-instead-of-initializer.stderr | 6 ++- 35 files changed, 214 insertions(+), 105 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 7e5d27d93b3cb..d2bef80ac365e 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3761,17 +3761,58 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { error_code: &str, c_variadic: bool, sugg_unit: bool| { + let (span, start_span, args) = match &expr.kind { + hir::ExprKind::Call(hir::Expr { span, .. }, args) => (*span, *span, &args[..]), + hir::ExprKind::MethodCall(path_segment, span, args) => ( + *span, + // `sp` doesn't point at the whole `foo.bar()`, only at `bar`. + path_segment + .args + .and_then(|args| args.args.iter().last()) + // Account for `foo.bar::()`. + .map(|arg| { + // Skip the closing `>`. + tcx.sess + .source_map() + .next_point(tcx.sess.source_map().next_point(arg.span())) + }) + .unwrap_or(*span), + &args[1..], // Skip the receiver. + ), + k => span_bug!(sp, "checking argument types on a non-call: `{:?}`", k), + }; + let arg_spans = if args.is_empty() { + // foo() + // ^^^-- supplied 0 arguments + // | + // expected 2 arguments + vec![tcx.sess.source_map().next_point(start_span).with_hi(sp.hi())] + } else { + // foo(1, 2, 3) + // ^^^ - - - supplied 3 arguments + // | + // expected 2 arguments + args.iter().map(|arg| arg.span).collect::>() + }; + let mut err = tcx.sess.struct_span_err_with_code( - sp, + span, &format!( "this function takes {}{} but {} {} supplied", if c_variadic { "at least " } else { "" }, - potentially_plural_count(expected_count, "parameter"), - potentially_plural_count(arg_count, "parameter"), + potentially_plural_count(expected_count, "argument"), + potentially_plural_count(arg_count, "argument"), if arg_count == 1 { "was" } else { "were" } ), DiagnosticId::Error(error_code.to_owned()), ); + let label = format!("supplied {}", potentially_plural_count(arg_count, "argument")); + for (i, span) in arg_spans.into_iter().enumerate() { + err.span_label( + span, + if arg_count == 0 || i + 1 == arg_count { &label } else { "" }, + ); + } if let Some(def_s) = def_span.map(|sp| tcx.sess.source_map().def_span(sp)) { err.span_label(def_s, "defined here"); @@ -3788,11 +3829,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } else { err.span_label( - sp, + span, format!( "expected {}{}", if c_variadic { "at least " } else { "" }, - potentially_plural_count(expected_count, "parameter") + potentially_plural_count(expected_count, "argument") ), ); } @@ -5494,8 +5535,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.tcx.sess.span_err( span, - "this function can only be invoked \ - directly, not through a function pointer", + "this function can only be invoked directly, not through a function pointer", ); } diff --git a/src/test/ui/arg-count-mismatch.rs b/src/test/ui/arg-count-mismatch.rs index cf7487069c14a..18926f5daf71a 100644 --- a/src/test/ui/arg-count-mismatch.rs +++ b/src/test/ui/arg-count-mismatch.rs @@ -1,4 +1,4 @@ -// error-pattern: parameters were supplied +// error-pattern: arguments were supplied fn f(x: isize) { } diff --git a/src/test/ui/arg-count-mismatch.stderr b/src/test/ui/arg-count-mismatch.stderr index 44f160413636a..7bc06134a690d 100644 --- a/src/test/ui/arg-count-mismatch.stderr +++ b/src/test/ui/arg-count-mismatch.stderr @@ -1,11 +1,13 @@ -error[E0061]: this function takes 1 parameter but 0 parameters were supplied +error[E0061]: this function takes 1 argument but 0 arguments were supplied --> $DIR/arg-count-mismatch.rs:5:28 | LL | fn f(x: isize) { } | -------------- defined here LL | LL | fn main() { let i: (); i = f(); } - | ^^^ expected 1 parameter + | ^-- supplied 0 arguments + | | + | expected 1 argument error: aborting due to previous error diff --git a/src/test/ui/c-variadic/variadic-ffi-1.rs b/src/test/ui/c-variadic/variadic-ffi-1.rs index 6a3ff24b6740f..e7197a9d16859 100644 --- a/src/test/ui/c-variadic/variadic-ffi-1.rs +++ b/src/test/ui/c-variadic/variadic-ffi-1.rs @@ -13,8 +13,8 @@ extern "C" fn bar(f: isize, x: u8) {} fn main() { unsafe { - foo(); //~ ERROR this function takes at least 2 parameters but 0 parameters were supplied - foo(1); //~ ERROR this function takes at least 2 parameters but 1 parameter was supplied + foo(); //~ ERROR this function takes at least 2 arguments but 0 arguments were supplied + foo(1); //~ ERROR this function takes at least 2 arguments but 1 argument was supplied let x: unsafe extern "C" fn(f: isize, x: u8) = foo; //~ ERROR mismatched types let y: extern "C" fn(f: isize, x: u8, ...) = bar; //~ ERROR mismatched types diff --git a/src/test/ui/c-variadic/variadic-ffi-1.stderr b/src/test/ui/c-variadic/variadic-ffi-1.stderr index 05d839a0c5507..318b8aabafb49 100644 --- a/src/test/ui/c-variadic/variadic-ffi-1.stderr +++ b/src/test/ui/c-variadic/variadic-ffi-1.stderr @@ -4,23 +4,27 @@ error[E0045]: C-variadic function must have C or cdecl calling convention LL | fn printf(_: *const u8, ...); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C-variadics require C or cdecl calling convention -error[E0060]: this function takes at least 2 parameters but 0 parameters were supplied +error[E0060]: this function takes at least 2 arguments but 0 arguments were supplied --> $DIR/variadic-ffi-1.rs:16:9 | LL | fn foo(f: isize, x: u8, ...); | ----------------------------- defined here ... LL | foo(); - | ^^^^^ expected at least 2 parameters + | ^^^-- supplied 0 arguments + | | + | expected at least 2 arguments -error[E0060]: this function takes at least 2 parameters but 1 parameter was supplied +error[E0060]: this function takes at least 2 arguments but 1 argument was supplied --> $DIR/variadic-ffi-1.rs:17:9 | LL | fn foo(f: isize, x: u8, ...); | ----------------------------- defined here ... LL | foo(1); - | ^^^^^^ expected at least 2 parameters + | ^^^ - supplied 1 argument + | | + | expected at least 2 arguments error[E0308]: mismatched types --> $DIR/variadic-ffi-1.rs:19:56 diff --git a/src/test/ui/error-codes/E0057.stderr b/src/test/ui/error-codes/E0057.stderr index 6b5890cac36c5..31579e2828964 100644 --- a/src/test/ui/error-codes/E0057.stderr +++ b/src/test/ui/error-codes/E0057.stderr @@ -1,14 +1,18 @@ -error[E0057]: this function takes 1 parameter but 0 parameters were supplied +error[E0057]: this function takes 1 argument but 0 arguments were supplied --> $DIR/E0057.rs:3:13 | LL | let a = f(); - | ^^^ expected 1 parameter + | ^-- supplied 0 arguments + | | + | expected 1 argument -error[E0057]: this function takes 1 parameter but 2 parameters were supplied +error[E0057]: this function takes 1 argument but 2 arguments were supplied --> $DIR/E0057.rs:5:13 | LL | let c = f(2, 3); - | ^^^^^^^ expected 1 parameter + | ^ - - supplied 2 arguments + | | + | expected 1 argument error: aborting due to 2 previous errors diff --git a/src/test/ui/error-codes/E0060.rs b/src/test/ui/error-codes/E0060.rs index 2bb490fb3eabf..941eb2a210bf3 100644 --- a/src/test/ui/error-codes/E0060.rs +++ b/src/test/ui/error-codes/E0060.rs @@ -5,5 +5,5 @@ extern "C" { fn main() { unsafe { printf(); } //~^ ERROR E0060 - //~| expected at least 1 parameter + //~| expected at least 1 argument } diff --git a/src/test/ui/error-codes/E0060.stderr b/src/test/ui/error-codes/E0060.stderr index 8a2e7d1a72cd7..a600592c6c2e7 100644 --- a/src/test/ui/error-codes/E0060.stderr +++ b/src/test/ui/error-codes/E0060.stderr @@ -1,11 +1,13 @@ -error[E0060]: this function takes at least 1 parameter but 0 parameters were supplied +error[E0060]: this function takes at least 1 argument but 0 arguments were supplied --> $DIR/E0060.rs:6:14 | LL | fn printf(_: *const u8, ...) -> u32; | ------------------------------------ defined here ... LL | unsafe { printf(); } - | ^^^^^^^^ expected at least 1 parameter + | ^^^^^^-- supplied 0 arguments + | | + | expected at least 1 argument error: aborting due to previous error diff --git a/src/test/ui/error-codes/E0061.rs b/src/test/ui/error-codes/E0061.rs index e64ea36ac9255..c7b5fe4310e93 100644 --- a/src/test/ui/error-codes/E0061.rs +++ b/src/test/ui/error-codes/E0061.rs @@ -5,9 +5,9 @@ fn f2(a: u16) {} fn main() { f(0); //~^ ERROR E0061 - //~| expected 2 parameters + //~| expected 2 arguments f2(); //~^ ERROR E0061 - //~| expected 1 parameter + //~| expected 1 argument } diff --git a/src/test/ui/error-codes/E0061.stderr b/src/test/ui/error-codes/E0061.stderr index 73103241f7a7d..dfefa0df31332 100644 --- a/src/test/ui/error-codes/E0061.stderr +++ b/src/test/ui/error-codes/E0061.stderr @@ -1,20 +1,24 @@ -error[E0061]: this function takes 2 parameters but 1 parameter was supplied +error[E0061]: this function takes 2 arguments but 1 argument was supplied --> $DIR/E0061.rs:6:5 | LL | fn f(a: u16, b: &str) {} | --------------------- defined here ... LL | f(0); - | ^^^^ expected 2 parameters + | ^ - supplied 1 argument + | | + | expected 2 arguments -error[E0061]: this function takes 1 parameter but 0 parameters were supplied +error[E0061]: this function takes 1 argument but 0 arguments were supplied --> $DIR/E0061.rs:10:5 | LL | fn f2(a: u16) {} | ------------- defined here ... LL | f2(); - | ^^^^ expected 1 parameter + | ^^-- supplied 0 arguments + | | + | expected 1 argument error: aborting due to 2 previous errors diff --git a/src/test/ui/hrtb/issue-58451.rs b/src/test/ui/hrtb/issue-58451.rs index 229e505767879..f36d549e476b8 100644 --- a/src/test/ui/hrtb/issue-58451.rs +++ b/src/test/ui/hrtb/issue-58451.rs @@ -9,5 +9,5 @@ where {} fn main() { - f(&[f()]); //~ ERROR this function takes 1 parameter + f(&[f()]); //~ ERROR this function takes 1 argument } diff --git a/src/test/ui/hrtb/issue-58451.stderr b/src/test/ui/hrtb/issue-58451.stderr index 4648c0182b98e..c0915808bf523 100644 --- a/src/test/ui/hrtb/issue-58451.stderr +++ b/src/test/ui/hrtb/issue-58451.stderr @@ -1,4 +1,4 @@ -error[E0061]: this function takes 1 parameter but 0 parameters were supplied +error[E0061]: this function takes 1 argument but 0 arguments were supplied --> $DIR/issue-58451.rs:12:9 | LL | / fn f(i: I) @@ -9,7 +9,9 @@ LL | | {} | |__- defined here ... LL | f(&[f()]); - | ^^^ expected 1 parameter + | ^-- supplied 0 arguments + | | + | expected 1 argument error: aborting due to previous error diff --git a/src/test/ui/issues/issue-16939.stderr b/src/test/ui/issues/issue-16939.stderr index 5df2119c14120..103f56fa04dd3 100644 --- a/src/test/ui/issues/issue-16939.stderr +++ b/src/test/ui/issues/issue-16939.stderr @@ -1,8 +1,10 @@ -error[E0057]: this function takes 0 parameters but 1 parameter was supplied +error[E0057]: this function takes 0 arguments but 1 argument was supplied --> $DIR/issue-16939.rs:5:9 | LL | |t| f(t); - | ^^^^ expected 0 parameters + | ^ - supplied 1 argument + | | + | expected 0 arguments error: aborting due to previous error diff --git a/src/test/ui/issues/issue-18819.stderr b/src/test/ui/issues/issue-18819.stderr index 41e8470ecd04f..a952c9b46c9d8 100644 --- a/src/test/ui/issues/issue-18819.stderr +++ b/src/test/ui/issues/issue-18819.stderr @@ -1,11 +1,13 @@ -error[E0061]: this function takes 2 parameters but 1 parameter was supplied +error[E0061]: this function takes 2 arguments but 1 argument was supplied --> $DIR/issue-18819.rs:16:5 | LL | fn print_x(_: &dyn Foo, extra: &str) { | ----------------------------------------------- defined here ... LL | print_x(X); - | ^^^^^^^^^^ expected 2 parameters + | ^^^^^^^ - supplied 1 argument + | | + | expected 2 arguments error: aborting due to previous error diff --git a/src/test/ui/issues/issue-26094.rs b/src/test/ui/issues/issue-26094.rs index b9433849853a8..78fb0491d82dd 100644 --- a/src/test/ui/issues/issue-26094.rs +++ b/src/test/ui/issues/issue-26094.rs @@ -1,13 +1,13 @@ macro_rules! some_macro { ($other: expr) => ({ - $other(None) - //~^ this function takes 0 parameters but 1 parameter was supplied + $other(None) //~ NOTE supplied 1 argument }) } -fn some_function() {} +fn some_function() {} //~ NOTE defined here fn main() { some_macro!(some_function); - //~^ in this expansion of some_macro! + //~^ ERROR this function takes 0 arguments but 1 argument was supplied + //~| NOTE expected 0 arguments } diff --git a/src/test/ui/issues/issue-26094.stderr b/src/test/ui/issues/issue-26094.stderr index 0b5b6d5a7504f..2038d88bf4651 100644 --- a/src/test/ui/issues/issue-26094.stderr +++ b/src/test/ui/issues/issue-26094.stderr @@ -1,16 +1,14 @@ -error[E0061]: this function takes 0 parameters but 1 parameter was supplied - --> $DIR/issue-26094.rs:3:9 +error[E0061]: this function takes 0 arguments but 1 argument was supplied + --> $DIR/issue-26094.rs:10:17 | LL | $other(None) - | ^^^^^^^^^^^^ expected 0 parameters + | ---- supplied 1 argument ... LL | fn some_function() {} | ------------------ defined here ... LL | some_macro!(some_function); - | --------------------------- in this macro invocation - | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + | ^^^^^^^^^^^^^ expected 0 arguments error: aborting due to previous error diff --git a/src/test/ui/issues/issue-3044.rs b/src/test/ui/issues/issue-3044.rs index 26db04b69b4a2..81d76a90eb0ac 100644 --- a/src/test/ui/issues/issue-3044.rs +++ b/src/test/ui/issues/issue-3044.rs @@ -2,5 +2,5 @@ fn main() { let needlesArr: Vec = vec!['a', 'f']; needlesArr.iter().fold(|x, y| { }); - //~^^ ERROR this function takes 2 parameters but 1 parameter was supplied + //~^^ ERROR this function takes 2 arguments but 1 argument was supplied } diff --git a/src/test/ui/issues/issue-3044.stderr b/src/test/ui/issues/issue-3044.stderr index 35a85b604b291..d2c010659edd0 100644 --- a/src/test/ui/issues/issue-3044.stderr +++ b/src/test/ui/issues/issue-3044.stderr @@ -1,8 +1,12 @@ -error[E0061]: this function takes 2 parameters but 1 parameter was supplied +error[E0061]: this function takes 2 arguments but 1 argument was supplied --> $DIR/issue-3044.rs:3:23 | -LL | needlesArr.iter().fold(|x, y| { - | ^^^^ expected 2 parameters +LL | needlesArr.iter().fold(|x, y| { + | _______________________^^^^_- + | | | + | | expected 2 arguments +LL | | }); + | |_____- supplied 1 argument error: aborting due to previous error diff --git a/src/test/ui/issues/issue-4935.rs b/src/test/ui/issues/issue-4935.rs index 3b258c35682c4..b342bbb1b8eab 100644 --- a/src/test/ui/issues/issue-4935.rs +++ b/src/test/ui/issues/issue-4935.rs @@ -3,4 +3,4 @@ fn foo(a: usize) {} //~^ defined here fn main() { foo(5, 6) } -//~^ ERROR this function takes 1 parameter but 2 parameters were supplied +//~^ ERROR this function takes 1 argument but 2 arguments were supplied diff --git a/src/test/ui/issues/issue-4935.stderr b/src/test/ui/issues/issue-4935.stderr index a99581fdca18f..0cc686e1cf87f 100644 --- a/src/test/ui/issues/issue-4935.stderr +++ b/src/test/ui/issues/issue-4935.stderr @@ -1,11 +1,13 @@ -error[E0061]: this function takes 1 parameter but 2 parameters were supplied +error[E0061]: this function takes 1 argument but 2 arguments were supplied --> $DIR/issue-4935.rs:5:13 | LL | fn foo(a: usize) {} | ---------------- defined here LL | LL | fn main() { foo(5, 6) } - | ^^^^^^^^^ expected 1 parameter + | ^^^ - - supplied 2 arguments + | | + | expected 1 argument error: aborting due to previous error diff --git a/src/test/ui/methods/method-call-err-msg.rs b/src/test/ui/methods/method-call-err-msg.rs index 5ff4b412667c2..9bfacc7babf2e 100644 --- a/src/test/ui/methods/method-call-err-msg.rs +++ b/src/test/ui/methods/method-call-err-msg.rs @@ -5,16 +5,18 @@ impl Foo { fn zero(self) -> Foo { self } fn one(self, _: isize) -> Foo { self } fn two(self, _: isize, _: isize) -> Foo { self } + fn three(self, _: T, _: T, _: T) -> Foo { self } } fn main() { let x = Foo; - x.zero(0) //~ ERROR this function takes 0 parameters but 1 parameter was supplied - .one() //~ ERROR this function takes 1 parameter but 0 parameters were supplied - .two(0); //~ ERROR this function takes 2 parameters but 1 parameter was supplied + x.zero(0) //~ ERROR this function takes 0 arguments but 1 argument was supplied + .one() //~ ERROR this function takes 1 argument but 0 arguments were supplied + .two(0); //~ ERROR this function takes 2 arguments but 1 argument was supplied let y = Foo; y.zero() .take() //~ ERROR no method named `take` found .one(0); + y.three::(); //~ ERROR this function takes 3 arguments but 0 arguments were supplied } diff --git a/src/test/ui/methods/method-call-err-msg.stderr b/src/test/ui/methods/method-call-err-msg.stderr index 7efdd91708a80..ab1ef5b9d5aed 100644 --- a/src/test/ui/methods/method-call-err-msg.stderr +++ b/src/test/ui/methods/method-call-err-msg.stderr @@ -1,32 +1,38 @@ -error[E0061]: this function takes 0 parameters but 1 parameter was supplied - --> $DIR/method-call-err-msg.rs:12:7 +error[E0061]: this function takes 0 arguments but 1 argument was supplied + --> $DIR/method-call-err-msg.rs:13:7 | LL | fn zero(self) -> Foo { self } | -------------------- defined here ... LL | x.zero(0) - | ^^^^ expected 0 parameters + | ^^^^ - supplied 1 argument + | | + | expected 0 arguments -error[E0061]: this function takes 1 parameter but 0 parameters were supplied - --> $DIR/method-call-err-msg.rs:13:7 +error[E0061]: this function takes 1 argument but 0 arguments were supplied + --> $DIR/method-call-err-msg.rs:14:7 | LL | fn one(self, _: isize) -> Foo { self } | ----------------------------- defined here ... LL | .one() - | ^^^ expected 1 parameter + | ^^^- supplied 0 arguments + | | + | expected 1 argument -error[E0061]: this function takes 2 parameters but 1 parameter was supplied - --> $DIR/method-call-err-msg.rs:14:7 +error[E0061]: this function takes 2 arguments but 1 argument was supplied + --> $DIR/method-call-err-msg.rs:15:7 | LL | fn two(self, _: isize, _: isize) -> Foo { self } | --------------------------------------- defined here ... LL | .two(0); - | ^^^ expected 2 parameters + | ^^^ - supplied 1 argument + | | + | expected 2 arguments error[E0599]: no method named `take` found for struct `Foo` in the current scope - --> $DIR/method-call-err-msg.rs:18:7 + --> $DIR/method-call-err-msg.rs:19:7 | LL | pub struct Foo; | --------------- method `take` not found for this @@ -41,7 +47,18 @@ LL | .take() candidate #1: `std::io::Read` candidate #2: `std::iter::Iterator` -error: aborting due to 4 previous errors +error[E0061]: this function takes 3 arguments but 0 arguments were supplied + --> $DIR/method-call-err-msg.rs:21:7 + | +LL | fn three(self, _: T, _: T, _: T) -> Foo { self } + | ------------------------------------------ defined here +... +LL | y.three::(); + | ^^^^^--------- supplied 0 arguments + | | + | expected 3 arguments + +error: aborting due to 5 previous errors Some errors have detailed explanations: E0061, E0599. For more information about an error, try `rustc --explain E0061`. diff --git a/src/test/ui/mismatched_types/overloaded-calls-bad.rs b/src/test/ui/mismatched_types/overloaded-calls-bad.rs index 73e74a97f06b0..902a6ec81d60b 100644 --- a/src/test/ui/mismatched_types/overloaded-calls-bad.rs +++ b/src/test/ui/mismatched_types/overloaded-calls-bad.rs @@ -27,7 +27,7 @@ fn main() { }; let ans = s("what"); //~ ERROR mismatched types let ans = s(); - //~^ ERROR this function takes 1 parameter but 0 parameters were supplied + //~^ ERROR this function takes 1 argument but 0 arguments were supplied let ans = s("burma", "shave"); - //~^ ERROR this function takes 1 parameter but 2 parameters were supplied + //~^ ERROR this function takes 1 argument but 2 arguments were supplied } diff --git a/src/test/ui/mismatched_types/overloaded-calls-bad.stderr b/src/test/ui/mismatched_types/overloaded-calls-bad.stderr index 5a5dd05626148..706e25529bfaf 100644 --- a/src/test/ui/mismatched_types/overloaded-calls-bad.stderr +++ b/src/test/ui/mismatched_types/overloaded-calls-bad.stderr @@ -4,17 +4,21 @@ error[E0308]: mismatched types LL | let ans = s("what"); | ^^^^^^ expected `isize`, found `&str` -error[E0057]: this function takes 1 parameter but 0 parameters were supplied +error[E0057]: this function takes 1 argument but 0 arguments were supplied --> $DIR/overloaded-calls-bad.rs:29:15 | LL | let ans = s(); - | ^^^ expected 1 parameter + | ^-- supplied 0 arguments + | | + | expected 1 argument -error[E0057]: this function takes 1 parameter but 2 parameters were supplied +error[E0057]: this function takes 1 argument but 2 arguments were supplied --> $DIR/overloaded-calls-bad.rs:31:15 | LL | let ans = s("burma", "shave"); - | ^^^^^^^^^^^^^^^^^^^ expected 1 parameter + | ^ ------- ------- supplied 2 arguments + | | + | expected 1 argument error: aborting due to 3 previous errors diff --git a/src/test/ui/not-enough-arguments.rs b/src/test/ui/not-enough-arguments.rs index 309283ed7f4fe..631bb1dd27415 100644 --- a/src/test/ui/not-enough-arguments.rs +++ b/src/test/ui/not-enough-arguments.rs @@ -8,5 +8,5 @@ fn foo(a: isize, b: isize, c: isize, d:isize) { fn main() { foo(1, 2, 3); - //~^ ERROR this function takes 4 parameters but 3 + //~^ ERROR this function takes 4 arguments but 3 } diff --git a/src/test/ui/not-enough-arguments.stderr b/src/test/ui/not-enough-arguments.stderr index c1ee43ea90484..f2b57f71400f1 100644 --- a/src/test/ui/not-enough-arguments.stderr +++ b/src/test/ui/not-enough-arguments.stderr @@ -1,11 +1,13 @@ -error[E0061]: this function takes 4 parameters but 3 parameters were supplied +error[E0061]: this function takes 4 arguments but 3 arguments were supplied --> $DIR/not-enough-arguments.rs:10:3 | LL | fn foo(a: isize, b: isize, c: isize, d:isize) { | --------------------------------------------- defined here ... LL | foo(1, 2, 3); - | ^^^^^^^^^^^^ expected 4 parameters + | ^^^ - - - supplied 3 arguments + | | + | expected 4 arguments error: aborting due to previous error diff --git a/src/test/ui/resolve/resolve-primitive-fallback.rs b/src/test/ui/resolve/resolve-primitive-fallback.rs index e5a3d689d23d8..992bcd7977fcf 100644 --- a/src/test/ui/resolve/resolve-primitive-fallback.rs +++ b/src/test/ui/resolve/resolve-primitive-fallback.rs @@ -2,7 +2,7 @@ fn main() { // Make sure primitive type fallback doesn't work in value namespace std::mem::size_of(u16); //~^ ERROR expected value, found builtin type `u16` - //~| ERROR this function takes 0 parameters but 1 parameter was supplied + //~| ERROR this function takes 0 arguments but 1 argument was supplied // Make sure primitive type fallback doesn't work with global paths let _: ::u8; diff --git a/src/test/ui/resolve/resolve-primitive-fallback.stderr b/src/test/ui/resolve/resolve-primitive-fallback.stderr index 92c2a03298381..6d61d2f16cafa 100644 --- a/src/test/ui/resolve/resolve-primitive-fallback.stderr +++ b/src/test/ui/resolve/resolve-primitive-fallback.stderr @@ -10,11 +10,13 @@ error[E0412]: cannot find type `u8` in the crate root LL | let _: ::u8; | ^^ not found in the crate root -error[E0061]: this function takes 0 parameters but 1 parameter was supplied +error[E0061]: this function takes 0 arguments but 1 argument was supplied --> $DIR/resolve-primitive-fallback.rs:3:5 | LL | std::mem::size_of(u16); - | ^^^^^^^^^^^^^^^^^^^^^^ expected 0 parameters + | ^^^^^^^^^^^^^^^^^ --- supplied 1 argument + | | + | expected 0 arguments error: aborting due to 3 previous errors diff --git a/src/test/ui/span/E0057.stderr b/src/test/ui/span/E0057.stderr index 6b5890cac36c5..31579e2828964 100644 --- a/src/test/ui/span/E0057.stderr +++ b/src/test/ui/span/E0057.stderr @@ -1,14 +1,18 @@ -error[E0057]: this function takes 1 parameter but 0 parameters were supplied +error[E0057]: this function takes 1 argument but 0 arguments were supplied --> $DIR/E0057.rs:3:13 | LL | let a = f(); - | ^^^ expected 1 parameter + | ^-- supplied 0 arguments + | | + | expected 1 argument -error[E0057]: this function takes 1 parameter but 2 parameters were supplied +error[E0057]: this function takes 1 argument but 2 arguments were supplied --> $DIR/E0057.rs:5:13 | LL | let c = f(2, 3); - | ^^^^^^^ expected 1 parameter + | ^ - - supplied 2 arguments + | | + | expected 1 argument error: aborting due to 2 previous errors diff --git a/src/test/ui/span/issue-34264.stderr b/src/test/ui/span/issue-34264.stderr index 80a237ac6aad4..116f5ddd5b4b2 100644 --- a/src/test/ui/span/issue-34264.stderr +++ b/src/test/ui/span/issue-34264.stderr @@ -50,14 +50,16 @@ help: if this is a type, explicitly ignore the parameter name LL | fn bar(_: x, y: usize) {} | ^^^^ -error[E0061]: this function takes 2 parameters but 3 parameters were supplied +error[E0061]: this function takes 2 arguments but 3 arguments were supplied --> $DIR/issue-34264.rs:7:5 | LL | fn foo(Option, String) {} | --------------------------- defined here ... LL | foo(Some(42), 2, ""); - | ^^^^^^^^^^^^^^^^^^^^ expected 2 parameters + | ^^^ -------- - -- supplied 3 arguments + | | + | expected 2 arguments error[E0308]: mismatched types --> $DIR/issue-34264.rs:8:13 @@ -65,14 +67,16 @@ error[E0308]: mismatched types LL | bar("", ""); | ^^ expected `usize`, found `&str` -error[E0061]: this function takes 2 parameters but 3 parameters were supplied +error[E0061]: this function takes 2 arguments but 3 arguments were supplied --> $DIR/issue-34264.rs:10:5 | LL | fn bar(x, y: usize) {} | ------------------- defined here ... LL | bar(1, 2, 3); - | ^^^^^^^^^^^^ expected 2 parameters + | ^^^ - - - supplied 3 arguments + | | + | expected 2 arguments error: aborting due to 6 previous errors diff --git a/src/test/ui/span/missing-unit-argument.stderr b/src/test/ui/span/missing-unit-argument.stderr index 90a96e3f17403..f6344fba3d368 100644 --- a/src/test/ui/span/missing-unit-argument.stderr +++ b/src/test/ui/span/missing-unit-argument.stderr @@ -1,68 +1,72 @@ -error[E0061]: this function takes 1 parameter but 0 parameters were supplied +error[E0061]: this function takes 1 argument but 0 arguments were supplied --> $DIR/missing-unit-argument.rs:11:33 | LL | let _: Result<(), String> = Ok(); - | ^^^^ + | ^^-- supplied 0 arguments | help: expected the unit value `()`; create it with empty parentheses | LL | let _: Result<(), String> = Ok(()); | ^^ -error[E0061]: this function takes 2 parameters but 0 parameters were supplied +error[E0061]: this function takes 2 arguments but 0 arguments were supplied --> $DIR/missing-unit-argument.rs:12:5 | LL | fn foo(():(), ():()) {} | -------------------- defined here ... LL | foo(); - | ^^^^^ expected 2 parameters + | ^^^-- supplied 0 arguments + | | + | expected 2 arguments -error[E0061]: this function takes 2 parameters but 1 parameter was supplied +error[E0061]: this function takes 2 arguments but 1 argument was supplied --> $DIR/missing-unit-argument.rs:13:5 | LL | fn foo(():(), ():()) {} | -------------------- defined here ... LL | foo(()); - | ^^^^^^^ expected 2 parameters + | ^^^ -- supplied 1 argument + | | + | expected 2 arguments -error[E0061]: this function takes 1 parameter but 0 parameters were supplied +error[E0061]: this function takes 1 argument but 0 arguments were supplied --> $DIR/missing-unit-argument.rs:14:5 | LL | fn bar(():()) {} | ------------- defined here ... LL | bar(); - | ^^^^^ + | ^^^-- supplied 0 arguments | help: expected the unit value `()`; create it with empty parentheses | LL | bar(()); | ^^ -error[E0061]: this function takes 1 parameter but 0 parameters were supplied +error[E0061]: this function takes 1 argument but 0 arguments were supplied --> $DIR/missing-unit-argument.rs:15:7 | LL | fn baz(self, (): ()) { } | -------------------- defined here ... LL | S.baz(); - | ^^^ + | ^^^- supplied 0 arguments | help: expected the unit value `()`; create it with empty parentheses | LL | S.baz(()); | ^^ -error[E0061]: this function takes 1 parameter but 0 parameters were supplied +error[E0061]: this function takes 1 argument but 0 arguments were supplied --> $DIR/missing-unit-argument.rs:16:7 | LL | fn generic(self, _: T) { } | ------------------------- defined here ... LL | S.generic::<()>(); - | ^^^^^^^ + | ^^^^^^^------ supplied 0 arguments | help: expected the unit value `()`; create it with empty parentheses | diff --git a/src/test/ui/type-alias-enum-variants/enum-variant-priority-higher-than-other-inherent.rs b/src/test/ui/type-alias-enum-variants/enum-variant-priority-higher-than-other-inherent.rs index fa3e1a35fc27a..d012687533bbe 100644 --- a/src/test/ui/type-alias-enum-variants/enum-variant-priority-higher-than-other-inherent.rs +++ b/src/test/ui/type-alias-enum-variants/enum-variant-priority-higher-than-other-inherent.rs @@ -18,6 +18,6 @@ impl E2 { } fn main() { - ::V(); //~ ERROR this function takes 1 parameter but 0 parameters were supplied + ::V(); //~ ERROR this function takes 1 argument but 0 arguments were supplied let _: u8 = ::V; //~ ERROR mismatched types } diff --git a/src/test/ui/type-alias-enum-variants/enum-variant-priority-higher-than-other-inherent.stderr b/src/test/ui/type-alias-enum-variants/enum-variant-priority-higher-than-other-inherent.stderr index 95c3a08c04aa8..46e7dd0c517e8 100644 --- a/src/test/ui/type-alias-enum-variants/enum-variant-priority-higher-than-other-inherent.stderr +++ b/src/test/ui/type-alias-enum-variants/enum-variant-priority-higher-than-other-inherent.stderr @@ -1,11 +1,13 @@ -error[E0061]: this function takes 1 parameter but 0 parameters were supplied +error[E0061]: this function takes 1 argument but 0 arguments were supplied --> $DIR/enum-variant-priority-higher-than-other-inherent.rs:21:5 | LL | V(u8) | ----- defined here ... LL | ::V(); - | ^^^^^^^^ expected 1 parameter + | ^^^^^^-- supplied 0 arguments + | | + | expected 1 argument error[E0308]: mismatched types --> $DIR/enum-variant-priority-higher-than-other-inherent.rs:22:17 diff --git a/src/test/ui/type/type-ascription-instead-of-initializer.rs b/src/test/ui/type/type-ascription-instead-of-initializer.rs index aef25250b159f..9f9b6f06bbc24 100644 --- a/src/test/ui/type/type-ascription-instead-of-initializer.rs +++ b/src/test/ui/type/type-ascription-instead-of-initializer.rs @@ -1,4 +1,4 @@ fn main() { let x: Vec::with_capacity(10, 20); //~ ERROR expected type, found `10` - //~^ ERROR this function takes 1 parameter + //~^ ERROR this function takes 1 argument } diff --git a/src/test/ui/type/type-ascription-instead-of-initializer.stderr b/src/test/ui/type/type-ascription-instead-of-initializer.stderr index 3fe676de59dab..530f77e5ae9b9 100644 --- a/src/test/ui/type/type-ascription-instead-of-initializer.stderr +++ b/src/test/ui/type/type-ascription-instead-of-initializer.stderr @@ -7,11 +7,13 @@ LL | let x: Vec::with_capacity(10, 20); | |help: use `=` if you meant to assign | while parsing the type for `x` -error[E0061]: this function takes 1 parameter but 2 parameters were supplied +error[E0061]: this function takes 1 argument but 2 arguments were supplied --> $DIR/type-ascription-instead-of-initializer.rs:2:12 | LL | let x: Vec::with_capacity(10, 20); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected 1 parameter + | ^^^^^^^^^^^^^^^^^^ -- -- supplied 2 arguments + | | + | expected 1 argument error: aborting due to 2 previous errors From c1ed84e6eccbd57dde9b69e2defd4d79faf41b1f Mon Sep 17 00:00:00 2001 From: jumbatm Date: Wed, 12 Feb 2020 08:37:06 +1000 Subject: [PATCH 32/32] Fix outdated doc comment. --- src/librustc/mir/interpret/error.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index df3971a5ac3f1..86e7bb28e0069 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -128,10 +128,9 @@ impl<'tcx> ConstEvalErr<'tcx> { } } - /// Sets the message passed in via `message`, then adds the span labels for you, before applying - /// further modifications in `emit`. It's up to you to call emit(), stash(..), etc. within the - /// `emit` method. If you don't need to do any additional processing, just use - /// struct_generic. + /// Sets the message passed in via `message` and adds span labels before handing control back + /// to `emit` to do any final processing. It's the caller's responsibility to call emit(), + /// stash(), etc. within the `emit` function to dispose of the diagnostic properly. fn struct_generic( &self, tcx: TyCtxtAt<'tcx>,