From 4b99699c8407f38df1e0ec1c22488c558d414582 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 17 Jul 2020 12:28:23 +0100 Subject: [PATCH] index: introduce and use `FiniteBitSet` This commit introduces a `FiniteBitSet` type which replaces the manual bit manipulation which was being performed in polymorphization. Signed-off-by: David Wood --- src/librustc_data_structures/stable_hasher.rs | 9 ++ src/librustc_index/bit_set.rs | 135 ++++++++++++++++++ src/librustc_index/lib.rs | 1 + src/librustc_metadata/rmeta/decoder.rs | 2 +- src/librustc_metadata/rmeta/mod.rs | 4 +- src/librustc_middle/query/mod.rs | 2 +- src/librustc_middle/ty/instance.rs | 8 +- src/librustc_middle/ty/query/mod.rs | 2 +- src/librustc_mir/monomorphize/polymorphize.rs | 109 ++++++-------- .../too-many-generic-params.rs | 12 +- 10 files changed, 211 insertions(+), 73 deletions(-) diff --git a/src/librustc_data_structures/stable_hasher.rs b/src/librustc_data_structures/stable_hasher.rs index 97b02eaef35..c1c79b174f4 100644 --- a/src/librustc_data_structures/stable_hasher.rs +++ b/src/librustc_data_structures/stable_hasher.rs @@ -469,6 +469,15 @@ impl HashStable for bit_set::BitMatrix } } +impl HashStable for bit_set::FiniteBitSet +where + T: HashStable + bit_set::FiniteBitSetTy, +{ + fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) { + self.0.hash_stable(hcx, hasher); + } +} + impl_stable_hash_via_hash!(::std::path::Path); impl_stable_hash_via_hash!(::std::path::PathBuf); diff --git a/src/librustc_index/bit_set.rs b/src/librustc_index/bit_set.rs index 3e1d4b68c6f..4e94250fc62 100644 --- a/src/librustc_index/bit_set.rs +++ b/src/librustc_index/bit_set.rs @@ -4,6 +4,7 @@ use std::fmt; use std::iter; use std::marker::PhantomData; use std::mem; +use std::ops::{BitAnd, BitAndAssign, BitOrAssign, Not, Range, Shl}; use std::slice; #[cfg(test)] @@ -1001,3 +1002,137 @@ fn word_index_and_mask(elem: T) -> (usize, Word) { let mask = 1 << (elem % WORD_BITS); (word_index, mask) } + +/// Integral type used to represent the bit set. +pub trait FiniteBitSetTy: + BitAnd + + BitAndAssign + + BitOrAssign + + Clone + + Copy + + Shl + + Not + + PartialEq + + Sized +{ + /// Size of the domain representable by this type, e.g. 64 for `u64`. + const DOMAIN_SIZE: u32; + + /// Value which represents the `FiniteBitSet` having every bit set. + const FILLED: Self; + /// Value which represents the `FiniteBitSet` having no bits set. + const EMPTY: Self; + + /// Value for one as the integral type. + const ONE: Self; + /// Value for zero as the integral type. + const ZERO: Self; + + /// Perform a checked left shift on the integral type. + fn checked_shl(self, rhs: u32) -> Option; + /// Perform a checked right shift on the integral type. + fn checked_shr(self, rhs: u32) -> Option; +} + +impl FiniteBitSetTy for u64 { + const DOMAIN_SIZE: u32 = 64; + + const FILLED: Self = Self::MAX; + const EMPTY: Self = Self::MIN; + + const ONE: Self = 1u64; + const ZERO: Self = 0u64; + + fn checked_shl(self, rhs: u32) -> Option { + self.checked_shl(rhs) + } + + fn checked_shr(self, rhs: u32) -> Option { + self.checked_shr(rhs) + } +} + +impl std::fmt::Debug for FiniteBitSet { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:064b}", self.0) + } +} + +impl FiniteBitSetTy for u128 { + const DOMAIN_SIZE: u32 = 128; + + const FILLED: Self = Self::MAX; + const EMPTY: Self = Self::MIN; + + const ONE: Self = 1u128; + const ZERO: Self = 0u128; + + fn checked_shl(self, rhs: u32) -> Option { + self.checked_shl(rhs) + } + + fn checked_shr(self, rhs: u32) -> Option { + self.checked_shr(rhs) + } +} + +impl std::fmt::Debug for FiniteBitSet { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:0128b}", self.0) + } +} + +/// A fixed-sized bitset type represented by an integer type. Indices outwith than the range +/// representable by `T` are considered set. +#[derive(Copy, Clone, Eq, PartialEq, RustcDecodable, RustcEncodable)] +pub struct FiniteBitSet(pub T); + +impl FiniteBitSet { + /// Creates a new, empty bitset. + pub fn new_empty() -> Self { + Self(T::EMPTY) + } + + /// Sets the `index`th bit. + pub fn set(&mut self, index: u32) { + self.0 |= T::ONE.checked_shl(index).unwrap_or(T::ZERO); + } + + /// Unsets the `index`th bit. + pub fn clear(&mut self, index: u32) { + self.0 &= !T::ONE.checked_shl(index).unwrap_or(T::ZERO); + } + + /// Sets the `i`th to `j`th bits. + pub fn set_range(&mut self, range: Range) { + let bits = T::FILLED + .checked_shl(range.end - range.start) + .unwrap_or(T::ZERO) + .not() + .checked_shl(range.start) + .unwrap_or(T::ZERO); + self.0 |= bits; + } + + /// Is the set empty? + pub fn is_empty(&self) -> bool { + self.0 == T::EMPTY + } + + /// Returns the domain size of the bitset. + pub fn within_domain(&self, index: u32) -> bool { + index < T::DOMAIN_SIZE + } + + /// Returns if the `index`th bit is set. + pub fn contains(&self, index: u32) -> Option { + self.within_domain(index) + .then(|| ((self.0.checked_shr(index).unwrap_or(T::ONE)) & T::ONE) == T::ONE) + } +} + +impl Default for FiniteBitSet { + fn default() -> Self { + Self::new_empty() + } +} diff --git a/src/librustc_index/lib.rs b/src/librustc_index/lib.rs index eaef4c7b54a..7ee881b0639 100644 --- a/src/librustc_index/lib.rs +++ b/src/librustc_index/lib.rs @@ -1,4 +1,5 @@ #![feature(allow_internal_unstable)] +#![feature(bool_to_option)] #![feature(const_fn)] #![feature(const_panic)] #![feature(extend_one)] diff --git a/src/librustc_metadata/rmeta/decoder.rs b/src/librustc_metadata/rmeta/decoder.rs index e4edb2427a2..a6d708ebe90 100644 --- a/src/librustc_metadata/rmeta/decoder.rs +++ b/src/librustc_metadata/rmeta/decoder.rs @@ -1132,7 +1132,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { .decode((self, tcx)) } - fn get_unused_generic_params(&self, id: DefIndex) -> u64 { + fn get_unused_generic_params(&self, id: DefIndex) -> FiniteBitSet { self.root .tables .unused_generic_params diff --git a/src/librustc_metadata/rmeta/mod.rs b/src/librustc_metadata/rmeta/mod.rs index 1d095ac06ec..e616e8cf00a 100644 --- a/src/librustc_metadata/rmeta/mod.rs +++ b/src/librustc_metadata/rmeta/mod.rs @@ -9,7 +9,7 @@ use rustc_hir as hir; use rustc_hir::def::CtorKind; use rustc_hir::def_id::{DefId, DefIndex}; use rustc_hir::lang_items; -use rustc_index::vec::IndexVec; +use rustc_index::{bit_set::FiniteBitSet, vec::IndexVec}; use rustc_middle::hir::exports::Export; use rustc_middle::middle::cstore::{DepKind, ForeignModule, LinkagePreference, NativeLib}; use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportLevel}; @@ -277,7 +277,7 @@ define_tables! { super_predicates: Table)>, mir: Table)>, promoted_mir: Table>)>, - unused_generic_params: Table>, + unused_generic_params: Table>>, } #[derive(Copy, Clone, RustcEncodable, RustcDecodable)] diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 0c895cd2a29..9b2242dd037 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -1309,7 +1309,7 @@ rustc_queries! { query codegen_unit(_: Symbol) -> &'tcx CodegenUnit<'tcx> { desc { "codegen_unit" } } - query unused_generic_params(key: DefId) -> u64 { + query unused_generic_params(key: DefId) -> FiniteBitSet { cache_on_disk_if { key.is_local() } desc { |tcx| "determining which generic parameters are unused by `{}`", diff --git a/src/librustc_middle/ty/instance.rs b/src/librustc_middle/ty/instance.rs index 9c204ab16fb..cdb883da32b 100644 --- a/src/librustc_middle/ty/instance.rs +++ b/src/librustc_middle/ty/instance.rs @@ -474,20 +474,20 @@ impl<'tcx> Instance<'tcx> { } if let InstanceDef::Item(def) = self.def { - let results = tcx.unused_generic_params(def.did); + let unused = tcx.unused_generic_params(def.did); - if results == 0 { + if unused.is_empty() { // Exit early if every parameter was used. return self; } - debug!("polymorphize: results={:064b}", results); + debug!("polymorphize: unused={:?}", unused); let polymorphized_substs = InternalSubsts::for_item(tcx, def.did, |param, _| match param.kind { // If parameter is a const or type parameter.. ty::GenericParamDefKind::Const | ty::GenericParamDefKind::Type { .. } if // ..and is within range and unused.. - param.index < 64 && ((results >> param.index) & 1) == 1 => + unused.contains(param.index).unwrap_or(false) => // ..then use the identity for this parameter. tcx.mk_param_from_def(param), // Otherwise, use the parameter as before. diff --git a/src/librustc_middle/ty/query/mod.rs b/src/librustc_middle/ty/query/mod.rs index 2ad49b1acce..2f7a9aee536 100644 --- a/src/librustc_middle/ty/query/mod.rs +++ b/src/librustc_middle/ty/query/mod.rs @@ -44,7 +44,7 @@ use rustc_hir::def::DefKind; use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet, LocalDefId}; use rustc_hir::lang_items::{LangItem, LanguageItems}; use rustc_hir::{Crate, HirIdSet, ItemLocalId, TraitCandidate}; -use rustc_index::vec::IndexVec; +use rustc_index::{bit_set::FiniteBitSet, vec::IndexVec}; use rustc_session::config::{EntryFnType, OptLevel, OutputFilenames, SymbolManglingVersion}; use rustc_session::utils::NativeLibKind; use rustc_session::CrateDisambiguator; diff --git a/src/librustc_mir/monomorphize/polymorphize.rs b/src/librustc_mir/monomorphize/polymorphize.rs index 2a1f8fb843f..071b9bb9711 100644 --- a/src/librustc_mir/monomorphize/polymorphize.rs +++ b/src/librustc_mir/monomorphize/polymorphize.rs @@ -6,6 +6,7 @@ //! for their size, offset of a field, etc.). use rustc_hir::{def::DefKind, def_id::DefId}; +use rustc_index::bit_set::FiniteBitSet; use rustc_middle::mir::{ visit::{TyContext, Visitor}, Local, LocalDecl, Location, @@ -25,14 +26,14 @@ pub fn provide(providers: &mut Providers) { } /// Determine which generic parameters are used by the function/method/closure represented by -/// `def_id`. Returns a `u64` where a bit is set if the parameter with that index is unused (ie. -/// a value of zero indicates that all parameters are used). -fn unused_generic_params(tcx: TyCtxt<'_>, def_id: DefId) -> u64 { +/// `def_id`. Returns a bitset where bits representing unused parameters are set (`is_empty` +/// indicates all parameters are used). +fn unused_generic_params(tcx: TyCtxt<'_>, def_id: DefId) -> FiniteBitSet { debug!("unused_generic_params({:?})", def_id); if !tcx.sess.opts.debugging_opts.polymorphize { // If polymorphization disabled, then all parameters are used. - return 0; + return FiniteBitSet::new_empty(); } let generics = tcx.generics_of(def_id); @@ -40,63 +41,42 @@ fn unused_generic_params(tcx: TyCtxt<'_>, def_id: DefId) -> u64 { // Exit early when there are no parameters to be unused. if generics.count() == 0 { - return 0; + return FiniteBitSet::new_empty(); } // Exit early when there is no MIR available. if !tcx.is_mir_available(def_id) { debug!("unused_generic_params: (no mir available) def_id={:?}", def_id); - return 0; + return FiniteBitSet::new_empty(); } - // Use a `u64` as a bitset. Starting with all ones, shift left by the number of parameters, - // leaving N zeros for each parameter. When a parameter is marked as used, the bit (from the - // left) corresponding to the parameter index will be flipped. This is the opposite of what - // will be returned. + // Create a bitset with N rightmost ones for each parameter. let generics_count: u32 = generics.count().try_into().expect("more generic parameters than can fit into a `u32`"); - let mut used_parameters = u64::max_value().checked_shl(generics_count).unwrap_or(0); - debug!("unused_generic_params: (start) used_parameters={:064b}", used_parameters); - mark_used_by_default_parameters(tcx, def_id, generics, &mut used_parameters); - debug!("unused_generic_params: (after default) used_parameters={:064b}", used_parameters); + let mut unused_parameters = FiniteBitSet::::new_empty(); + unused_parameters.set_range(0..generics_count); + debug!("unused_generic_params: (start) unused_parameters={:?}", unused_parameters); + mark_used_by_default_parameters(tcx, def_id, generics, &mut unused_parameters); + debug!("unused_generic_params: (after default) unused_parameters={:?}", unused_parameters); // Visit MIR and accumululate used generic parameters. let body = tcx.optimized_mir(def_id); let mut vis = - UsedGenericParametersVisitor { tcx, def_id, used_parameters: &mut used_parameters }; + UsedGenericParametersVisitor { tcx, def_id, unused_parameters: &mut unused_parameters }; vis.visit_body(body); - debug!("unused_generic_params: (after visitor) used_parameters={:064b}", used_parameters); + debug!("unused_generic_params: (after visitor) unused_parameters={:?}", unused_parameters); - mark_used_by_predicates(tcx, def_id, &mut used_parameters); - debug!("unused_generic_params: (after predicates) used_parameters={:064b}", used_parameters); - - // Invert the u64 so that used is 0 and unused is 1. This makes checking if all parameters are - // used easy - just compare with zero. - debug!("unused_generic_params: (end) used_parameters={:064b}", used_parameters); - let unused_parameters: u64 = !used_parameters; - debug!("unused_generic_params: (flipped) unused_parameters={:064b}", unused_parameters); + mark_used_by_predicates(tcx, def_id, &mut unused_parameters); + debug!("unused_generic_params: (end) unused_parameters={:?}", unused_parameters); // Emit errors for debugging and testing if enabled. - let is_full = unused_parameters == 0; - if !is_full { - emit_unused_generic_params_error(tcx, def_id, generics, unused_parameters); + if !unused_parameters.is_empty() { + emit_unused_generic_params_error(tcx, def_id, generics, &unused_parameters); } unused_parameters } -/// Checks if the `param_index`th bit is set (or out-of-range). -fn is_bit_set(parameters: u64, param_index: u32) -> bool { - param_index >= 64 || ((parameters.checked_shr(param_index).unwrap_or(1)) & 1) == 1 -} - -/// Flips the bit corresponding to the parameter index. -fn set_bit(used_parameters: &mut u64, param_index: u32) { - debug!("set_bit: used_parameters={:064b} param_index={:?}", used_parameters, param_index); - *used_parameters |= 1u64.checked_shl(param_index).unwrap_or(0); - debug!("set_bit: used_parameters={:064b}", used_parameters); -} - /// Some parameters are considered used-by-default, such as non-generic parameters and the dummy /// generic parameters from closures, this function marks them as used. `leaf_is_closure` should /// be `true` if the item that `unused_generic_params` was invoked on is a closure. @@ -104,43 +84,47 @@ fn mark_used_by_default_parameters<'tcx>( tcx: TyCtxt<'tcx>, def_id: DefId, generics: &'tcx ty::Generics, - used_parameters: &mut u64, + unused_parameters: &mut FiniteBitSet, ) { if !tcx.is_trait(def_id) && (tcx.is_closure(def_id) || tcx.type_of(def_id).is_generator()) { for param in &generics.params { debug!("mark_used_by_default_parameters: (closure/gen) param={:?}", param); - set_bit(used_parameters, param.index); + unused_parameters.clear(param.index); } } else { for param in &generics.params { debug!("mark_used_by_default_parameters: (other) param={:?}", param); if let ty::GenericParamDefKind::Lifetime = param.kind { - set_bit(used_parameters, param.index); + unused_parameters.clear(param.index); } } } if let Some(parent) = generics.parent { - mark_used_by_default_parameters(tcx, parent, tcx.generics_of(parent), used_parameters); + mark_used_by_default_parameters(tcx, parent, tcx.generics_of(parent), unused_parameters); } } /// Search the predicates on used generic parameters for any unused generic parameters, and mark /// those as used. -fn mark_used_by_predicates<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, used_parameters: &mut u64) { +fn mark_used_by_predicates<'tcx>( + tcx: TyCtxt<'tcx>, + def_id: DefId, + unused_parameters: &mut FiniteBitSet, +) { let def_id = tcx.closure_base_def_id(def_id); - let is_self_ty_used = |used_parameters: &mut u64, self_ty: Ty<'tcx>| { + let is_self_ty_used = |unused_parameters: &mut FiniteBitSet, self_ty: Ty<'tcx>| { debug!("unused_generic_params: self_ty={:?}", self_ty); if let ty::Param(param) = self_ty.kind { - is_bit_set(*used_parameters, param.index) + !unused_parameters.contains(param.index).unwrap_or(false) } else { false } }; - let mark_ty = |used_parameters: &mut u64, ty: Ty<'tcx>| { - let mut vis = UsedGenericParametersVisitor { tcx, def_id, used_parameters }; + let mark_ty = |unused_parameters: &mut FiniteBitSet, ty: Ty<'tcx>| { + let mut vis = UsedGenericParametersVisitor { tcx, def_id, unused_parameters }; ty.visit_with(&mut vis); }; @@ -150,19 +134,19 @@ fn mark_used_by_predicates<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, used_paramete match predicate.kind() { ty::PredicateKind::Trait(predicate, ..) => { let trait_ref = predicate.skip_binder().trait_ref; - if is_self_ty_used(used_parameters, trait_ref.self_ty()) { + if is_self_ty_used(unused_parameters, trait_ref.self_ty()) { for ty in trait_ref.substs.types() { debug!("unused_generic_params: (trait) ty={:?}", ty); - mark_ty(used_parameters, ty); + mark_ty(unused_parameters, ty); } } } ty::PredicateKind::Projection(predicate, ..) => { let self_ty = predicate.skip_binder().projection_ty.self_ty(); - if is_self_ty_used(used_parameters, self_ty) { + if is_self_ty_used(unused_parameters, self_ty) { let ty = predicate.ty(); debug!("unused_generic_params: (projection) ty={:?}", ty); - mark_ty(used_parameters, ty.skip_binder()); + mark_ty(unused_parameters, ty.skip_binder()); } } _ => (), @@ -176,7 +160,7 @@ fn emit_unused_generic_params_error<'tcx>( tcx: TyCtxt<'tcx>, def_id: DefId, generics: &'tcx ty::Generics, - unused_parameters: u64, + unused_parameters: &FiniteBitSet, ) { debug!("emit_unused_generic_params_error: def_id={:?}", def_id); let base_def_id = tcx.closure_base_def_id(def_id); @@ -184,7 +168,7 @@ fn emit_unused_generic_params_error<'tcx>( return; } - debug!("emit_unused_generic_params_error: unused_parameters={:064b}", unused_parameters); + debug!("emit_unused_generic_params_error: unused_parameters={:?}", unused_parameters); let fn_span = match tcx.opt_item_name(def_id) { Some(ident) => ident.span, _ => tcx.def_span(def_id), @@ -195,7 +179,7 @@ fn emit_unused_generic_params_error<'tcx>( let mut next_generics = Some(generics); while let Some(generics) = next_generics { for param in &generics.params { - if is_bit_set(unused_parameters, param.index) { + if unused_parameters.contains(param.index).unwrap_or(false) { debug!("emit_unused_generic_params_error: param={:?}", param); let def_span = tcx.def_span(param.def_id); err.span_label(def_span, &format!("generic parameter `{}` is unused", param.name)); @@ -212,7 +196,7 @@ fn emit_unused_generic_params_error<'tcx>( struct UsedGenericParametersVisitor<'a, 'tcx> { tcx: TyCtxt<'tcx>, def_id: DefId, - used_parameters: &'a mut u64, + unused_parameters: &'a mut FiniteBitSet, } impl<'a, 'tcx> Visitor<'tcx> for UsedGenericParametersVisitor<'a, 'tcx> { @@ -252,7 +236,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for UsedGenericParametersVisitor<'a, 'tcx> { match c.val { ty::ConstKind::Param(param) => { debug!("visit_const: param={:?}", param); - set_bit(self.used_parameters, param.index); + self.unused_parameters.clear(param.index); false } _ => c.super_visit_with(self), @@ -277,21 +261,22 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for UsedGenericParametersVisitor<'a, 'tcx> { // parent. let unused = self.tcx.unused_generic_params(def_id); debug!( - "visit_ty: used_parameters={:064b} unused={:064b}", - self.used_parameters, unused + "visit_ty: unused_parameters={:?} unused={:?}", + self.unused_parameters, unused ); for (i, arg) in substs.iter().enumerate() { - if !is_bit_set(unused, i.try_into().unwrap()) { + let i = i.try_into().unwrap(); + if !unused.contains(i).unwrap_or(false) { arg.visit_with(self); } } - debug!("visit_ty: used_parameters={:064b}", self.used_parameters); + debug!("visit_ty: unused_parameters={:?}", self.unused_parameters); false } ty::Param(param) => { debug!("visit_ty: param={:?}", param); - set_bit(self.used_parameters, param.index); + self.unused_parameters.clear(param.index); false } _ => ty.super_visit_with(self), diff --git a/src/test/ui/polymorphization/too-many-generic-params.rs b/src/test/ui/polymorphization/too-many-generic-params.rs index 4b296c01908..ec6244630fd 100644 --- a/src/test/ui/polymorphization/too-many-generic-params.rs +++ b/src/test/ui/polymorphization/too-many-generic-params.rs @@ -7,7 +7,7 @@ #[rustc_polymorphize_error] fn bar() + AX, AY, AZ, BA, BB, BC, BD, BE, BF, BG, BH, BI, BJ, BK, BL, BM>() { let _: Option = None; let _: Option = None; @@ -60,6 +60,7 @@ fn bar = None; let _: Option = None; let _: Option = None; + let _: Option = None; let _: Option = None; let _: Option = None; let _: Option = None; @@ -72,6 +73,13 @@ fn bar = None; let _: Option = None; let _: Option = None; + let _: Option = None; } -fn main() { } +fn main() { + bar::(); +}