tamer: Remove Ix generic from ASG

This is simply not worth it; the size is not going to be the bottleneck (at
least any time soon) and the generic not only pollutes all the things that
will use ASG in the near future, but is also incompatible with the SymbolId
default that is used everywhere; if we have to force it to 32 bits anyway,
then we may as well just default it right off the bat.

I thought that this seemed like a good idea at the time, and saving bits is
certainly tempting, but it was premature.
main
Mike Gerwitz 2022-01-14 10:21:49 -05:00
parent 5af698d15c
commit 4c5b860195
6 changed files with 91 additions and 111 deletions

View File

@ -19,14 +19,18 @@
//! Base concrete [`Asg`] implementation.
use super::graph::{Asg, AsgEdge, AsgResult, IndexType, Node, ObjectRef};
use super::graph::{Asg, AsgEdge, AsgResult, Node, ObjectRef};
use super::ident::IdentKind;
use super::object::{
FragmentText, IdentObjectData, IdentObjectState, Source, TransitionResult,
};
use crate::global;
use crate::sym::SymbolId;
use petgraph::graph::{DiGraph, Graph, NodeIndex};
/// Index size for Graph nodes and edges.
type Ix = global::ProgSymSize;
/// Concrete ASG.
///
/// This implementation is currently based on [`petgraph`].
@ -37,10 +41,7 @@ use petgraph::graph::{DiGraph, Graph, NodeIndex};
///
/// For more information,
/// see [`Asg`].
pub struct BaseAsg<O, Ix>
where
Ix: IndexType,
{
pub struct BaseAsg<O> {
// TODO: private; see `ld::xmle::lower`.
/// Directed graph on which objects are stored.
pub graph: DiGraph<Node<O>, AsgEdge, Ix>,
@ -57,9 +58,8 @@ where
empty_node: NodeIndex<Ix>,
}
impl<O, Ix> BaseAsg<O, Ix>
impl<O> BaseAsg<O>
where
Ix: IndexType,
O: IdentObjectState<O> + IdentObjectData,
{
/// Create a new ASG.
@ -133,7 +133,7 @@ where
/// reference to it.
///
/// See [`IdentObjectState::declare`] for more information.
fn lookup_or_missing(&mut self, ident: SymbolId) -> ObjectRef<Ix> {
fn lookup_or_missing(&mut self, ident: SymbolId) -> ObjectRef {
self.lookup(ident).unwrap_or_else(|| {
let index = self.graph.add_node(Some(O::declare(ident)));
@ -155,7 +155,7 @@ where
&mut self,
name: SymbolId,
f: F,
) -> AsgResult<ObjectRef<Ix>>
) -> AsgResult<ObjectRef>
where
F: FnOnce(O) -> TransitionResult<O>,
{
@ -170,11 +170,7 @@ where
///
/// This will safely restore graph state to the original identifier
/// value on transition failure.
fn with_ident<F>(
&mut self,
identi: ObjectRef<Ix>,
f: F,
) -> AsgResult<ObjectRef<Ix>>
fn with_ident<F>(&mut self, identi: ObjectRef, f: F) -> AsgResult<ObjectRef>
where
F: FnOnce(O) -> TransitionResult<O>,
{
@ -196,9 +192,8 @@ where
}
}
impl<O, Ix> Asg<O, Ix> for BaseAsg<O, Ix>
impl<O> Asg<O> for BaseAsg<O>
where
Ix: IndexType,
O: IdentObjectState<O> + IdentObjectData,
{
fn declare(
@ -206,7 +201,7 @@ where
name: SymbolId,
kind: IdentKind,
src: Source,
) -> AsgResult<ObjectRef<Ix>> {
) -> AsgResult<ObjectRef> {
self.with_ident_lookup(name, |obj| obj.resolve(kind, src))
}
@ -215,20 +210,20 @@ where
name: SymbolId,
kind: IdentKind,
src: Source,
) -> AsgResult<ObjectRef<Ix>> {
) -> AsgResult<ObjectRef> {
self.with_ident_lookup(name, |obj| obj.extern_(kind, src))
}
fn set_fragment(
&mut self,
identi: ObjectRef<Ix>,
identi: ObjectRef,
text: FragmentText,
) -> AsgResult<ObjectRef<Ix>> {
) -> AsgResult<ObjectRef> {
self.with_ident(identi, |obj| obj.set_fragment(text))
}
#[inline]
fn get<I: Into<ObjectRef<Ix>>>(&self, index: I) -> Option<&O> {
fn get<I: Into<ObjectRef>>(&self, index: I) -> Option<&O> {
self.graph.node_weight(index.into().into()).map(|node| {
node.as_ref()
.expect("internal error: BaseAsg::get missing Node data")
@ -236,7 +231,7 @@ where
}
#[inline]
fn lookup(&self, name: SymbolId) -> Option<ObjectRef<Ix>> {
fn lookup(&self, name: SymbolId) -> Option<ObjectRef> {
let i = name.as_usize();
self.index
@ -245,13 +240,13 @@ where
.map(|ni| ObjectRef::new(*ni))
}
fn add_dep(&mut self, identi: ObjectRef<Ix>, depi: ObjectRef<Ix>) {
fn add_dep(&mut self, identi: ObjectRef, depi: ObjectRef) {
self.graph
.update_edge(identi.into(), depi.into(), Default::default());
}
#[inline]
fn has_dep(&self, ident: ObjectRef<Ix>, dep: ObjectRef<Ix>) -> bool {
fn has_dep(&self, ident: ObjectRef, dep: ObjectRef) -> bool {
self.graph.contains_edge(ident.into(), dep.into())
}
@ -259,7 +254,7 @@ where
&mut self,
ident: SymbolId,
dep: SymbolId,
) -> (ObjectRef<Ix>, ObjectRef<Ix>) {
) -> (ObjectRef, ObjectRef) {
let identi = self.lookup_or_missing(ident);
let depi = self.lookup_or_missing(dep);
@ -372,7 +367,7 @@ mod test {
}
}
type Sut = BaseAsg<StubIdentObject, u16>;
type Sut = BaseAsg<StubIdentObject>;
#[test]
fn create_with_capacity() {

View File

@ -42,9 +42,8 @@ impl<T: petgraph::graph::IndexType> IndexType for T {}
///
/// For more information,
/// see the [module-level documentation][self].
pub trait Asg<O, Ix>
pub trait Asg<O>
where
Ix: IndexType,
O: IdentObjectState<O>,
{
/// Declare a concrete identifier.
@ -83,7 +82,7 @@ where
name: SymbolId,
kind: IdentKind,
src: Source,
) -> AsgResult<ObjectRef<Ix>>;
) -> AsgResult<ObjectRef>;
/// Declare an abstract identifier.
///
@ -110,7 +109,7 @@ where
name: SymbolId,
kind: IdentKind,
src: Source,
) -> AsgResult<ObjectRef<Ix>>;
) -> AsgResult<ObjectRef>;
/// Set the fragment associated with a concrete identifier.
///
@ -119,9 +118,9 @@ where
/// see [`IdentObjectState::set_fragment`].
fn set_fragment(
&mut self,
identi: ObjectRef<Ix>,
identi: ObjectRef,
text: FragmentText,
) -> AsgResult<ObjectRef<Ix>>;
) -> AsgResult<ObjectRef>;
/// Retrieve an object from the graph by [`ObjectRef`].
///
@ -130,7 +129,7 @@ where
/// this should never fail so long as references are not shared
/// between multiple graphs.
/// It is nevertheless wrapped in an [`Option`] just in case.
fn get<I: Into<ObjectRef<Ix>>>(&self, index: I) -> Option<&O>;
fn get<I: Into<ObjectRef>>(&self, index: I) -> Option<&O>;
/// Attempt to retrieve an identifier from the graph by name.
///
@ -138,7 +137,7 @@ where
/// this method cannot be used to retrieve all possible objects on the
/// graph---for
/// that, see [`Asg::get`].
fn lookup(&self, name: SymbolId) -> Option<ObjectRef<Ix>>;
fn lookup(&self, name: SymbolId) -> Option<ObjectRef>;
/// Declare that `dep` is a dependency of `ident`.
///
@ -149,10 +148,10 @@ where
/// See [`add_dep_lookup`][Asg::add_dep_lookup] if identifiers have to
/// be looked up by [`SymbolId`] or if they may not yet have been
/// declared.
fn add_dep(&mut self, ident: ObjectRef<Ix>, dep: ObjectRef<Ix>);
fn add_dep(&mut self, ident: ObjectRef, dep: ObjectRef);
/// Check whether `dep` is a dependency of `ident`.
fn has_dep(&self, ident: ObjectRef<Ix>, dep: ObjectRef<Ix>) -> bool;
fn has_dep(&self, ident: ObjectRef, dep: ObjectRef) -> bool;
/// Declare that `dep` is a dependency of `ident`,
/// regardless of whether they are known.
@ -171,7 +170,7 @@ where
&mut self,
ident: SymbolId,
dep: SymbolId,
) -> (ObjectRef<Ix>, ObjectRef<Ix>);
) -> (ObjectRef, ObjectRef);
}
/// A [`Result`] with a hard-coded [`AsgError`] error type.
@ -186,22 +185,22 @@ pub type AsgResult<T> = Result<T, AsgError>;
/// not pointers.
/// See the [module-level documentation][self] for more information.
#[derive(Debug, Copy, Clone, Default, PartialEq, Eq)]
pub struct ObjectRef<Ix>(NodeIndex<Ix>);
pub struct ObjectRef(NodeIndex);
impl<Ix: IndexType> ObjectRef<Ix> {
pub fn new(index: NodeIndex<Ix>) -> Self {
impl ObjectRef {
pub fn new(index: NodeIndex) -> Self {
Self(index)
}
}
impl<Ix: IndexType> From<NodeIndex<Ix>> for ObjectRef<Ix> {
fn from(index: NodeIndex<Ix>) -> Self {
impl From<NodeIndex> for ObjectRef {
fn from(index: NodeIndex) -> Self {
Self(index)
}
}
impl<Ix: IndexType> From<ObjectRef<Ix>> for NodeIndex<Ix> {
fn from(objref: ObjectRef<Ix>) -> Self {
impl From<ObjectRef> for NodeIndex {
fn from(objref: ObjectRef) -> Self {
objref.0
}
}
@ -265,7 +264,7 @@ mod test {
#[test]
fn to_from_nodeindex() {
let index = NodeIndex::<u32>::new(5);
let objref: ObjectRef<u32> = ObjectRef::from(index);
let objref: ObjectRef = ObjectRef::from(index);
assert_eq!(index, objref.0);
assert_eq!(index, objref.into());

View File

@ -206,4 +206,4 @@ pub use object::{
};
/// Default concrete ASG implementation.
pub type DefaultAsg<O, Ix = crate::global::ProgSymSize> = base::BaseAsg<O, Ix>;
pub type DefaultAsg<O> = base::BaseAsg<O>;

View File

@ -25,7 +25,6 @@ use super::xmle::{
xir::lower_iter,
XmleSections,
};
use crate::global;
use crate::sym::SymbolId;
use crate::sym::{GlobalSymbolIntern, GlobalSymbolResolve};
use crate::xir::writer::XmlWriter;
@ -52,10 +51,8 @@ use std::io::Write;
use std::io::{BufReader, BufWriter};
use std::path::{Path, PathBuf};
type LinkerAsg = DefaultAsg<IdentObject, global::ProgIdentSize>;
type LinkerAsgBuilderState =
AsgBuilderState<FxBuildHasher, global::ProgIdentSize>;
type LinkerAsg = DefaultAsg<IdentObject>;
type LinkerAsgBuilderState = AsgBuilderState<FxBuildHasher>;
pub fn xmle(package_path: &str, output: &str) -> Result<(), Box<dyn Error>> {
let mut fs = VisitOnceFilesystem::new();

View File

@ -23,26 +23,25 @@
use super::section::{SectionsError, XmleSections};
use crate::{
asg::{Asg, BaseAsg, IdentKind, IdentObject, IndexType, ObjectRef},
asg::{Asg, BaseAsg, IdentKind, IdentObject, ObjectRef},
sym::{st, GlobalSymbolResolve, SymbolId},
};
use petgraph::visit::DfsPostOrder;
// Result of [`sort`].
pub type SortResult<T, Ix> = Result<T, SortError<Ix>>;
pub type SortResult<T> = Result<T, SortError>;
/// Lower ASG into [`XmleSections`] by ordering relocatable text fragments.
///
/// This performs the equivalent of a topological sort,
/// although function cycles are permitted.
/// The actual operation performed is a post-order depth-first traversal.
pub fn sort<'a, Ix, S: XmleSections<'a>>(
asg: &'a BaseAsg<IdentObject, Ix>,
roots: &[ObjectRef<Ix>],
pub fn sort<'a, S: XmleSections<'a>>(
asg: &'a BaseAsg<IdentObject>,
roots: &[ObjectRef],
mut dest: S,
) -> SortResult<S, Ix>
) -> SortResult<S>
where
Ix: IndexType,
S: XmleSections<'a>,
{
// TODO: we should check for cycles as we sort (as the POC did).
@ -72,12 +71,11 @@ where
Ok(dest)
}
fn get_ident<'a, Ix, S>(
depgraph: &'a BaseAsg<IdentObject, Ix>,
fn get_ident<'a, S>(
depgraph: &'a BaseAsg<IdentObject>,
name: S,
) -> &'a IdentObject
where
Ix: IndexType,
S: Into<SymbolId>,
{
let sym = name.into();
@ -99,10 +97,7 @@ where
///
/// We loop through all SCCs and check that they are not all functions. If
/// they are, we ignore the cycle, otherwise we will return an error.
fn check_cycles<Ix>(asg: &BaseAsg<IdentObject, Ix>) -> SortResult<(), Ix>
where
Ix: IndexType,
{
fn check_cycles(asg: &BaseAsg<IdentObject>) -> SortResult<()> {
// While `tarjan_scc` does do a topological sort, it does not suit our
// needs because we need to filter out some allowed cycles. It would
// still be possible to use this, but we also need to only check nodes
@ -151,21 +146,21 @@ where
/// It does not represent bad underlying data that does not affect the
/// sorting process.
#[derive(Debug, PartialEq)]
pub enum SortError<Ix: IndexType> {
pub enum SortError {
/// Error while lowering into [`XmleSections`].
SectionsError(SectionsError),
/// The graph has a cyclic dependency.
Cycles(Vec<Vec<ObjectRef<Ix>>>),
Cycles(Vec<Vec<ObjectRef>>),
}
impl<Ix: IndexType> From<SectionsError> for SortError<Ix> {
impl From<SectionsError> for SortError {
fn from(err: SectionsError) -> Self {
Self::SectionsError(err)
}
}
impl<Ix: IndexType> std::fmt::Display for SortError<Ix> {
impl std::fmt::Display for SortError {
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Self::SectionsError(err) => err.fmt(fmt),
@ -174,7 +169,7 @@ impl<Ix: IndexType> std::fmt::Display for SortError<Ix> {
}
}
impl<Ix: IndexType> std::error::Error for SortError<Ix> {
impl std::error::Error for SortError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
None
}
@ -190,7 +185,7 @@ mod test {
sym::GlobalSymbolIntern,
};
type TestAsg = BaseAsg<IdentObject, u16>;
type TestAsg = BaseAsg<IdentObject>;
/// Create a graph with the expected {ret,}map head/tail identifiers.
fn make_asg() -> TestAsg {
@ -234,7 +229,7 @@ mod test {
}
#[test]
fn graph_sort() -> SortResult<(), u16> {
fn graph_sort() -> SortResult<()> {
// We care only about the order of pushes, not the sections they end
// up in.
struct StubSections<'a> {
@ -318,7 +313,7 @@ mod test {
}
#[test]
fn graph_sort_missing_node() -> SortResult<(), u16> {
fn graph_sort_missing_node() -> SortResult<()> {
let mut asg = make_asg();
let sym = "sym".intern();
@ -354,7 +349,7 @@ mod test {
}
#[test]
fn graph_sort_no_roots_same_as_empty_graph() -> SortResult<(), u16> {
fn graph_sort_no_roots_same_as_empty_graph() -> SortResult<()> {
let mut asg_nonempty_no_roots = make_asg();
// "empty" (it has the head/tail {ret,}map objects)
@ -374,7 +369,7 @@ mod test {
}
#[test]
fn graph_sort_simple_cycle() -> SortResult<(), u16> {
fn graph_sort_simple_cycle() -> SortResult<()> {
let mut asg = make_asg();
let sym = "sym".intern();
@ -412,7 +407,7 @@ mod test {
let result = sort(&asg, &vec![sym_node], Sections::new());
let expected: Vec<Vec<ObjectRef<u16>>> =
let expected: Vec<Vec<ObjectRef>> =
vec![vec![dep_node.into(), sym_node.into()]];
match result {
Ok(_) => panic!("sort did not detect cycle"),
@ -424,7 +419,7 @@ mod test {
}
#[test]
fn graph_sort_two_simple_cycles() -> SortResult<(), u16> {
fn graph_sort_two_simple_cycles() -> SortResult<()> {
let mut asg = make_asg();
let sym = "sym".intern();
@ -492,7 +487,7 @@ mod test {
let result = sort(&asg, &vec![sym_node], Sections::new());
let expected: Vec<Vec<ObjectRef<u16>>> = vec![
let expected: Vec<Vec<ObjectRef>> = vec![
vec![dep_node.into(), sym_node.into()],
vec![dep2_node.into(), sym2_node.into()],
];
@ -506,7 +501,7 @@ mod test {
}
#[test]
fn graph_sort_no_cycle_with_edge_to_same_node() -> SortResult<(), u16> {
fn graph_sort_no_cycle_with_edge_to_same_node() -> SortResult<()> {
let mut asg = make_asg();
let sym = "sym".intern();
@ -553,7 +548,7 @@ mod test {
}
#[test]
fn graph_sort_cycle_with_a_few_steps() -> SortResult<(), u16> {
fn graph_sort_cycle_with_a_few_steps() -> SortResult<()> {
let mut asg = make_asg();
let sym1 = "sym1".intern();
@ -606,7 +601,7 @@ mod test {
let result = sort(&asg, &vec![sym1_node], Sections::new());
let expected: Vec<Vec<ObjectRef<u16>>> =
let expected: Vec<Vec<ObjectRef>> =
vec![vec![sym3_node.into(), sym2_node.into(), sym1_node.into()]];
match result {
Ok(_) => panic!("sort did not detect cycle"),
@ -619,7 +614,7 @@ mod test {
#[test]
fn graph_sort_cyclic_function_with_non_function_with_a_few_steps(
) -> SortResult<(), u16> {
) -> SortResult<()> {
let mut asg = make_asg();
let sym1 = "sym1".intern();
@ -672,7 +667,7 @@ mod test {
let result = sort(&asg, &vec![sym1_node], Sections::new());
let expected: Vec<Vec<ObjectRef<u16>>> =
let expected: Vec<Vec<ObjectRef>> =
vec![vec![sym3_node.into(), sym2_node.into(), sym1_node.into()]];
match result {
Ok(_) => panic!("sort did not detect cycle"),
@ -684,7 +679,7 @@ mod test {
}
#[test]
fn graph_sort_cyclic_bookended_by_functions() -> SortResult<(), u16> {
fn graph_sort_cyclic_bookended_by_functions() -> SortResult<()> {
let mut asg = make_asg();
let sym1 = "sym1".intern();
@ -737,7 +732,7 @@ mod test {
let result = sort(&asg, &vec![sym1_node], Sections::new());
let expected: Vec<Vec<ObjectRef<u16>>> =
let expected: Vec<Vec<ObjectRef>> =
vec![vec![sym3_node.into(), sym2_node.into(), sym1_node.into()]];
match result {
Ok(_) => panic!("sort did not detect cycle"),
@ -749,7 +744,7 @@ mod test {
}
#[test]
fn graph_sort_cyclic_function_ignored() -> SortResult<(), u16> {
fn graph_sort_cyclic_function_ignored() -> SortResult<()> {
let mut asg = make_asg();
let sym = "sym".intern();
@ -796,7 +791,7 @@ mod test {
}
#[test]
fn graph_sort_cyclic_function_is_bookended() -> SortResult<(), u16> {
fn graph_sort_cyclic_function_is_bookended() -> SortResult<()> {
let mut asg = make_asg();
let sym1 = "sym1".intern();
@ -849,7 +844,7 @@ mod test {
let result = sort(&asg, &vec![sym1_node], Sections::new());
let expected: Vec<Vec<ObjectRef<u16>>> =
let expected: Vec<Vec<ObjectRef>> =
vec![vec![sym3_node.into(), sym2_node.into(), sym1_node.into()]];
match result {
Ok(_) => panic!("sort did not detect cycle"),
@ -861,7 +856,7 @@ mod test {
}
#[test]
fn graph_sort_ignore_non_linked() -> SortResult<(), u16> {
fn graph_sort_ignore_non_linked() -> SortResult<()> {
let mut asg = make_asg();
let sym = "sym".intern();

View File

@ -42,8 +42,8 @@ use super::{
XmloError,
};
use crate::asg::{
Asg, AsgError, IdentKind, IdentKindError, IdentObjectState, IndexType,
ObjectRef, Source,
Asg, AsgError, IdentKind, IdentKindError, IdentObjectState, ObjectRef,
Source,
};
use crate::sym::SymbolId;
use std::collections::HashSet;
@ -52,8 +52,7 @@ use std::error::Error;
use std::fmt::Display;
use std::hash::BuildHasher;
pub type Result<S, Ix> =
std::result::Result<AsgBuilderState<S, Ix>, AsgBuilderError>;
pub type Result<S> = std::result::Result<AsgBuilderState<S>, AsgBuilderError>;
/// Builder state between imports.
///
@ -75,17 +74,16 @@ pub type Result<S, Ix> =
/// This is used by the linker to only include dependencies that are
/// actually used by a particular program.
#[derive(Debug, Default)]
pub struct AsgBuilderState<S, Ix>
pub struct AsgBuilderState<S>
where
S: BuildHasher,
Ix: IndexType,
{
/// Discovered roots.
///
/// Roots represent starting points for a topological sort of the
/// graph.
/// They are meaningful to the linker.
pub roots: Vec<ObjectRef<Ix>>,
pub roots: Vec<ObjectRef>,
/// Relative paths to imported packages that have been discovered.
///
@ -108,10 +106,9 @@ where
pub relroot: Option<SymbolId>,
}
impl<S, Ix> AsgBuilderState<S, Ix>
impl<S> AsgBuilderState<S>
where
S: BuildHasher + Default,
Ix: IndexType,
{
/// Create a new, empty state.
pub fn new() -> Self {
@ -133,11 +130,10 @@ where
/// For more information on what data are processed,
/// see [`AsgBuilderState`].
/// See the [module-level documentation](self) for example usage.
pub trait AsgBuilder<O, S, Ix>
pub trait AsgBuilder<O, S>
where
O: IdentObjectState<O>,
S: BuildHasher,
Ix: IndexType,
{
/// Import [`XmloResult`]s into an [`Asg`].
///
@ -152,22 +148,21 @@ where
fn import_xmlo(
&mut self,
xmlo: impl Iterator<Item = XmloResult<XmloEvent>>,
state: AsgBuilderState<S, Ix>,
) -> Result<S, Ix>;
state: AsgBuilderState<S>,
) -> Result<S>;
}
impl<O, S, Ix, G> AsgBuilder<O, S, Ix> for G
impl<O, S, G> AsgBuilder<O, S> for G
where
O: IdentObjectState<O>,
S: BuildHasher + Default,
Ix: IndexType,
G: Asg<O, Ix>,
G: Asg<O>,
{
fn import_xmlo(
&mut self,
mut xmlo: impl Iterator<Item = XmloResult<XmloEvent>>,
mut state: AsgBuilderState<S, Ix>,
) -> Result<S, Ix> {
mut state: AsgBuilderState<S>,
) -> Result<S> {
let mut elig = None;
let first = state.is_first();
let found = state.found.get_or_insert(Default::default());
@ -333,9 +328,8 @@ mod test {
use crate::sym::GlobalSymbolIntern;
use std::collections::hash_map::RandomState;
type SutIx = u16;
type Sut<'i> = DefaultAsg<IdentObject, SutIx>;
type SutState<'i> = AsgBuilderState<RandomState, SutIx>;
type Sut<'i> = DefaultAsg<IdentObject>;
type SutState<'i> = AsgBuilderState<RandomState>;
#[test]
fn gets_data_from_package_event() {
@ -584,7 +578,7 @@ mod test {
// This is all that's needed to not consider this to be the first
// package, so that pkg_name is retained below.
let state = AsgBuilderState::<RandomState, SutIx> {
let state = AsgBuilderState::<RandomState> {
name: Some(pkg_name),
..Default::default()
};