tamer: asg: Restrict index-related operations to AIR

This is in the same spirit as previous commits modifying (or removing)
tests and benchmarks related to accessing the ASG and its indexes directly.

With this change, only `asg::air` uses the indexing and lookup methods on
`Asg`.  This will allow me to extract the index from `Asg` entirely and have
`Air` solely responsible for lookup; the graph will be responsible only for,
well, being a graph.  Indexing is an optimization strategy.

More information in the commit to follow.  But notice how this moving
environment-related concerns away from `Asg` and into AIR, and how the
remaining environment concerns are index-related.

But there is one remaining barrier: to fully move the indexing away from
`Asg`, we have to use an alternative (and complete)
abstraction---AirAggregateCtx with its ability to resolve and introduce
scope based on the stack.  The `AirIdent` token subset doesn't yet do that,
and all the work up to this point was in prepartion for doing that.  Since
introducing indexing at Root a few commits ago, it's now possible to
proceed.

DEV-13162
main
Mike Gerwitz 2023-05-17 11:31:56 -04:00
parent 92eb991df3
commit 716e217c9f
8 changed files with 40 additions and 69 deletions

View File

@ -459,7 +459,13 @@ impl AirAggregateCtx {
})
}
/// Create a new rooted package and record it as the active package.
/// Create a new rooted package of the given canonical name and record
/// it as the active package.
///
/// A canonical package name is a path relative to the project root.
///
/// This operation will fail if a package of the same name has already
/// been declared.
fn pkg_begin(
&mut self,
start: Span,
@ -468,9 +474,22 @@ impl AirAggregateCtx {
let Self(asg, _, pkg) = self;
let oi_root = asg.root(start);
let oi_pkg = oi_root.create_pkg(asg, start, name)?;
let oi_pkg = 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);
// 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);
Ok(oi_pkg)
}

View File

@ -171,7 +171,10 @@ impl ParseState for AirPkgAggregate {
(Toplevel(oi_pkg), AirIdent(IdentRoot(name))) => {
let asg = ctx.asg_mut();
asg.root(name).root_ident(asg, name);
let oi_root = asg.root(name);
let oi_ident = asg.lookup_or_missing(oi_root, name);
oi_root.root_ident(asg, oi_ident);
Transition(Toplevel(oi_pkg)).incomplete()
}

View File

@ -155,8 +155,8 @@ test_scopes! {
#[test]
inner == [
// Same as above,
// since the environment is the same.
// Same as above since the environment is the same;
// `Expr` does not introduce a new environment.
(Root, S0, Visible),
(Pkg, m(S1, S8), Visible),
];

View File

@ -779,12 +779,16 @@ impl<O: ObjectKind> ObjectIndex<O> {
/// Attempt to look up a locally bound [`Ident`] via a linear search of
/// `self`'s edges.
///
/// This should be used only when an index is not available,
/// and is currently restricted to tests.
///
/// See [`ObjectIndexRelTo::lookup_local_linear`].
//
// TODO: This method exists here only as a fallback when Rust is unable
// to infer the proper type for [`ObjectIndexRelTo`].
// It can be removed once that is resolved;
// delete this method and compile to see.
#[cfg(test)]
pub fn lookup_local_linear(
&self,
asg: &Asg,

View File

@ -42,7 +42,7 @@ impl Pkg {
/// Create a new package with a canonicalized name.
///
/// A canonical package name is a path relative to the project root.
pub(super) fn new_canonical<S: Into<Span>>(
pub(in crate::asg) fn new_canonical<S: Into<Span>>(
start: S,
name: SPair,
) -> Result<Self, AsgError> {

View File

@ -843,6 +843,9 @@ pub trait ObjectIndexRelTo<OB: ObjectRelatable>: Sized + Clone + Copy {
/// Attempt to look up a locally bound [`Ident`] via a linear search of
/// `self`'s edges.
///
/// This should be used only when an index is not available,
/// and is currently restricted to tests.
///
/// Performance
/// ===========
/// _This is a linear (O(1)) search of the edges of the node
@ -863,6 +866,7 @@ pub trait ObjectIndexRelTo<OB: ObjectRelatable>: Sized + Clone + Copy {
/// circumstances during profiling,
/// then you should consider caching like the global environment
/// (see [`Asg::lookup`]).
#[cfg(test)]
fn lookup_local_linear(
&self,
asg: &Asg,

View File

@ -20,11 +20,6 @@
//! Root node of the ASG.
use super::{prelude::*, Ident, Pkg};
use crate::{
asg::{air::EnvScopeKind, IdentKind, Source},
parse::util::SPair,
span::Span,
};
use std::fmt::Display;
#[cfg(doc)]
@ -67,63 +62,11 @@ impl ObjectIndex<Root> {
/// An identifier ought to be rooted by a package that defines it;
/// this is intended as a legacy operation for `tameld` that will be
/// removed in the future.
pub fn root_ident(&self, asg: &mut Asg, name: SPair) -> ObjectIndex<Ident> {
asg.lookup_or_missing(*self, name)
.add_edge_from(asg, *self, None)
}
/// Attempt to retrieve an indexed [`Ident`] owned by `self`.
///
/// See [`Self::root_ident`].
pub fn lookup_or_missing(
pub fn root_ident(
&self,
asg: &mut Asg,
name: SPair,
oi: ObjectIndex<Ident>,
) -> ObjectIndex<Ident> {
asg.lookup_or_missing(*self, name)
}
/// Declare a concrete identifier.
///
/// See [`ObjectIndex::<Ident>::declare`] for more information.
pub fn declare(
&self,
asg: &mut Asg,
name: SPair,
kind: IdentKind,
src: Source,
) -> Result<ObjectIndex<Ident>, AsgError> {
self.lookup_or_missing(asg, name)
.declare(asg, name, kind, src)
}
/// Attempt to declare a package with the given canonical name.
///
/// A canonical package name is a path relative to the project root.
///
/// This assignment will fail if the package either already has a name
/// or if a package of the same name has already been declared.
pub fn create_pkg(
self,
asg: &mut Asg,
start: Span,
name: SPair,
) -> Result<ObjectIndex<Pkg>, AsgError> {
let oi_pkg = asg.create(Pkg::new_canonical(start, name)?);
// TODO: We shouldn't be responsible for this
let eoi_pkg = EnvScopeKind::Visible(oi_pkg);
asg.try_index(self, name, eoi_pkg).map_err(|oi_prev| {
let prev = oi_prev.resolve(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)
})?;
Ok(oi_pkg.root(asg))
oi.add_edge_from(asg, *self, None)
}
}

View File

@ -269,9 +269,7 @@ fn omits_unreachable() {
.next()
.expect("cannot find Pkg on graph");
let oi_b = asg
.lookup::<object::Ident>(oi_pkg, id_b)
.expect("missing oi_b");
let oi_b = oi_pkg.lookup_local_linear(asg, id_b).expect("missing oi_b");
// We'll use only `oi_b` as the root,
// which will include it and its (only) dependency.