diff --git a/tamer/src/asg/air.rs b/tamer/src/asg/air.rs index 96058530..3bf39d2f 100644 --- a/tamer/src/asg/air.rs +++ b/tamer/src/asg/air.rs @@ -36,7 +36,10 @@ //! air such cringeworthy dad jokes here. use super::{ - graph::object::{Object, ObjectIndexTo, ObjectIndexToTree, Pkg, Root, Tpl}, + graph::object::{ + Object, ObjectIndexRelTo, ObjectIndexTo, ObjectIndexToTree, + ObjectRelTy, ObjectRelatable, Pkg, Root, Tpl, + }, Asg, AsgError, Expr, Ident, ObjectIndex, }; use crate::{ @@ -48,11 +51,9 @@ use crate::{ }; use std::fmt::{Debug, Display}; -#[cfg(test)] -use super::{graph::object::ObjectRelatable, ObjectIndexRelTo}; - #[macro_use] mod ir; +use fxhash::FxHashMap; pub use ir::Air; mod expr; @@ -430,8 +431,21 @@ pub struct AirAggregateCtx { /// This is not necessarily the current compilation unit, /// as the parser may be parsing imports. ooi_pkg: Option>, + + /// Cache of object scope at a given environment. + /// + // TODO: FxHashMap is used for other parts of TAMER, + // but here it's really used primarily for bucketing, + // so it may be worth profiling compared to alternatives. + index: ScopeIndex, } +/// Object scope index at a given environment. +type ScopeIndex = FxHashMap< + (ObjectRelTy, SymbolId, ObjectIndex), + EnvScopeKind>, +>; + /// Limit of the maximum number of held parser frames. /// /// Note that this is the number of [`ParseState`]s held, @@ -534,6 +548,83 @@ impl AirAggregateCtx { }) } + fn try_index< + O: ObjectRelatable, + OS: ObjectIndexRelTo, + S: Into, + >( + index: &mut ScopeIndex, + imm_env: OS, + name: S, + eoi: EnvScopeKind>, + ) -> Result<(), ObjectIndex> { + let sym = name.into(); + let prev = index.insert( + (O::rel_ty(), sym, imm_env.widen()), + eoi.map(ObjectIndex::widen), + ); + + match prev { + None => Ok(()), + Some(eoi) => Err(eoi.into_inner().must_narrow_into::()), + } + } + + /// Index the provided symbol `name` as representing the + /// [`ObjectIndex`] in the immediate environment `imm_env`. + /// + /// An index does not require the existence of an edge, + /// but an index may only be created if an edge `imm_env->oi` _could_ + /// be constructed. + /// + /// This index permits `O(1)` object lookups. + /// The term "immediate environment" is meant to convey that this index + /// applies only to the provided `imm_env` node and does not + /// propagate to any other objects that share this environment. + /// + /// After an object is indexed it is not expected to be re-indexed + /// to another node. + /// Debug builds contain an assertion that will panic in this instance. + fn index, S: Into>( + index: &mut ScopeIndex, + imm_env: OS, + name: S, + eoi: EnvScopeKind>, + ) { + let sym = name.into(); + let prev = Self::try_index(index, imm_env, sym, eoi); + + // We should never overwrite indexes + #[allow(unused_variables)] // used only for debug + #[allow(unused_imports)] + if let Err(prev_oi) = prev { + use crate::fmt::{DisplayWrapper, TtQuote}; + crate::debug_diagnostic_panic!( + vec![ + imm_env.widen().note("at this scope boundary"), + prev_oi.note("previously indexed identifier was here"), + eoi.internal_error( + "this identifier has already been indexed at the above scope boundary" + ), + eoi.help( + "this is a bug in the system responsible for analyzing \ + identifier scope;" + ), + eoi.help( + " you can try to work around it by duplicating the definition of " + ), + eoi.help( + format!( + " {} as a _new_ identifier with a different name.", + TtQuote::wrap(sym), + ) + ), + ], + "re-indexing of identifier at scope boundary", + ); + } + } + /// Create a new rooted package of the given canonical name and record /// it as the active package. /// @@ -546,26 +637,24 @@ impl AirAggregateCtx { start: Span, name: SPair, ) -> Result, AsgError> { - let Self { - asg, ooi_pkg: pkg, .. - } = self; - - let oi_root = asg.root(start); - let oi_pkg = asg.create(Pkg::new_canonical(start, name)?); + let oi_root = self.asg.root(start); + let oi_pkg = self.asg.create(Pkg::new_canonical(start, name)?); let eoi_pkg = EnvScopeKind::Visible(oi_pkg); - asg.try_index(oi_root, name, eoi_pkg).map_err(|oi_prev| { - let prev = oi_prev.resolve(asg); + Self::try_index(&mut self.index, oi_root, name, eoi_pkg).map_err( + |oi_prev| { + let prev = oi_prev.resolve(&self.asg); - // unwrap note: a canonical name must exist for this error to - // have been thrown, - // but this will at least not blow up if something really - // odd happens. - AsgError::PkgRedeclare(prev.canonical_name(), name) - })?; + // unwrap note: a canonical name must exist for this error to + // have been thrown, + // but this will at least not blow up if something really + // odd happens. + AsgError::PkgRedeclare(prev.canonical_name(), name) + }, + )?; - oi_pkg.root(asg); - pkg.replace(oi_pkg); + oi_pkg.root(&mut self.asg); + self.ooi_pkg.replace(oi_pkg); Ok(oi_pkg) } @@ -681,7 +770,7 @@ impl AirAggregateCtx { env: impl ObjectIndexRelTo, name: SPair, ) -> Option>> { - self.asg_ref().lookup_raw(env, name) + self.lookup_raw(env, name) } /// Resolve an identifier at the scope of the provided environment. @@ -699,6 +788,47 @@ impl AirAggregateCtx { .map(EnvScopeKind::into_inner) } + /// Attempt to retrieve an identifier from the graph by name relative to + /// the immediate environment `imm_env`. + /// + /// Since only identifiers carry a name, + /// this method cannot be used to retrieve all possible objects on the + /// graph---for + /// that, see [`Asg::get`]. + #[inline] + fn lookup( + &self, + imm_env: impl ObjectIndexRelTo, + id: SPair, + ) -> Option> { + self.lookup_raw(imm_env, id) + .and_then(EnvScopeKind::in_scope) + .map(EnvScopeKind::into_inner) + } + + /// Attempt to retrieve an identifier and its scope information from the + /// graph by name relative to the immediate environment `imm_env`. + /// + /// See [`Self::lookup`] for more information. + #[inline] + fn lookup_raw( + &self, + imm_env: impl ObjectIndexRelTo, + id: SPair, + ) -> Option>> { + // The type `O` is encoded into the index on [`Self::index`] and so + // should always be able to be narrowed into the expected type. + // If this invariant somehow does not hold, + // then the system will panic when the object is resolved. + // Maybe future Rust will have dependent types that allow for better + // static assurances. + self.index + .get(&(O::rel_ty(), id.symbol(), imm_env.widen())) + .map(|&eoi| { + eoi.map(|oi| oi.overwrite(id.span()).must_narrow_into::()) + }) + } + /// Attempt to locate a lexically scoped identifier in the current stack, /// or create a new one if missing. /// @@ -725,12 +855,10 @@ impl AirAggregateCtx { /// To retrieve the span of a previously declared object, /// you must resolve the [`ObjectIndex`] and inspect it. fn lookup_lexical_or_missing(&mut self, name: SPair) -> ObjectIndex { - let Self { asg, stack, .. } = self; - - stack + self.stack .iter() .filter_map(|st| st.active_env_oi()) - .find_map(|oi| asg.lookup(oi, name)) + .find_map(|oi| self.lookup(oi, name)) .unwrap_or_else(|| self.create_env_indexed_ident(name)) } @@ -738,10 +866,9 @@ impl AirAggregateCtx { /// /// TODO: More information as this is formalized. fn create_env_indexed_ident(&mut self, name: SPair) -> ObjectIndex { - let Self { asg, stack, .. } = self; - let oi_ident = asg.create(Ident::declare(name)); + let oi_ident = self.asg.create(Ident::declare(name)); - stack + self.stack .iter() .rev() .filter_map(|frame| frame.active_env_oi().map(|oi| (oi, frame))) @@ -755,7 +882,7 @@ impl AirAggregateCtx { // There is no use in indexing something that will be // filtered out on retrieval. EnvScopeKind::Hidden(_) => (), - _ => asg.index(imm_oi, name, eoi_next), + _ => Self::index(&mut self.index, imm_oi, name, eoi_next), } Some(eoi_next) diff --git a/tamer/src/asg/graph.rs b/tamer/src/asg/graph.rs index d95c414c..eeec2e0d 100644 --- a/tamer/src/asg/graph.rs +++ b/tamer/src/asg/graph.rs @@ -26,16 +26,13 @@ use self::object::{ ObjectRelatable, Root, }; -use super::{air::EnvScopeKind, AsgError, Object, ObjectIndex, ObjectKind}; +use super::{AsgError, Object, ObjectIndex, ObjectKind}; use crate::{ diagnose::{panic::DiagnosticPanic, Annotate, AnnotatedSpan}, f::Functor, global, - parse::{util::SPair, Token}, span::Span, - sym::SymbolId, }; -use fxhash::FxHashMap; use petgraph::{ graph::{DiGraph, Graph, NodeIndex}, visit::EdgeRef, @@ -86,54 +83,16 @@ type Ix = global::ProgSymSize; /// /// For more information, /// see the [module-level documentation][self]. +#[derive(Debug)] pub struct Asg { /// Directed graph on which objects are stored. graph: DiGraph, - /// Environment cache of [`SymbolId`][crate::sym::SymbolId] to - /// [`ObjectIndex`]es. - /// - /// This maps a `(SymbolId, NodeIndex)` pair to a node on the graph for - /// a given [`ObjectRelTy`]. - /// _This indexing is not automatic_; - /// it must be explicitly performed using [`Self::index`]. - /// - /// This index serves as a shortcut for finding nodes on a graph, - /// _but makes no claims about the structure of the graph_. - /// - /// This allows for `O(1)` lookup of identifiers in the graph relative - /// to a given node. - /// Note that, - /// while we store [`NodeIndex`] internally, - /// the public API encapsulates it within an [`ObjectIndex`]. - index: FxHashMap< - (ObjectRelTy, SymbolId, ObjectIndex), - EnvScopeKind>, - >, - /// The root node used for reachability analysis and topological /// sorting. root_node: NodeIndex, } -impl Debug for Asg { - /// Trimmed-down Asg [`Debug`] output. - /// - /// This primarily hides the large `self.index` that takes up so much - /// space in parser traces, - /// but also hides irrelevant information. - /// - /// The better option in the future may be to create a newtype for - /// `index` if it sticks around in its current form, - /// which in turn can encapsulate `self.empty_node`. - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - f.debug_struct("Asg") - .field("root_node", &self.root_node) - .field("graph", &self.graph) - .finish_non_exhaustive() - } -} - impl Default for Asg { fn default() -> Self { Self::new() @@ -161,19 +120,13 @@ impl Asg { /// edge to another object. pub fn with_capacity(objects: usize, edges: usize) -> Self { let mut graph = Graph::with_capacity(objects, edges); - let index = - FxHashMap::with_capacity_and_hasher(objects, Default::default()); // Automatically add the root which will be used to determine what // identifiers ought to be retained by the final program. // This is not indexed and is not accessable by name. let root_node = graph.add_node(Object::Root(Root).into()); - Self { - graph, - index, - root_node, - } + Self { graph, root_node } } /// Get the underlying Graph @@ -190,87 +143,6 @@ impl Asg { self.graph.node_count() } - pub(super) fn try_index< - O: ObjectRelatable, - OS: ObjectIndexRelTo, - S: Into, - >( - &mut self, - imm_env: OS, - name: S, - eoi: EnvScopeKind>, - ) -> Result<(), ObjectIndex> { - let sym = name.into(); - let prev = self.index.insert( - (O::rel_ty(), sym, imm_env.widen()), - eoi.map(ObjectIndex::widen), - ); - - match prev { - None => Ok(()), - Some(eoi) => Err(eoi.into_inner().must_narrow_into::()), - } - } - - /// Index the provided symbol `name` as representing the - /// [`ObjectIndex`] in the immediate environment `imm_env`. - /// - /// An index does not require the existence of an edge, - /// but an index may only be created if an edge `imm_env->oi` _could_ - /// be constructed. - /// - /// This index permits `O(1)` object lookups. - /// The term "immediate environment" is meant to convey that this index - /// applies only to the provided `imm_env` node and does not - /// propagate to any other objects that share this environment. - /// - /// After an object is indexed it is not expected to be re-indexed - /// to another node. - /// Debug builds contain an assertion that will panic in this instance. - pub(super) fn index< - O: ObjectRelatable, - OS: ObjectIndexRelTo, - S: Into, - >( - &mut self, - imm_env: OS, - name: S, - eoi: EnvScopeKind>, - ) { - let sym = name.into(); - let prev = self.try_index(imm_env, sym, eoi); - - // We should never overwrite indexes - #[allow(unused_variables)] // used only for debug - #[allow(unused_imports)] - if let Err(prev_oi) = prev { - use crate::fmt::{DisplayWrapper, TtQuote}; - crate::debug_diagnostic_panic!( - vec![ - imm_env.widen().note("at this scope boundary"), - prev_oi.note("previously indexed identifier was here"), - eoi.internal_error( - "this identifier has already been indexed at the above scope boundary" - ), - eoi.help( - "this is a bug in the system responsible for analyzing \ - identifier scope;" - ), - eoi.help( - " you can try to work around it by duplicating the definition of " - ), - eoi.help( - format!( - " {} as a _new_ identifier with a different name.", - TtQuote::wrap(sym), - ) - ), - ], - "re-indexing of identifier at scope boundary", - ); - } - } - /// Root object. /// /// All [`Object`]s reachable from the root will be included in the @@ -479,47 +351,6 @@ impl Asg { obj_container.get() } - - /// Attempt to retrieve an identifier from the graph by name relative to - /// the immediate environment `imm_env`. - /// - /// Since only identifiers carry a name, - /// this method cannot be used to retrieve all possible objects on the - /// graph---for - /// that, see [`Asg::get`]. - #[inline] - pub fn lookup( - &self, - imm_env: impl ObjectIndexRelTo, - id: SPair, - ) -> Option> { - self.lookup_raw(imm_env, id) - .and_then(EnvScopeKind::in_scope) - .map(EnvScopeKind::into_inner) - } - - /// Attempt to retrieve an identifier and its scope information from the - /// graph by name relative to the immediate environment `imm_env`. - /// - /// See [`Self::lookup`] for more information. - #[inline] - pub(super) fn lookup_raw( - &self, - imm_env: impl ObjectIndexRelTo, - id: SPair, - ) -> Option>> { - // The type `O` is encoded into the index on [`Self::index`] and so - // should always be able to be narrowed into the expected type. - // If this invariant somehow does not hold, - // then the system will panic when the object is resolved. - // Maybe future Rust will have dependent types that allow for better - // static assurances. - self.index - .get(&(O::rel_ty(), id.symbol(), imm_env.widen())) - .map(|&eoi| { - eoi.map(|oi| oi.overwrite(id.span()).must_narrow_into::()) - }) - } } fn diagnostic_node_missing_desc( diff --git a/tamer/src/asg/graph/test.rs b/tamer/src/asg/graph/test.rs index 661ba6ae..7cb16394 100644 --- a/tamer/src/asg/graph/test.rs +++ b/tamer/src/asg/graph/test.rs @@ -18,7 +18,7 @@ // along with this program. If not, see . use super::*; -use crate::span::dummy::*; +use crate::{parse::util::SPair, span::dummy::*}; use object::Ident; use std::convert::Infallible; @@ -33,7 +33,6 @@ fn create_with_capacity() { let (nc, ec) = sut.graph.capacity(); assert!(nc >= node_capacity); assert!(ec >= edge_capacity); - assert!(sut.index.capacity() >= node_capacity); } #[test]