tamer: asg::graph::object::rel: Hash impls for ObjectIndexTo{,Tree}

All ObjectIndex-like objects hash using only the underlying identifier,
which ultimately boils down to a `NodeIndex` (petgraph), which is just a
u32.  And so in that sense, the only purpose we have for hashing it is to
(a) reduce the space required to store mappings, and (b) compose with other
`Hash`es.

DEV-13708
main
Mike Gerwitz 2023-04-04 14:38:37 -04:00
parent 3660c15d5a
commit a738a05461
3 changed files with 103 additions and 17 deletions

View File

@ -22,8 +22,8 @@
//! ![Visualization of ASG ontology](../ontviz.svg)
use self::object::{
DynObjectRel, ObjectIndexRelTo, ObjectRelFrom, ObjectRelTy,
ObjectRelatable, Root,
DynObjectRel, ObjectIndexRelTo, ObjectIndexToTree, ObjectIndexTreeRelTo,
ObjectRelFrom, ObjectRelTy, ObjectRelatable, Root,
};
use super::{
@ -124,7 +124,7 @@ pub struct Asg {
/// Note that,
/// while we store [`NodeIndex`] internally,
/// the public API encapsulates it within an [`ObjectIndex`].
index: FxHashMap<(SymbolId, NodeIndex<Ix>), NodeIndex<Ix>>,
index: FxHashMap<(SymbolId, ObjectIndexToTree<Ident>), ObjectIndex<Ident>>,
/// The root node used for reachability analysis and topological
/// sorting.
@ -207,16 +207,45 @@ impl Asg {
/// After an identifier is indexed it is not expected to be reassigned
/// to another node.
/// Debug builds contain an assertion that will panic in this instance.
fn index_identifier(
pub(super) fn index_identifier<
OS: ObjectIndexTreeRelTo<Ident>,
S: Into<SymbolId>,
>(
&mut self,
imm_env: NodeIndex<Ix>,
name: SymbolId,
node: NodeIndex<Ix>,
imm_env: OS,
name: S,
oi: ObjectIndex<Ident>,
) {
let prev = self.index.insert((name, imm_env), node);
let sym = name.into();
let prev = self.index.insert((sym, imm_env.into()), oi);
// We should never overwrite indexes
debug_assert!(prev.is_none());
#[allow(unused_variables)] // used only for debug
if let Some(prev_oi) = prev {
crate::debug_diagnostic_panic!(
vec![
imm_env.into().note("at this scope boundary"),
prev_oi.note("previously indexed identifier was here"),
oi.internal_error(
"this identifier has already been indexed at the above scope boundary"
),
oi.help(
"this is a bug in the system responsible for analyzing \
identifier scope;"
),
oi.help(
" you can try to work around it by duplicating the definition of "
),
oi.help(
format!(
" {} as a _new_ identifier with a different name.",
TtQuote::wrap(sym),
)
),
],
"re-indexing of identifier at scope boundary",
);
}
}
/// Lookup `ident` or add a missing identifier to the graph relative to
@ -233,16 +262,22 @@ impl Asg {
/// you must resolve the [`Ident`] object and inspect it.
///
/// See [`Ident::declare`] for more information.
pub(super) fn lookup_or_missing<OS: ObjectIndexRelTo<Ident>>(
pub(super) fn lookup_or_missing<OS: ObjectIndexTreeRelTo<Ident>>(
&mut self,
imm_env: OS,
name: SPair,
) -> ObjectIndex<Ident> {
self.lookup(imm_env, name).unwrap_or_else(|| {
let index = self.graph.add_node(Ident::declare(name).into());
let oi = ObjectIndex::new(index, name.span());
self.index_identifier(self.root_node, name.symbol(), index);
ObjectIndex::new(index, name.span())
self.index_identifier(
ObjectIndex::<Root>::new(self.root_node, name.span()),
name.symbol(),
oi,
);
oi
})
}
@ -741,14 +776,14 @@ impl Asg {
/// compilation unit,
/// which is a package.
#[inline]
pub(super) fn lookup<OS: ObjectIndexRelTo<Ident>>(
pub(super) fn lookup<OS: ObjectIndexTreeRelTo<Ident>>(
&self,
imm_env: OS,
id: SPair,
) -> Option<ObjectIndex<Ident>> {
self.index
.get(&(id.symbol(), imm_env.widen().into()))
.map(|&ni| ObjectIndex::new(ni, id.span()))
.get(&(id.symbol(), imm_env.into()))
.map(|&ni| ni.overwrite(id.span()))
}
/// Attempt to retrieve an identifier from the graph by name utilizing

View File

@ -125,6 +125,7 @@ use petgraph::graph::NodeIndex;
use std::{
convert::Infallible,
fmt::{Debug, Display},
hash::{Hash, Hasher},
marker::PhantomData,
};
@ -466,7 +467,7 @@ where
/// event that the object somehow becomes unavailable for later
/// operations.
///
/// _The span is not accounted for in [`PartialEq`]_,
/// _The span is not accounted for in [`PartialEq`] or [`Hash`]_,
/// since it represents the context in which the [`ObjectIndex`] was
/// retrieved,
/// and the span associated with the underlying [`Object`] may evolve
@ -794,6 +795,14 @@ impl<O: ObjectKind> PartialEq for ObjectIndex<O> {
impl<O: ObjectKind> Eq for ObjectIndex<O> {}
impl<O: ObjectKind> Hash for ObjectIndex<O> {
fn hash<H: Hasher>(&self, state: &mut H) {
match self {
Self(index, _, _) => index.hash(state),
}
}
}
impl<O: ObjectKind> From<ObjectIndex<O>> for NodeIndex {
fn from(objref: ObjectIndex<O>) -> Self {
match objref {

View File

@ -801,6 +801,8 @@ pub use private::{ObjectIndexTo, ObjectIndexToTree};
/// Private inner module to ensure that nothing is able to bypass invariants
/// by constructing [`ObjectIndexTo`] manually.
mod private {
use std::hash::{Hash, Hasher};
use super::*;
/// Some [`ObjectIndex`] that is able to [`ObjectRelTo`] `OB`.
@ -830,7 +832,7 @@ mod private {
/// which will in turn lead to internal system failures when trying
/// to operate on the graph data down the line.
/// There are no memory safety concerns.
#[derive(Debug, PartialEq)]
#[derive(Debug)]
pub struct ObjectIndexTo<OB: ObjectRelatable>(
(ObjectIndex<Object>, ObjectRelTy),
PhantomData<OB>,
@ -852,6 +854,28 @@ mod private {
}
}
// Ignore metadata that should always be consistent with the underlying
// `ObjectIndex`.
impl<OB: ObjectRelatable> PartialEq for ObjectIndexTo<OB> {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(Self((oi, _), _), Self((oi_other, _), _)) => oi == oi_other,
}
}
}
impl<OB: ObjectRelatable> Eq for ObjectIndexTo<OB> {}
// Ignore metadata that should always be consistent with the underlying
// `ObjectIndex`.
impl<OB: ObjectRelatable> Hash for ObjectIndexTo<OB> {
fn hash<H: Hasher>(&self, state: &mut H) {
match self {
Self((oi, _), _) => oi.hash(state),
}
}
}
// Deriving `Clone`/`Copy` as of 2023-03 was introducing a
// `Clone`/`Copy` bound on `OB`.
impl<OB: ObjectRelatable> Clone for ObjectIndexTo<OB> {
@ -893,6 +917,24 @@ mod private {
// Deriving any of the below were introducing trait bounds on `OB`.
impl<OB: ObjectRelatable> PartialEq for ObjectIndexToTree<OB> {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(Self(oi), Self(oi_other)) => oi == oi_other,
}
}
}
impl<OB: ObjectRelatable> Eq for ObjectIndexToTree<OB> {}
impl<OB: ObjectRelatable> Hash for ObjectIndexToTree<OB> {
fn hash<H: Hasher>(&self, state: &mut H) {
match self {
Self(oi) => oi.hash(state),
}
}
}
impl<OB: ObjectRelatable> Clone for ObjectIndexToTree<OB> {
fn clone(&self) -> Self {
Self(self.0)