tamer: asg::graph: Encapsulate edge additions
AIR is no longer able to explicitly add edges without going through an object-specific `ObjectIndex` API. `Asg::add_edge` was already private, but `ObjectIndex::add_edge_{to,from}` was not. The problem is that I want to augment the graph with other invariants, such as caches. I'd normally have this built into the graph system itself, but I don't have the time for the engineering effort to extend or replace Petgraph, so I'm going to build atop of it. To have confidence in any sort of caching, I need assurances that the graph can't change out from underneath an object. This gets _close_ to accomplishing that, but I'm still uncomfortable: - We're one `pub` addition away from breaking these invariants; and - Other `Object` types can still manipulates one-anothers' edges. So this is a first step that at least proves encapsulation within `asg::graph`, but ideally we'd have the system enforce, statically, that `Objects` own their _outgoing_ edges, and no other `Object` is able to manipulate them. This would ensure that any accidental future changes, or bugs, will cause compilation failures rather than e.g. allowing caches to get out of sync with the graph. DEV-13163main
parent
0f93f3a498
commit
e414782def
|
@ -1005,11 +1005,9 @@ impl AirAggregateCtx {
|
|||
.rooting_oi()
|
||||
.ok_or(AsgError::InvalidBindContext(binding_name))?;
|
||||
|
||||
Ok(self.lookup_lexical_or_missing(binding_name).add_edge_from(
|
||||
self.asg_mut(),
|
||||
oi_root,
|
||||
None,
|
||||
))
|
||||
Ok(self
|
||||
.lookup_lexical_or_missing(binding_name)
|
||||
.defined_by(self.asg_mut(), oi_root))
|
||||
}
|
||||
|
||||
/// Define an abstract identifier within the context of a container that
|
||||
|
@ -1058,7 +1056,7 @@ impl AirAggregateCtx {
|
|||
|
||||
Ok(oi_meta_ident
|
||||
.new_abstract_ident(self.asg_mut(), meta_name.span())
|
||||
.add_edge_from(self.asg_mut(), oi_root, None))
|
||||
.defined_by(self.asg_mut(), oi_root))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -30,10 +30,7 @@ use super::{
|
|||
AirAggregate, AirAggregateCtx,
|
||||
};
|
||||
use crate::{
|
||||
asg::{
|
||||
graph::object::{ObjectIndexRelTo, ObjectIndexTo},
|
||||
ObjectKind,
|
||||
},
|
||||
asg::{graph::object::ObjectIndexTo, ObjectKind},
|
||||
f::Functor,
|
||||
parse::prelude::*,
|
||||
};
|
||||
|
@ -207,9 +204,10 @@ impl AirExprAggregate {
|
|||
oi_root: Option<ObjectIndexTo<Expr>>,
|
||||
oi_expr: ObjectIndex<Expr>,
|
||||
) -> Result<(), AsgError> {
|
||||
oi_root
|
||||
.ok_or(AsgError::DanglingExpr(oi_expr.resolve(asg).span()))?
|
||||
.add_edge_to(asg, oi_expr, None);
|
||||
let oi_container = oi_root
|
||||
.ok_or(AsgError::DanglingExpr(oi_expr.resolve(asg).span()))?;
|
||||
|
||||
oi_expr.held_by(asg, oi_container);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
|
|
@ -197,6 +197,11 @@ impl Asg {
|
|||
///
|
||||
/// For more information on how the ASG's ontology is enforced statically,
|
||||
/// see [`ObjectRelTo`](object::ObjectRelTo).
|
||||
///
|
||||
/// Callers external to this module should use [`ObjectIndex`] APIs to
|
||||
/// manipulate the graph;
|
||||
/// this allows those objects to uphold their own invariants
|
||||
/// relative to the state of the graph.
|
||||
fn add_edge<OB: ObjectKind + ObjectRelatable>(
|
||||
&mut self,
|
||||
from_oi: impl ObjectIndexRelTo<OB>,
|
||||
|
|
|
@ -587,13 +587,44 @@ impl<O: ObjectKind> ObjectIndex<O> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Add an edge from `self` to `to_oi` on the provided [`Asg`].
|
||||
///
|
||||
/// Since the only invariant asserted by [`ObjectIndexRelTo`] is that
|
||||
/// it may be related to `OB`,
|
||||
/// this method will only permit edges to `OB`;
|
||||
/// nothing else about the inner object is statically known.
|
||||
///
|
||||
/// See also [`Self::add_edge_from`].
|
||||
///
|
||||
/// _This method must remain private_,
|
||||
/// forcing callers to go through APIs for specific operations that
|
||||
/// allow objects to enforce their own invariants.
|
||||
/// This is also the reason why this method is defined here rather than
|
||||
/// on [`ObjectIndexRelTo`].
|
||||
fn add_edge_to<OB: ObjectRelatable>(
|
||||
self,
|
||||
asg: &mut Asg,
|
||||
to_oi: ObjectIndex<OB>,
|
||||
ctx_span: Option<Span>,
|
||||
) -> Self
|
||||
where
|
||||
Self: ObjectIndexRelTo<OB>,
|
||||
{
|
||||
asg.add_edge(self, to_oi, ctx_span);
|
||||
self
|
||||
}
|
||||
|
||||
/// Add an edge from `from_oi` to `self` on the provided [`Asg`].
|
||||
///
|
||||
/// An edge can only be added if ontologically valid;
|
||||
/// see [`ObjectRelTo`] for more information.
|
||||
///
|
||||
/// See also [`Self::add_edge_to`].
|
||||
pub fn add_edge_from<OF: ObjectIndexRelTo<O>>(
|
||||
///
|
||||
/// _This method must remain private_,
|
||||
/// forcing callers to go through APIs for specific operations that
|
||||
/// allow objects to enforce their own invariants.
|
||||
fn add_edge_from<OF: ObjectIndexRelTo<O>>(
|
||||
self,
|
||||
asg: &mut Asg,
|
||||
from_oi: OF,
|
||||
|
@ -831,6 +862,15 @@ impl<O: ObjectKind> ObjectIndex<O> {
|
|||
self.incoming_edges_filtered(asg).next()
|
||||
}
|
||||
|
||||
/// Indicate that the given identifier `oi` is defined by this object.
|
||||
pub fn defines(self, asg: &mut Asg, oi: ObjectIndex<Ident>) -> Self
|
||||
where
|
||||
Self: ObjectIndexRelTo<Ident>,
|
||||
{
|
||||
oi.defined_by(asg, self);
|
||||
self
|
||||
}
|
||||
|
||||
/// Describe this expression using a short independent clause.
|
||||
///
|
||||
/// This is intended to be a concise description for use either as a
|
||||
|
|
|
@ -19,7 +19,7 @@
|
|||
|
||||
//! Expressions on the ASG.
|
||||
|
||||
use super::{prelude::*, Doc, Ident, Tpl};
|
||||
use super::{prelude::*, Doc, Ident, ObjectIndexTo, Tpl};
|
||||
use crate::{f::Functor, num::Dim, span::Span};
|
||||
use std::fmt::Display;
|
||||
|
||||
|
@ -243,4 +243,24 @@ impl ObjectIndex<Expr> {
|
|||
pub fn ref_expr(self, asg: &mut Asg, oi_ident: ObjectIndex<Ident>) -> Self {
|
||||
self.add_edge_to(asg, oi_ident, Some(oi_ident.span()))
|
||||
}
|
||||
|
||||
/// The expression is held by the container `oi_container`.
|
||||
///
|
||||
/// This is intended to convey that an expression would otherwise be
|
||||
/// dangling (unreachable) were it not for the properties
|
||||
/// of `oi_container`.
|
||||
/// If this is not true,
|
||||
/// consider using:
|
||||
///
|
||||
/// 1. [`Self::create_subexpr`] to create and assign ownership of
|
||||
/// expressions contained within other expressions; or
|
||||
/// 2. [`ObjectIndex<Ident>::bind_definition`] if this expression is to
|
||||
/// be assigned to an identifier.
|
||||
pub fn held_by(
|
||||
&self,
|
||||
asg: &mut Asg,
|
||||
oi_container: ObjectIndexTo<Expr>,
|
||||
) -> Self {
|
||||
self.add_edge_from(asg, oi_container, None)
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1316,6 +1316,20 @@ impl ObjectIndex<Ident> {
|
|||
self.incoming_edges_filtered(asg).next()
|
||||
}
|
||||
|
||||
/// Root this identifier into the provided object,
|
||||
/// as if making the statement "`oi_root` defines `self`".
|
||||
///
|
||||
/// This causes `oi_root` to act as an owner of this identifier.
|
||||
/// An identifier really only ought to have one owner,
|
||||
/// but this is not enforced here.
|
||||
pub fn defined_by(
|
||||
&self,
|
||||
asg: &mut Asg,
|
||||
oi_root: impl ObjectIndexRelTo<Ident>,
|
||||
) -> Self {
|
||||
self.add_edge_from(asg, oi_root, None)
|
||||
}
|
||||
|
||||
/// Declare that `oi_dep` is an opaque dependency of `self`.
|
||||
pub fn add_opaque_dep(
|
||||
&self,
|
||||
|
|
|
@ -826,39 +826,11 @@ pub trait ObjectIndexRelTo<OB: ObjectRelatable>: Sized + Clone + Copy {
|
|||
/// See [`ObjectIndex::widen`] for more information.
|
||||
fn widen(&self) -> ObjectIndex<Object>;
|
||||
|
||||
/// Add an edge from `self` to `to_oi` on the provided [`Asg`].
|
||||
///
|
||||
/// Since the only invariant asserted by [`ObjectIndexRelTo`] is that
|
||||
/// it may be related to `OB`,
|
||||
/// this method will only permit edges to `OB`;
|
||||
/// nothing else about the inner object is statically known.
|
||||
/// To create edges to other types of objects,
|
||||
/// and for more information about this operation
|
||||
/// (including `ctx_span`),
|
||||
/// see [`ObjectIndex::add_edge_to`].
|
||||
fn add_edge_to(
|
||||
self,
|
||||
asg: &mut Asg,
|
||||
to_oi: ObjectIndex<OB>,
|
||||
ctx_span: Option<Span>,
|
||||
) -> Self {
|
||||
asg.add_edge(self, to_oi, ctx_span);
|
||||
self
|
||||
}
|
||||
|
||||
/// Check whether an edge exists from `self` to `to_oi`.
|
||||
fn has_edge_to(&self, asg: &Asg, to_oi: ObjectIndex<OB>) -> bool {
|
||||
asg.has_edge(*self, to_oi)
|
||||
}
|
||||
|
||||
/// Indicate that the given identifier `oi` is defined by this object.
|
||||
fn defines(self, asg: &mut Asg, oi: ObjectIndex<Ident>) -> Self
|
||||
where
|
||||
Self: ObjectIndexRelTo<Ident>,
|
||||
{
|
||||
self.add_edge_to(asg, oi, None)
|
||||
}
|
||||
|
||||
/// Iterate over the [`ObjectIndex`]es of the outgoing edges of `self`
|
||||
/// that match the [`ObjectKind`] `OB`.
|
||||
///
|
||||
|
|
Loading…
Reference in New Issue