diff --git a/.forgejo/workflows/test.yml b/.forgejo/workflows/test.yml index 49fb3e4..d878758 100644 --- a/.forgejo/workflows/test.yml +++ b/.forgejo/workflows/test.yml @@ -59,3 +59,4 @@ jobs: - run: cargo build --tests --features=unstable-doc - run: cargo test --doc --features=unstable-doc - run: cargo doc --features=unstable-doc + - run: cargo test --test=module --features=unstable-doc,unstable-test-hasher diff --git a/Cargo.lock b/Cargo.lock index 23cdc34..e0c32e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,18 +2,6 @@ # It is not intended for manual editing. version = 3 -[[package]] -name = "ahash" -version = "0.8.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77c3a9648d43b9cd48db467b3f87fdd6e146bcc88ab0180006cef2179fe11d01" -dependencies = [ - "cfg-if", - "once_cell", - "version_check", - "zerocopy", -] - [[package]] name = "allocator-api2" version = "0.2.16" @@ -365,6 +353,12 @@ version = "0.5.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d674e81391d1e1ab681a28d99df07927c6d4aa5b027d7da16ba32d1d21ecd99" +[[package]] +name = "foldhash" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" + [[package]] name = "funty" version = "2.0.0" @@ -400,12 +394,13 @@ checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" [[package]] name = "hashbrown" -version = "0.14.3" +version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "290f1a1d9242c78d09ce40a5e87e7554ee637af1351968159f4952f028f75604" +checksum = "bf151400ff0baff5465007dd2f3e717f3fe502074ca563069ce3a6629d07b289" dependencies = [ - "ahash", "allocator-api2", + "equivalent", + "foldhash", ] [[package]] @@ -431,9 +426,9 @@ checksum = "ce23b50ad8242c51a442f3ff322d56b02f08852c77e4c0b4d3fd684abc89c683" [[package]] name = "indexmap" -version = "2.5.0" +version = "2.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68b900aa2f7301e21c36462b170ee99994de34dff39a4a6a528e80e7376d07e5" +checksum = "cea70ddb795996207ad57735b50c5982d8844f38ba9ee5f1aedcfb708a2aa11e" dependencies = [ "equivalent", "hashbrown", @@ -524,11 +519,14 @@ dependencies = [ [[package]] name = "petgraph" -version = "0.6.5" -source = "git+https://github.com/programmerjake/petgraph.git?rev=258ea8071209a924b73fe96f9f87a3b7b45cbc9f#258ea8071209a924b73fe96f9f87a3b7b45cbc9f" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a98c6720655620a521dcc722d0ad66cd8afd5d86e34a89ef691c50b7b24de06" dependencies = [ "fixedbitset", + "hashbrown", "indexmap", + "serde", ] [[package]] @@ -893,23 +891,3 @@ checksum = "05f360fc0b24296329c78fda852a1e9ae82de9cf7b27dae4b7f62f118f77b9ed" dependencies = [ "tap", ] - -[[package]] -name = "zerocopy" -version = "0.7.32" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "74d4d3961e53fa4c9a25a8637fc2bfaf2595b3d3ae34875568a5cf64787716be" -dependencies = [ - "zerocopy-derive", -] - -[[package]] -name = "zerocopy-derive" -version = "0.7.32" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9ce1b18ccd8e73a9321186f97e46f9f04b778851177567b1975109d26a08d2a6" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] diff --git a/Cargo.toml b/Cargo.toml index 54de3a8..d681425 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,14 +23,13 @@ blake3 = { version = "1.5.4", features = ["serde"] } clap = { version = "4.5.9", features = ["derive", "env", "string"] } ctor = "0.2.8" eyre = "0.6.12" -hashbrown = "0.14.3" +hashbrown = "0.15.2" indexmap = { version = "2.5.0", features = ["serde"] } jobslot = "0.2.19" num-bigint = "0.4.6" num-traits = "0.2.16" os_pipe = "1.2.1" -# TODO: switch back to crates.io once petgraph accepts PR #684 and releases a new version -petgraph = { git = "https://github.com/programmerjake/petgraph.git", rev = "258ea8071209a924b73fe96f9f87a3b7b45cbc9f" } +petgraph = "0.8.1" prettyplease = "0.2.20" proc-macro2 = "1.0.83" quote = "1.0.36" diff --git a/crates/fayalite/Cargo.toml b/crates/fayalite/Cargo.toml index 2652792..f176698 100644 --- a/crates/fayalite/Cargo.toml +++ b/crates/fayalite/Cargo.toml @@ -40,6 +40,7 @@ fayalite-visit-gen.workspace = true [features] unstable-doc = [] +unstable-test-hasher = [] [package.metadata.docs.rs] features = ["unstable-doc"] diff --git a/crates/fayalite/src/annotations.rs b/crates/fayalite/src/annotations.rs index 8eff4a0..1c517ae 100644 --- a/crates/fayalite/src/annotations.rs +++ b/crates/fayalite/src/annotations.rs @@ -12,7 +12,7 @@ use std::{ ops::Deref, }; -#[derive(Clone)] +#[derive(Clone, Debug)] struct CustomFirrtlAnnotationFieldsImpl { value: serde_json::Map, serialized: Interned, diff --git a/crates/fayalite/src/bundle.rs b/crates/fayalite/src/bundle.rs index 0fd89f1..240c0c6 100644 --- a/crates/fayalite/src/bundle.rs +++ b/crates/fayalite/src/bundle.rs @@ -14,9 +14,9 @@ use crate::{ impl_match_variant_as_self, CanonicalType, MatchVariantWithoutScope, OpaqueSimValue, StaticType, Type, TypeProperties, TypeWithDeref, }, + util::HashMap, }; use bitvec::{slice::BitSlice, vec::BitVec}; -use hashbrown::HashMap; use serde::{Deserialize, Serialize}; use std::{fmt, marker::PhantomData}; @@ -160,7 +160,7 @@ impl Default for BundleTypePropertiesBuilder { impl Bundle { #[track_caller] pub fn new(fields: Interned<[BundleField]>) -> Self { - let mut name_indexes = HashMap::with_capacity(fields.len()); + let mut name_indexes = HashMap::with_capacity_and_hasher(fields.len(), Default::default()); let mut field_offsets = Vec::with_capacity(fields.len()); let mut type_props_builder = BundleTypePropertiesBuilder::new(); for (index, &BundleField { name, flipped, ty }) in fields.iter().enumerate() { diff --git a/crates/fayalite/src/cli.rs b/crates/fayalite/src/cli.rs index 66741ef..1f208a8 100644 --- a/crates/fayalite/src/cli.rs +++ b/crates/fayalite/src/cli.rs @@ -258,7 +258,7 @@ pub struct VerilogArgs { default_value = "firtool", env = "FIRTOOL", value_hint = ValueHint::CommandName, - value_parser = OsStringValueParser::new().try_map(which::which) + value_parser = OsStringValueParser::new().try_map(which) )] pub firtool: PathBuf, #[arg(long)] @@ -428,6 +428,13 @@ impl clap::Args for FormalAdjustArgs { } } +fn which(v: std::ffi::OsString) -> which::Result { + #[cfg(not(miri))] + return which::which(v); + #[cfg(miri)] + return Ok(Path::new("/").join(v)); +} + #[derive(Parser, Clone)] #[non_exhaustive] pub struct FormalArgs { @@ -438,7 +445,7 @@ pub struct FormalArgs { default_value = "sby", env = "SBY", value_hint = ValueHint::CommandName, - value_parser = OsStringValueParser::new().try_map(which::which) + value_parser = OsStringValueParser::new().try_map(which) )] pub sby: PathBuf, #[arg(long)] diff --git a/crates/fayalite/src/enum_.rs b/crates/fayalite/src/enum_.rs index 6205855..36b5aa7 100644 --- a/crates/fayalite/src/enum_.rs +++ b/crates/fayalite/src/enum_.rs @@ -19,9 +19,9 @@ use crate::{ CanonicalType, MatchVariantAndInactiveScope, OpaqueSimValue, StaticType, Type, TypeProperties, }, + util::HashMap, }; use bitvec::{order::Lsb0, slice::BitSlice, view::BitView}; -use hashbrown::HashMap; use serde::{Deserialize, Serialize}; use std::{convert::Infallible, fmt, iter::FusedIterator, sync::Arc}; @@ -193,7 +193,8 @@ impl Default for EnumTypePropertiesBuilder { impl Enum { #[track_caller] pub fn new(variants: Interned<[EnumVariant]>) -> Self { - let mut name_indexes = HashMap::with_capacity(variants.len()); + let mut name_indexes = + HashMap::with_capacity_and_hasher(variants.len(), Default::default()); let mut type_props_builder = EnumTypePropertiesBuilder::new(); for (index, EnumVariant { name, ty }) in variants.iter().enumerate() { if let Some(old_index) = name_indexes.insert(*name, index) { diff --git a/crates/fayalite/src/expr.rs b/crates/fayalite/src/expr.rs index f511c97..e070674 100644 --- a/crates/fayalite/src/expr.rs +++ b/crates/fayalite/src/expr.rs @@ -274,6 +274,17 @@ pub struct Expr { impl fmt::Debug for Expr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + #[cfg(debug_assertions)] + { + let Self { + __enum, + __ty, + __flow, + } = self; + let expr_ty = __ty.canonical(); + let enum_ty = __enum.to_expr().__ty; + assert_eq!(expr_ty, enum_ty, "expr ty mismatch:\nExpr {{\n__enum: {__enum:?},\n__ty: {__ty:?},\n__flow: {__flow:?}\n}}"); + } self.__enum.fmt(f) } } diff --git a/crates/fayalite/src/firrtl.rs b/crates/fayalite/src/firrtl.rs index d082187..d33c7a9 100644 --- a/crates/fayalite/src/firrtl.rs +++ b/crates/fayalite/src/firrtl.rs @@ -36,12 +36,11 @@ use crate::{ ty::{CanonicalType, Type}, util::{ const_str_array_is_strictly_ascending, BitSliceWriteWithBase, DebugAsRawString, - GenericConstBool, + GenericConstBool, HashMap, HashSet, }, }; use bitvec::slice::BitSlice; use clap::value_parser; -use hashbrown::{HashMap, HashSet}; use num_traits::Signed; use serde::Serialize; use std::{ @@ -2622,7 +2621,7 @@ fn export_impl( indent_depth: &indent_depth, indent: " ", }, - seen_modules: HashSet::new(), + seen_modules: HashSet::default(), unwritten_modules: VecDeque::new(), global_ns, module: ModuleState::default(), diff --git a/crates/fayalite/src/intern.rs b/crates/fayalite/src/intern.rs index 3780ad3..af91f0a 100644 --- a/crates/fayalite/src/intern.rs +++ b/crates/fayalite/src/intern.rs @@ -1,9 +1,9 @@ // SPDX-License-Identifier: LGPL-3.0-or-later // See Notices.txt for copyright information #![allow(clippy::type_complexity)] -use crate::intern::type_map::TypeIdMap; +use crate::{intern::type_map::TypeIdMap, util::DefaultBuildHasher}; use bitvec::{ptr::BitPtr, slice::BitSlice, vec::BitVec}; -use hashbrown::{hash_map::RawEntryMut, HashMap, HashTable}; +use hashbrown::HashTable; use serde::{Deserialize, Serialize}; use std::{ any::{Any, TypeId}, @@ -17,7 +17,7 @@ use std::{ sync::{Mutex, RwLock}, }; -pub mod type_map; +mod type_map; pub trait LazyInternedTrait: Send + Sync + Any { fn get(&self) -> Interned; @@ -316,8 +316,13 @@ pub trait Intern: Any + Send + Sync { } } +struct InternerState { + table: HashTable<&'static T>, + hasher: DefaultBuildHasher, +} + pub struct Interner { - map: Mutex>, + state: Mutex>, } impl Interner { @@ -330,7 +335,10 @@ impl Interner { impl Default for Interner { fn default() -> Self { Self { - map: Default::default(), + state: Mutex::new(InternerState { + table: HashTable::new(), + hasher: Default::default(), + }), } } } @@ -341,17 +349,16 @@ impl Interner { alloc: F, value: Cow<'_, T>, ) -> Interned { - let mut map = self.map.lock().unwrap(); - let hasher = map.hasher().clone(); - let hash = hasher.hash_one(&*value); - let inner = match map.raw_entry_mut().from_hash(hash, |k| **k == *value) { - RawEntryMut::Occupied(entry) => *entry.key(), - RawEntryMut::Vacant(entry) => { - *entry - .insert_with_hasher(hash, alloc(value), (), |k| hasher.hash_one(&**k)) - .0 - } - }; + let mut state = self.state.lock().unwrap(); + let InternerState { table, hasher } = &mut *state; + let inner = *table + .entry( + hasher.hash_one(&*value), + |k| **k == *value, + |k| hasher.hash_one(&**k), + ) + .or_insert_with(|| alloc(value)) + .get(); Interned { inner } } } @@ -742,7 +749,7 @@ pub trait MemoizeGeneric: 'static + Send + Sync + Hash + Eq + Copy { fn get_cow(self, input: Self::InputCow<'_>) -> Self::Output { static TYPE_ID_MAP: TypeIdMap = TypeIdMap::new(); let map: &RwLock<( - hashbrown::hash_map::DefaultHashBuilder, + DefaultBuildHasher, HashTable<(Self, Self::InputOwned, Self::Output)>, )> = TYPE_ID_MAP.get_or_insert_default(); fn hash_eq_key<'a, 'b, T: MemoizeGeneric>( diff --git a/crates/fayalite/src/intern/type_map.rs b/crates/fayalite/src/intern/type_map.rs index 48433af..945116b 100644 --- a/crates/fayalite/src/intern/type_map.rs +++ b/crates/fayalite/src/intern/type_map.rs @@ -1,10 +1,8 @@ // SPDX-License-Identifier: LGPL-3.0-or-later // See Notices.txt for copyright information -use hashbrown::HashMap; use std::{ any::{Any, TypeId}, hash::{BuildHasher, Hasher}, - ptr::NonNull, sync::RwLock, }; @@ -75,59 +73,36 @@ impl BuildHasher for TypeIdBuildHasher { } } -struct Value(NonNull); - -impl Value { - unsafe fn get_transmute_lifetime<'b>(&self) -> &'b (dyn Any + Send + Sync) { - unsafe { &*self.0.as_ptr() } - } - fn new(v: Box) -> Self { - unsafe { Self(NonNull::new_unchecked(Box::into_raw(v))) } - } -} - -unsafe impl Send for Value {} -unsafe impl Sync for Value {} - -impl Drop for Value { - fn drop(&mut self) { - unsafe { std::ptr::drop_in_place(self.0.as_ptr()) } - } -} - -pub struct TypeIdMap(RwLock>); +pub(crate) struct TypeIdMap( + RwLock>, +); impl TypeIdMap { - pub const fn new() -> Self { - Self(RwLock::new(HashMap::with_hasher(TypeIdBuildHasher))) + pub(crate) const fn new() -> Self { + Self(RwLock::new(hashbrown::HashMap::with_hasher( + TypeIdBuildHasher, + ))) } #[cold] - unsafe fn insert_slow( + fn insert_slow( &self, type_id: TypeId, make: fn() -> Box, - ) -> &(dyn Any + Sync + Send) { - let value = Value::new(make()); + ) -> &'static (dyn Any + Sync + Send) { + let value = Box::leak(make()); let mut write_guard = self.0.write().unwrap(); - unsafe { - write_guard - .entry(type_id) - .or_insert(value) - .get_transmute_lifetime() - } + *write_guard.entry(type_id).or_insert(value) } - pub fn get_or_insert_default(&self) -> &T { + pub(crate) fn get_or_insert_default(&self) -> &T { let type_id = TypeId::of::(); let read_guard = self.0.read().unwrap(); - let retval = read_guard - .get(&type_id) - .map(|v| unsafe { Value::get_transmute_lifetime(v) }); + let retval = read_guard.get(&type_id).map(|v| *v); drop(read_guard); let retval = match retval { Some(retval) => retval, - None => unsafe { self.insert_slow(type_id, move || Box::new(T::default())) }, + None => self.insert_slow(type_id, move || Box::new(T::default())), }; - unsafe { &*(retval as *const dyn Any as *const T) } + retval.downcast_ref().expect("known to have correct TypeId") } } diff --git a/crates/fayalite/src/memory.rs b/crates/fayalite/src/memory.rs index 1101157..622ffc6 100644 --- a/crates/fayalite/src/memory.rs +++ b/crates/fayalite/src/memory.rs @@ -470,7 +470,7 @@ pub enum ReadUnderWrite { Undefined, } -#[derive(Copy, Clone, PartialEq, Eq, Hash)] +#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] struct MemImpl { scoped_name: ScopedNameId, source_location: SourceLocation, diff --git a/crates/fayalite/src/module.rs b/crates/fayalite/src/module.rs index 1fcb529..920b0af 100644 --- a/crates/fayalite/src/module.rs +++ b/crates/fayalite/src/module.rs @@ -24,10 +24,10 @@ use crate::{ sim::{ExternModuleSimGenerator, ExternModuleSimulation}, source_location::SourceLocation, ty::{CanonicalType, Type}, - util::ScopedRef, + util::{HashMap, HashSet, ScopedRef}, wire::{IncompleteWire, Wire}, }; -use hashbrown::{hash_map::Entry, HashMap, HashSet}; +use hashbrown::hash_map::Entry; use num_bigint::BigInt; use std::{ cell::RefCell, @@ -1498,7 +1498,7 @@ impl TargetState { .collect(), }, CanonicalType::PhantomConst(_) => TargetStateInner::Decomposed { - subtargets: HashMap::new(), + subtargets: HashMap::default(), }, CanonicalType::Array(ty) => TargetStateInner::Decomposed { subtargets: (0..ty.len()) @@ -1864,7 +1864,7 @@ impl Module { AssertValidityState { module: self.canonical(), blocks: vec![], - target_states: HashMap::with_capacity(64), + target_states: HashMap::with_capacity_and_hasher(64, Default::default()), } .assert_validity(); } @@ -2125,8 +2125,8 @@ impl ModuleBuilder { incomplete_declarations: vec![], stmts: vec![], }], - annotations_map: HashMap::new(), - memory_map: HashMap::new(), + annotations_map: HashMap::default(), + memory_map: HashMap::default(), }, }), }; @@ -2136,7 +2136,7 @@ impl ModuleBuilder { impl_: RefCell::new(ModuleBuilderImpl { body, io: vec![], - io_indexes: HashMap::new(), + io_indexes: HashMap::default(), module_annotations: vec![], }), }; diff --git a/crates/fayalite/src/module/transform/deduce_resets.rs b/crates/fayalite/src/module/transform/deduce_resets.rs index a70dc33..5fb829e 100644 --- a/crates/fayalite/src/module/transform/deduce_resets.rs +++ b/crates/fayalite/src/module/transform/deduce_resets.rs @@ -24,8 +24,9 @@ use crate::{ }, prelude::*, reset::{ResetType, ResetTypeDispatch}, + util::{HashMap, HashSet}, }; -use hashbrown::{hash_map::Entry, HashMap, HashSet}; +use hashbrown::hash_map::Entry; use num_bigint::BigInt; use petgraph::unionfind::UnionFind; use std::{fmt, marker::PhantomData}; @@ -2251,9 +2252,9 @@ pub fn deduce_resets( fallback_to_sync_reset: bool, ) -> Result>, DeduceResetsError> { let mut state = State { - modules_added_to_graph: HashSet::new(), - substituted_modules: HashMap::new(), - expr_resets: HashMap::new(), + modules_added_to_graph: HashSet::default(), + substituted_modules: HashMap::default(), + expr_resets: HashMap::default(), reset_graph: ResetGraph::default(), fallback_to_sync_reset, }; diff --git a/crates/fayalite/src/module/transform/simplify_enums.rs b/crates/fayalite/src/module/transform/simplify_enums.rs index e8b6168..333451d 100644 --- a/crates/fayalite/src/module/transform/simplify_enums.rs +++ b/crates/fayalite/src/module/transform/simplify_enums.rs @@ -18,10 +18,10 @@ use crate::{ }, source_location::SourceLocation, ty::{CanonicalType, Type}, + util::HashMap, wire::Wire, }; use core::fmt; -use hashbrown::HashMap; #[derive(Debug)] pub enum SimplifyEnumsError { @@ -965,8 +965,8 @@ pub fn simplify_enums( kind: SimplifyEnumsKind, ) -> Result>, SimplifyEnumsError> { module.fold(&mut State { - enum_types: HashMap::new(), - replacement_mem_ports: HashMap::new(), + enum_types: HashMap::default(), + replacement_mem_ports: HashMap::default(), kind, module_state_stack: vec![], }) diff --git a/crates/fayalite/src/module/transform/simplify_memories.rs b/crates/fayalite/src/module/transform/simplify_memories.rs index 101385e..6357843 100644 --- a/crates/fayalite/src/module/transform/simplify_memories.rs +++ b/crates/fayalite/src/module/transform/simplify_memories.rs @@ -14,11 +14,10 @@ use crate::{ }, source_location::SourceLocation, ty::{CanonicalType, Type}, - util::MakeMutSlice, + util::{HashMap, MakeMutSlice}, wire::Wire, }; use bitvec::{slice::BitSlice, vec::BitVec}; -use hashbrown::HashMap; use std::{ convert::Infallible, fmt::Write, @@ -897,7 +896,7 @@ impl Folder for State { module, ModuleState { output_module: None, - memories: HashMap::new(), + memories: HashMap::default(), }, ); let mut this = PushedState::push_module(self, module); diff --git a/crates/fayalite/src/sim.rs b/crates/fayalite/src/sim.rs index 563cd37..6659391 100644 --- a/crates/fayalite/src/sim.rs +++ b/crates/fayalite/src/sim.rs @@ -41,10 +41,9 @@ use crate::{ value::SimValue, }, ty::StaticType, - util::{BitSliceWriteWithBase, DebugAsDisplay}, + util::{BitSliceWriteWithBase, DebugAsDisplay, HashMap, HashSet}, }; use bitvec::{bits, order::Lsb0, slice::BitSlice, vec::BitVec, view::BitView}; -use hashbrown::{HashMap, HashSet}; use num_bigint::BigInt; use num_traits::{Signed, Zero}; use petgraph::{ @@ -580,8 +579,9 @@ impl Assignments { big_slots, }) = self.slot_readers(); AssignmentsElements { - node_indexes: HashMap::with_capacity( + node_indexes: HashMap::with_capacity_and_hasher( self.assignments().len() + small_slots.len() + big_slots.len(), + Default::default(), ), nodes: self.node_references(), edges: self.edge_references(), @@ -1022,6 +1022,16 @@ impl VisitMap for AssignmentsVisitMap { AssignmentOrSlotIndex::BigSlot(slot) => self.slots.contains(slot), } } + + fn unvisit(&mut self, n: AssignmentOrSlotIndex) -> bool { + match n { + AssignmentOrSlotIndex::AssignmentIndex(assignment_index) => { + mem::replace(&mut self.assignments[assignment_index], false) + } + AssignmentOrSlotIndex::SmallSlot(slot) => self.slots.remove(slot), + AssignmentOrSlotIndex::BigSlot(slot) => self.slots.remove(slot), + } + } } impl Visitable for Assignments { @@ -1676,18 +1686,18 @@ impl Compiler { insns: Insns::new(), original_base_module, base_module, - modules: HashMap::new(), + modules: HashMap::default(), extern_modules: Vec::new(), - compiled_values: HashMap::new(), - compiled_exprs: HashMap::new(), - compiled_exprs_to_values: HashMap::new(), - decl_conditions: HashMap::new(), - compiled_values_to_dyn_array_indexes: HashMap::new(), - compiled_value_bool_dest_is_small_map: HashMap::new(), + compiled_values: HashMap::default(), + compiled_exprs: HashMap::default(), + compiled_exprs_to_values: HashMap::default(), + decl_conditions: HashMap::default(), + compiled_values_to_dyn_array_indexes: HashMap::default(), + compiled_value_bool_dest_is_small_map: HashMap::default(), assignments: Assignments::default(), clock_triggers: Vec::new(), - compiled_value_to_clock_trigger_map: HashMap::new(), - enum_discriminants: HashMap::new(), + compiled_value_to_clock_trigger_map: HashMap::default(), + enum_discriminants: HashMap::default(), registers: Vec::new(), traces: SimTraces(Vec::new()), memories: Vec::new(), @@ -5783,7 +5793,7 @@ where } } -#[derive(Clone, PartialEq, Eq, Hash)] +#[derive(Clone, PartialEq, Eq, Hash, Debug)] struct SimTrace { kind: K, state: S, @@ -5976,8 +5986,8 @@ impl SimulationModuleState { fn new(base_targets: impl IntoIterator)>) -> Self { let mut retval = Self { base_targets: Vec::new(), - uninitialized_ios: HashMap::new(), - io_targets: HashMap::new(), + uninitialized_ios: HashMap::default(), + io_targets: HashMap::default(), did_initial_settle: false, }; for (base_target, value) in base_targets { @@ -6207,7 +6217,7 @@ impl Default for EarliestWaitTargets { Self { settle: false, instant: None, - changes: HashMap::new(), + changes: HashMap::default(), } } } @@ -6217,14 +6227,14 @@ impl EarliestWaitTargets { Self { settle: true, instant: None, - changes: HashMap::new(), + changes: HashMap::default(), } } fn instant(instant: SimInstant) -> Self { Self { settle: false, instant: Some(instant), - changes: HashMap::new(), + changes: HashMap::default(), } } fn len(&self) -> usize { diff --git a/crates/fayalite/src/sim/interpreter.rs b/crates/fayalite/src/sim/interpreter.rs index 22f6f5f..de582f0 100644 --- a/crates/fayalite/src/sim/interpreter.rs +++ b/crates/fayalite/src/sim/interpreter.rs @@ -7,10 +7,9 @@ use crate::{ intern::{Intern, Interned, Memoize}, source_location::SourceLocation, ty::CanonicalType, - util::get_many_mut, + util::{get_many_mut, HashMap, HashSet}, }; use bitvec::{boxed::BitBox, slice::BitSlice}; -use hashbrown::{HashMap, HashSet}; use num_bigint::BigInt; use num_traits::{One, Signed, ToPrimitive, Zero}; use std::{ diff --git a/crates/fayalite/src/sim/vcd.rs b/crates/fayalite/src/sim/vcd.rs index fde30be..fcf6743 100644 --- a/crates/fayalite/src/sim/vcd.rs +++ b/crates/fayalite/src/sim/vcd.rs @@ -14,9 +14,10 @@ use crate::{ TraceModuleIO, TraceReg, TraceSInt, TraceScalar, TraceScalarId, TraceScope, TraceSyncReset, TraceUInt, TraceWire, TraceWriter, TraceWriterDecls, }, + util::HashMap, }; use bitvec::{order::Lsb0, slice::BitSlice}; -use hashbrown::{hash_map::Entry, HashMap}; +use hashbrown::hash_map::Entry; use std::{ fmt::{self, Write as _}, io, mem, diff --git a/crates/fayalite/src/source_location.rs b/crates/fayalite/src/source_location.rs index d143f22..1a168b1 100644 --- a/crates/fayalite/src/source_location.rs +++ b/crates/fayalite/src/source_location.rs @@ -2,9 +2,8 @@ // See Notices.txt for copyright information use crate::{ intern::{Intern, Interned}, - util::DebugAsDisplay, + util::{DebugAsDisplay, HashMap}, }; -use hashbrown::HashMap; use std::{cell::RefCell, fmt, num::NonZeroUsize, panic, path::Path}; #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -97,7 +96,7 @@ impl NormalizeFilesForTestsState { fn new() -> Self { Self { test_position: panic::Location::caller(), - file_pattern_matches: HashMap::new(), + file_pattern_matches: HashMap::default(), } } } @@ -143,7 +142,7 @@ impl From<&'_ panic::Location<'_>> for SourceLocation { map.entry_ref(file) .or_insert_with(|| NormalizedFileForTestState { file_name_id: NonZeroUsize::new(len + 1).unwrap(), - positions_map: HashMap::new(), + positions_map: HashMap::default(), }); file_str = m.generate_file_name(file_state.file_name_id); file = &file_str; diff --git a/crates/fayalite/src/testing.rs b/crates/fayalite/src/testing.rs index 4517e34..b81bc3f 100644 --- a/crates/fayalite/src/testing.rs +++ b/crates/fayalite/src/testing.rs @@ -3,9 +3,9 @@ use crate::{ cli::{FormalArgs, FormalMode, FormalOutput, RunPhase}, firrtl::ExportOptions, + util::HashMap, }; use clap::Parser; -use hashbrown::HashMap; use serde::Deserialize; use std::{ fmt::Write, @@ -87,7 +87,7 @@ fn get_assert_formal_target_path(test_name: &dyn std::fmt::Display) -> PathBuf { let index = *DIRS .lock() .unwrap() - .get_or_insert_with(HashMap::new) + .get_or_insert_with(HashMap::default) .entry_ref(&dir) .and_modify(|v| *v += 1) .or_insert(0); diff --git a/crates/fayalite/src/util.rs b/crates/fayalite/src/util.rs index 804ff19..233867e 100644 --- a/crates/fayalite/src/util.rs +++ b/crates/fayalite/src/util.rs @@ -8,6 +8,16 @@ mod const_usize; mod misc; mod scoped_ref; pub(crate) mod streaming_read_utf8; +mod test_hasher; + +// allow easily switching the hasher crate-wide for testing +#[cfg(feature = "unstable-test-hasher")] +pub type DefaultBuildHasher = test_hasher::DefaultBuildHasher; +#[cfg(not(feature = "unstable-test-hasher"))] +pub(crate) type DefaultBuildHasher = hashbrown::DefaultHashBuilder; + +pub(crate) type HashMap = hashbrown::HashMap; +pub(crate) type HashSet = hashbrown::HashSet; #[doc(inline)] pub use const_bool::{ConstBool, ConstBoolDispatch, ConstBoolDispatchTag, GenericConstBool}; diff --git a/crates/fayalite/src/util/test_hasher.rs b/crates/fayalite/src/util/test_hasher.rs new file mode 100644 index 0000000..20df5b7 --- /dev/null +++ b/crates/fayalite/src/util/test_hasher.rs @@ -0,0 +1,240 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later +// See Notices.txt for copyright information +#![cfg(feature = "unstable-test-hasher")] + +use std::{ + fmt::Write as _, + hash::{BuildHasher, Hash, Hasher}, + io::Write as _, + marker::PhantomData, + sync::LazyLock, +}; + +type BoxDynHasher = Box; +type BoxDynBuildHasher = Box; +type BoxDynMakeBuildHasher = Box BoxDynBuildHasher + Send + Sync>; + +trait TryGetDynBuildHasher: Copy { + type Type; + fn try_get_make_build_hasher(self) -> Option; +} + +impl TryGetDynBuildHasher for PhantomData { + type Type = T; + fn try_get_make_build_hasher(self) -> Option { + None + } +} + +impl + Send + Sync + 'static + Clone> + TryGetDynBuildHasher for &'_ PhantomData +{ + type Type = T; + fn try_get_make_build_hasher(self) -> Option { + Some(Box::new(|| Box::>::default())) + } +} + +#[derive(Default, Clone)] +struct DynBuildHasher(T); + +trait DynBuildHasherTrait: BuildHasher { + fn clone_dyn_build_hasher(&self) -> BoxDynBuildHasher; +} + +impl> BuildHasher for DynBuildHasher { + type Hasher = BoxDynHasher; + + fn build_hasher(&self) -> Self::Hasher { + Box::new(self.0.build_hasher()) + } + + fn hash_one(&self, x: T) -> u64 { + self.0.hash_one(x) + } +} + +impl DynBuildHasherTrait for DynBuildHasher +where + Self: Clone + BuildHasher + Send + Sync + 'static, +{ + fn clone_dyn_build_hasher(&self) -> BoxDynBuildHasher { + Box::new(self.clone()) + } +} + +pub struct DefaultBuildHasher(BoxDynBuildHasher); + +impl Clone for DefaultBuildHasher { + fn clone(&self) -> Self { + DefaultBuildHasher(self.0.clone_dyn_build_hasher()) + } +} + +const ENV_VAR_NAME: &'static str = "FAYALITE_TEST_HASHER"; + +struct EnvVarValue { + key: &'static str, + try_get_make_build_hasher: fn() -> Option, + description: &'static str, +} + +macro_rules! env_var_value { + ( + key: $key:literal, + build_hasher: $build_hasher:ty, + description: $description:literal, + ) => { + EnvVarValue { + key: $key, + try_get_make_build_hasher: || { + // use rust method resolution to detect if $build_hasher is usable + // (e.g. hashbrown's hasher won't be usable without the right feature enabled) + (&PhantomData::>).try_get_make_build_hasher() + }, + description: $description, + } + }; +} + +#[derive(Default)] +struct AlwaysZeroHasher; + +impl Hasher for AlwaysZeroHasher { + fn write(&mut self, _bytes: &[u8]) {} + fn finish(&self) -> u64 { + 0 + } +} + +const ENV_VAR_VALUES: &'static [EnvVarValue] = &[ + env_var_value! { + key: "std", + build_hasher: std::hash::RandomState, + description: "use std::hash::RandomState", + }, + env_var_value! { + key: "hashbrown", + build_hasher: hashbrown::DefaultHashBuilder, + description: "use hashbrown's DefaultHashBuilder", + }, + env_var_value! { + key: "always_zero", + build_hasher: std::hash::BuildHasherDefault, + description: "use a hasher that always returns 0 for all hashes,\n \ + this is useful for checking that PartialEq impls are correct", + }, +]; + +fn report_bad_env_var(msg: impl std::fmt::Display) -> ! { + let mut msg = format!("{ENV_VAR_NAME}: {msg}\n"); + for &EnvVarValue { + key, + try_get_make_build_hasher, + description, + } in ENV_VAR_VALUES + { + let availability = match try_get_make_build_hasher() { + Some(_) => "available", + None => "unavailable", + }; + writeln!(msg, "{key}: ({availability})\n {description}").expect("can't fail"); + } + std::io::stderr() + .write_all(msg.as_bytes()) + .expect("should be able to write to stderr"); + std::process::abort(); +} + +impl Default for DefaultBuildHasher { + fn default() -> Self { + static DEFAULT_FN: LazyLock = LazyLock::new(|| { + let var = std::env::var_os(ENV_VAR_NAME); + let var = var.as_deref().unwrap_or("std".as_ref()); + for &EnvVarValue { + key, + try_get_make_build_hasher, + description: _, + } in ENV_VAR_VALUES + { + if var.as_encoded_bytes().eq_ignore_ascii_case(key.as_bytes()) { + return try_get_make_build_hasher().unwrap_or_else(|| { + report_bad_env_var(format_args!( + "unavailable hasher: {key} (is the appropriate feature enabled?)" + )); + }); + } + } + report_bad_env_var(format_args!("unrecognized hasher: {var:?}")); + }); + Self(DEFAULT_FN()) + } +} + +pub struct DefaultHasher(BoxDynHasher); + +impl BuildHasher for DefaultBuildHasher { + type Hasher = DefaultHasher; + + fn build_hasher(&self) -> Self::Hasher { + DefaultHasher(self.0.build_hasher()) + } +} + +impl Hasher for DefaultHasher { + fn finish(&self) -> u64 { + self.0.finish() + } + + fn write(&mut self, bytes: &[u8]) { + self.0.write(bytes) + } + + fn write_u8(&mut self, i: u8) { + self.0.write_u8(i) + } + + fn write_u16(&mut self, i: u16) { + self.0.write_u16(i) + } + + fn write_u32(&mut self, i: u32) { + self.0.write_u32(i) + } + + fn write_u64(&mut self, i: u64) { + self.0.write_u64(i) + } + + fn write_u128(&mut self, i: u128) { + self.0.write_u128(i) + } + + fn write_usize(&mut self, i: usize) { + self.0.write_usize(i) + } + + fn write_i8(&mut self, i: i8) { + self.0.write_i8(i) + } + + fn write_i16(&mut self, i: i16) { + self.0.write_i16(i) + } + + fn write_i32(&mut self, i: i32) { + self.0.write_i32(i) + } + + fn write_i64(&mut self, i: i64) { + self.0.write_i64(i) + } + + fn write_i128(&mut self, i: i128) { + self.0.write_i128(i) + } + + fn write_isize(&mut self, i: isize) { + self.0.write_isize(i) + } +} diff --git a/crates/fayalite/tests/ui/simvalue_is_not_internable.rs b/crates/fayalite/tests/ui/simvalue_is_not_internable.rs new file mode 100644 index 0000000..d40990f --- /dev/null +++ b/crates/fayalite/tests/ui/simvalue_is_not_internable.rs @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later +// See Notices.txt for copyright information + +//! check that SimValue can't be interned, since equality may ignore types + +use fayalite::{ + intern::{Intern, Interned}, + sim::value::SimValue, +}; + +fn f(v: SimValue<()>) -> Interned> { + Intern::intern_sized(v) +} + +fn main() {} diff --git a/crates/fayalite/tests/ui/simvalue_is_not_internable.stderr b/crates/fayalite/tests/ui/simvalue_is_not_internable.stderr new file mode 100644 index 0000000..eb8877b --- /dev/null +++ b/crates/fayalite/tests/ui/simvalue_is_not_internable.stderr @@ -0,0 +1,178 @@ +error[E0277]: `Cell` cannot be shared between threads safely + --> tests/ui/simvalue_is_not_internable.rs:11:26 + | +11 | fn f(v: SimValue<()>) -> Interned> { + | ^^^^^^^^^^^^^^^^^^^^^^ `Cell` cannot be shared between threads safely + | + = help: within `SimValue<()>`, the trait `Sync` is not implemented for `Cell`, which is required by `SimValue<()>: Sync` + = note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` +note: required because it appears within the type `util::alternating_cell::AlternatingCell>` + --> src/util/alternating_cell.rs + | + | pub(crate) struct AlternatingCell { + | ^^^^^^^^^^^^^^^ +note: required because it appears within the type `SimValue<()>` + --> src/sim/value.rs + | + | pub struct SimValue { + | ^^^^^^^^ +note: required by a bound in `fayalite::intern::Interned` + --> src/intern.rs + | + | pub struct Interned { + | ^^^^ required by this bound in `Interned` + +error[E0277]: `UnsafeCell>` cannot be shared between threads safely + --> tests/ui/simvalue_is_not_internable.rs:11:26 + | +11 | fn f(v: SimValue<()>) -> Interned> { + | ^^^^^^^^^^^^^^^^^^^^^^ `UnsafeCell>` cannot be shared between threads safely + | + = help: within `SimValue<()>`, the trait `Sync` is not implemented for `UnsafeCell>`, which is required by `SimValue<()>: Sync` +note: required because it appears within the type `util::alternating_cell::AlternatingCell>` + --> src/util/alternating_cell.rs + | + | pub(crate) struct AlternatingCell { + | ^^^^^^^^^^^^^^^ +note: required because it appears within the type `SimValue<()>` + --> src/sim/value.rs + | + | pub struct SimValue { + | ^^^^^^^^ +note: required by a bound in `fayalite::intern::Interned` + --> src/intern.rs + | + | pub struct Interned { + | ^^^^ required by this bound in `Interned` + +error[E0277]: the trait bound `SimValue<()>: Intern` is not satisfied + --> tests/ui/simvalue_is_not_internable.rs:12:26 + | +12 | Intern::intern_sized(v) + | -------------------- ^ the trait `Hash` is not implemented for `SimValue<()>`, which is required by `SimValue<()>: Intern` + | | + | required by a bound introduced by this call + | + = help: the following other types implement trait `Intern`: + BitSlice + [T] + str + = note: required for `SimValue<()>` to implement `Intern` + +error[E0277]: the trait bound `SimValue<()>: Intern` is not satisfied + --> tests/ui/simvalue_is_not_internable.rs:12:26 + | +12 | Intern::intern_sized(v) + | -------------------- ^ the trait `std::cmp::Eq` is not implemented for `SimValue<()>`, which is required by `SimValue<()>: Intern` + | | + | required by a bound introduced by this call + | + = help: the following other types implement trait `Intern`: + BitSlice + [T] + str + = note: required for `SimValue<()>` to implement `Intern` + +error[E0277]: `Cell` cannot be shared between threads safely + --> tests/ui/simvalue_is_not_internable.rs:12:26 + | +12 | Intern::intern_sized(v) + | -------------------- ^ `Cell` cannot be shared between threads safely + | | + | required by a bound introduced by this call + | + = help: within `SimValue<()>`, the trait `Sync` is not implemented for `Cell`, which is required by `SimValue<()>: Sync` + = note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` +note: required because it appears within the type `util::alternating_cell::AlternatingCell>` + --> src/util/alternating_cell.rs + | + | pub(crate) struct AlternatingCell { + | ^^^^^^^^^^^^^^^ +note: required because it appears within the type `SimValue<()>` + --> src/sim/value.rs + | + | pub struct SimValue { + | ^^^^^^^^ +note: required by a bound in `intern_sized` + --> src/intern.rs + | + | pub trait Intern: Any + Send + Sync { + | ^^^^ required by this bound in `Intern::intern_sized` + | fn intern(&self) -> Interned; + | fn intern_sized(self) -> Interned + | ------------ required by a bound in this associated function + +error[E0277]: `UnsafeCell>` cannot be shared between threads safely + --> tests/ui/simvalue_is_not_internable.rs:12:26 + | +12 | Intern::intern_sized(v) + | -------------------- ^ `UnsafeCell>` cannot be shared between threads safely + | | + | required by a bound introduced by this call + | + = help: within `SimValue<()>`, the trait `Sync` is not implemented for `UnsafeCell>`, which is required by `SimValue<()>: Sync` +note: required because it appears within the type `util::alternating_cell::AlternatingCell>` + --> src/util/alternating_cell.rs + | + | pub(crate) struct AlternatingCell { + | ^^^^^^^^^^^^^^^ +note: required because it appears within the type `SimValue<()>` + --> src/sim/value.rs + | + | pub struct SimValue { + | ^^^^^^^^ +note: required by a bound in `intern_sized` + --> src/intern.rs + | + | pub trait Intern: Any + Send + Sync { + | ^^^^ required by this bound in `Intern::intern_sized` + | fn intern(&self) -> Interned; + | fn intern_sized(self) -> Interned + | ------------ required by a bound in this associated function + +error[E0277]: `Cell` cannot be shared between threads safely + --> tests/ui/simvalue_is_not_internable.rs:12:5 + | +12 | Intern::intern_sized(v) + | ^^^^^^^^^^^^^^^^^^^^^^^ `Cell` cannot be shared between threads safely + | + = help: within `SimValue<()>`, the trait `Sync` is not implemented for `Cell`, which is required by `SimValue<()>: Sync` + = note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` +note: required because it appears within the type `util::alternating_cell::AlternatingCell>` + --> src/util/alternating_cell.rs + | + | pub(crate) struct AlternatingCell { + | ^^^^^^^^^^^^^^^ +note: required because it appears within the type `SimValue<()>` + --> src/sim/value.rs + | + | pub struct SimValue { + | ^^^^^^^^ +note: required by a bound in `fayalite::intern::Interned` + --> src/intern.rs + | + | pub struct Interned { + | ^^^^ required by this bound in `Interned` + +error[E0277]: `UnsafeCell>` cannot be shared between threads safely + --> tests/ui/simvalue_is_not_internable.rs:12:5 + | +12 | Intern::intern_sized(v) + | ^^^^^^^^^^^^^^^^^^^^^^^ `UnsafeCell>` cannot be shared between threads safely + | + = help: within `SimValue<()>`, the trait `Sync` is not implemented for `UnsafeCell>`, which is required by `SimValue<()>: Sync` +note: required because it appears within the type `util::alternating_cell::AlternatingCell>` + --> src/util/alternating_cell.rs + | + | pub(crate) struct AlternatingCell { + | ^^^^^^^^^^^^^^^ +note: required because it appears within the type `SimValue<()>` + --> src/sim/value.rs + | + | pub struct SimValue { + | ^^^^^^^^ +note: required by a bound in `fayalite::intern::Interned` + --> src/intern.rs + | + | pub struct Interned { + | ^^^^ required by this bound in `Interned`