From 28b83ad6a3d7ef3c60ad2b8895b031758c8ee3d1 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 24 Jul 2023 16:41:32 -0400 Subject: [PATCH] tamer: asg::graph::AsgObjectMut: Allow objects to assert ownership over relationships There's a lot to say about this; it's been a bit of a struggle figuring out what I wanted to do here. First: this allows objects to use `AsgObjectMut` to control whether an edge is permitted to be added, or to cache information about an edge that is about to be added. But no object does that yet; it just uses the default trait implementation, and so this _does not change any current behavior_. It also is approximately equivalent cycle-count-wise, according to Valgrind (within ~100 cycles out of hundreds of millions on large package tests). Adding edges to the graph is still infallible _after having received permission_ from an `ObjectIndexRelTo`, but the object is free to reject the edge with an `AsgError`. As an example of where this will be useful: the template system needs to keep track of what is in the body of a template as it is defined. But the `TplAirAggregate` parser is sidelined while expressions in the body are parsed, and edges are added to a dynamic source using `ObjectIndexRelTo`. Consequently, we cannot rely on a static API to cache information; we have to be able to react dynamically. This will allow `Tpl` objects to know any time edges are added and, therefore, determine their shape as the graph is being built, rather than having to traverse the tree after encountering a close. (I _could_ change this, but `ObjectIndexRelTo` removes a significant amount of complexity for the caller, so I'd rather not.) I did explore other options. I rejected the first one, then rejected this one, then rejected the first one again before returning back to this one after having previously sidelined the entire thing, because of the above example. The core point is: I need confidence that the graph isn't being changed in ways that I forgot about, and because of the complexity of the system and the heavy refactoring that I do, I need the compiler's help; otherwise I risk introducing subtle bugs as objects get out of sync with the actual state of the graph. (I wish the graph supported these things directly, but that's a project well outside the scope of my TAMER work. So I have to make do, as I have been all this time, by layering atop of Petgraph.) (...I'm beginning to ramble.) (...beginning?) Anyway: my other rejected idea was to provide attestation via the `ObjectIndex` APIs to force callers to go through those APIs to add an edge to the graph; it would use sealed objects that are inaccessible to any modules other than the objects, and assert that the caller is able to provide a zero-sized object of that sealed type. The problem with this is...exactly what was mentioned above: `ObjectIndexRelTo` is dynamic. We don't always know the source object type statically, and so we cannot make those static assertions. I could have tried the same tricks to store attestation at some other time, but what a confusing mess it would be. And so here we are. Most of this work is cleaning up the callers---adding edges is now fallible, from the `ObjectIndex` API standpoint, and so AIR needed to be set up to handle those failures. There _aren't_ any failures yet, but again, since things are dynamic, they could appear at any moment. Furthermore, since ref/def is commutative (things can be defined and referenced in any order), there could be surprise errors on edge additions in places that might not otherwise expect it in the future. We're now ready for that, and I'll be able to e.g. traverse incoming edges on a `Missing->Transparent` definition to notify dependents. This project is going to be the end of me. As interesting as it is. I can see why Rust just chose to require macro definitions _before_ use. So much less work. DEV-13163 --- tamer/src/asg/air.rs | 11 ++-- tamer/src/asg/air/expr.rs | 27 +++++----- tamer/src/asg/air/meta.rs | 22 ++++---- tamer/src/asg/air/opaque.rs | 16 +++--- tamer/src/asg/air/pkg.rs | 8 +-- tamer/src/asg/air/tpl.rs | 36 ++++++------- tamer/src/asg/graph.rs | 69 ++++++++++++++++++++++--- tamer/src/asg/graph/object.rs | 46 ++++++++++------- tamer/src/asg/graph/object/doc.rs | 2 + tamer/src/asg/graph/object/expr.rs | 12 +++-- tamer/src/asg/graph/object/ident.rs | 10 ++-- tamer/src/asg/graph/object/meta.rs | 20 ++++++-- tamer/src/asg/graph/object/pkg.rs | 10 +++- tamer/src/asg/graph/object/rel.rs | 79 ++++++++++++++++++++++++++++- tamer/src/asg/graph/object/root.rs | 4 +- tamer/src/asg/graph/object/tpl.rs | 12 +++-- 16 files changed, 283 insertions(+), 101 deletions(-) diff --git a/tamer/src/asg/air.rs b/tamer/src/asg/air.rs index 839778af..4e529b78 100644 --- a/tamer/src/asg/air.rs +++ b/tamer/src/asg/air.rs @@ -863,7 +863,7 @@ impl AirAggregateCtx { }, )?; - oi_pkg.root(&mut self.asg); + oi_pkg.root(&mut self.asg)?; self.ooi_pkg.replace(oi_pkg); Ok(oi_pkg) @@ -1005,9 +1005,8 @@ impl AirAggregateCtx { .rooting_oi() .ok_or(AsgError::InvalidBindContext(binding_name))?; - Ok(self - .lookup_lexical_or_missing(binding_name) - .defined_by(self.asg_mut(), oi_root)) + 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 @@ -1054,9 +1053,9 @@ impl AirAggregateCtx { Some((_, oi_root)) => { let oi_meta_ident = self.lookup_lexical_or_missing(meta_name); - Ok(oi_meta_ident + oi_meta_ident .new_abstract_ident(self.asg_mut(), meta_name.span()) - .defined_by(self.asg_mut(), oi_root)) + .and_then(|oi| oi.defined_by(self.asg_mut(), oi_root)) } } } diff --git a/tamer/src/asg/air/expr.rs b/tamer/src/asg/air/expr.rs index e93fd92a..11b29ecd 100644 --- a/tamer/src/asg/air/expr.rs +++ b/tamer/src/asg/air/expr.rs @@ -93,8 +93,12 @@ impl ParseState for AirExprAggregate { } (BuildingExpr(es, poi), AirExpr(ExprStart(op, span))) => { - let oi = poi.create_subexpr(ctx.asg_mut(), Expr::new(op, span)); - Transition(BuildingExpr(es.push(poi), oi)).incomplete() + match poi.create_subexpr(ctx.asg_mut(), Expr::new(op, span)) { + Ok(oi) => { + Transition(BuildingExpr(es.push(poi), oi)).incomplete() + } + Err(e) => Transition(BuildingExpr(es, poi)).err(e), + } } (BuildingExpr(es, oi), AirExpr(ExprEnd(end))) => { @@ -156,17 +160,16 @@ impl ParseState for AirExprAggregate { (BuildingExpr(es, oi), AirBind(RefIdent(name))) => { let oi_ident = ctx.lookup_lexical_or_missing(name); - Transition(BuildingExpr( - es, - oi.ref_expr(ctx.asg_mut(), oi_ident), - )) - .incomplete() + + oi.ref_expr(ctx.asg_mut(), oi_ident) + .map(|_| ()) + .transition(BuildingExpr(es, oi)) } - (BuildingExpr(es, oi), AirDoc(DocIndepClause(clause))) => { - oi.add_desc_short(ctx.asg_mut(), clause); - Transition(BuildingExpr(es, oi)).incomplete() - } + (BuildingExpr(es, oi), AirDoc(DocIndepClause(clause))) => oi + .add_desc_short(ctx.asg_mut(), clause) + .map(|_| ()) + .transition(BuildingExpr(es, oi)), (BuildingExpr(es, oi), AirDoc(DocText(text))) => Transition( BuildingExpr(es, oi), @@ -207,7 +210,7 @@ impl AirExprAggregate { let oi_container = oi_root .ok_or(AsgError::DanglingExpr(oi_expr.resolve(asg).span()))?; - oi_expr.held_by(asg, oi_container); + oi_expr.held_by(asg, oi_container)?; Ok(()) } diff --git a/tamer/src/asg/air/meta.rs b/tamer/src/asg/air/meta.rs index b26a3350..6815b0b8 100644 --- a/tamer/src/asg/air/meta.rs +++ b/tamer/src/asg/air/meta.rs @@ -75,10 +75,10 @@ impl ParseState for AirMetaAggregate { Transition(Ready).incomplete() } - (TplMeta(oi_meta), AirMeta(MetaLexeme(lexeme))) => Transition( - TplMeta(oi_meta.append_lexeme(ctx.asg_mut(), lexeme)), - ) - .incomplete(), + (TplMeta(oi_meta), AirMeta(MetaLexeme(lexeme))) => oi_meta + .append_lexeme(ctx.asg_mut(), lexeme) + .map(|_| ()) + .transition(TplMeta(oi_meta)), (TplMeta(oi_meta), AirBind(BindIdent(name))) => ctx .defines_concrete(name) @@ -101,10 +101,10 @@ impl ParseState for AirMetaAggregate { ) } - (TplMeta(oi_meta), AirDoc(DocIndepClause(clause))) => { - oi_meta.add_desc_short(ctx.asg_mut(), clause); - Transition(TplMeta(oi_meta)).incomplete() - } + (TplMeta(oi_meta), AirDoc(DocIndepClause(clause))) => oi_meta + .add_desc_short(ctx.asg_mut(), clause) + .map(|_| ()) + .transition(TplMeta(oi_meta)), // TODO: The user _probably_ meant to use `` in XML NIR, // so maybe we should have an error to that effect. @@ -121,8 +121,10 @@ impl ParseState for AirMetaAggregate { (TplMeta(oi_meta), AirBind(RefIdent(name))) => { let oi_ref = ctx.lookup_lexical_or_missing(name); - Transition(TplMeta(oi_meta.concat_ref(ctx.asg_mut(), oi_ref))) - .incomplete() + oi_meta + .concat_ref(ctx.asg_mut(), oi_ref) + .map(|_| ()) + .transition(TplMeta(oi_meta)) } (TplMeta(..), tok @ AirMeta(MetaStart(..))) => { diff --git a/tamer/src/asg/air/opaque.rs b/tamer/src/asg/air/opaque.rs index 2b5c7917..7a4b631f 100644 --- a/tamer/src/asg/air/opaque.rs +++ b/tamer/src/asg/air/opaque.rs @@ -73,9 +73,11 @@ impl ParseState for AirOpaqueAggregate { (Ready, IdentDep(name, dep)) => { let oi_from = ctx.lookup_lexical_or_missing(name); let oi_to = ctx.lookup_lexical_or_missing(dep); - oi_from.add_opaque_dep(ctx.asg_mut(), oi_to); - Transition(Ready).incomplete() + oi_from + .add_opaque_dep(ctx.asg_mut(), oi_to) + .map(|_| ()) + .transition(Ready) } (Ready, IdentFragment(name, text)) => ctx @@ -84,11 +86,11 @@ impl ParseState for AirOpaqueAggregate { .map(|_| ()) .transition(Ready), - (Ready, IdentRoot(name)) => { - ctx.lookup_lexical_or_missing(name).root(ctx.asg_mut()); - - Transition(Ready).incomplete() - } + (Ready, IdentRoot(name)) => ctx + .lookup_lexical_or_missing(name) + .root(ctx.asg_mut()) + .map(|_| ()) + .transition(Ready), } } diff --git a/tamer/src/asg/air/pkg.rs b/tamer/src/asg/air/pkg.rs index e68d1545..714a6e5f 100644 --- a/tamer/src/asg/air/pkg.rs +++ b/tamer/src/asg/air/pkg.rs @@ -112,10 +112,10 @@ impl ParseState for AirPkgAggregate { ) } - (Toplevel(oi_pkg), AirDoc(DocText(text))) => { - oi_pkg.append_doc_text(ctx.asg_mut(), text); - Transition(Toplevel(oi_pkg)).incomplete() - } + (Toplevel(oi_pkg), AirDoc(DocText(text))) => oi_pkg + .append_doc_text(ctx.asg_mut(), text) + .map(|_| ()) + .transition(Toplevel(oi_pkg)), // Package import (Toplevel(oi_pkg), AirPkg(PkgImport(namespec))) => oi_pkg diff --git a/tamer/src/asg/air/tpl.rs b/tamer/src/asg/air/tpl.rs index 5d488165..5bcab3e4 100644 --- a/tamer/src/asg/air/tpl.rs +++ b/tamer/src/asg/air/tpl.rs @@ -199,20 +199,23 @@ impl ParseState for AirTplAggregate { let tpl_oi = tpl.oi(); let ref_oi = ctx.lookup_lexical_or_missing(name); - tpl_oi.apply_named_tpl(ctx.asg_mut(), ref_oi, name.span()); - - Transition(Toplevel(tpl)).incomplete() + tpl_oi + .apply_named_tpl(ctx.asg_mut(), ref_oi, name.span()) + .map(|_| ()) + .transition(Toplevel(tpl)) } - (Toplevel(tpl), AirDoc(DocIndepClause(clause))) => { - tpl.oi().add_desc_short(ctx.asg_mut(), clause); - Transition(Toplevel(tpl)).incomplete() - } + (Toplevel(tpl), AirDoc(DocIndepClause(clause))) => tpl + .oi() + .add_desc_short(ctx.asg_mut(), clause) + .map(|_| ()) + .transition(Toplevel(tpl)), - (Toplevel(tpl), AirDoc(DocText(text))) => { - tpl.oi().append_doc_text(ctx.asg_mut(), text); - Transition(Toplevel(tpl)).incomplete() - } + (Toplevel(tpl), AirDoc(DocText(text))) => tpl + .oi() + .append_doc_text(ctx.asg_mut(), text) + .map(|_| ()) + .transition(Toplevel(tpl)), (Toplevel(tpl), AirTpl(TplEnd(span))) => { tpl.close(ctx.asg_mut(), span).transition(Done) @@ -224,12 +227,11 @@ impl ParseState for AirTplAggregate { // we are effectively discarding the ref and translating // into a `TplEnd`. match ctx.expansion_oi() { - Some(oi_target) => { - tpl.oi().expand_into(ctx.asg_mut(), oi_target); - - Transition(Toplevel(tpl.anonymous_reachable())) - .incomplete() - } + Some(oi_target) => tpl + .oi() + .expand_into(ctx.asg_mut(), oi_target) + .map(|_| ()) + .transition(Toplevel(tpl.anonymous_reachable())), None => Transition(Toplevel(tpl)) .err(AsgError::InvalidExpansionContext(span)), } diff --git a/tamer/src/asg/graph.rs b/tamer/src/asg/graph.rs index 79488c4c..c24520c2 100644 --- a/tamer/src/asg/graph.rs +++ b/tamer/src/asg/graph.rs @@ -202,17 +202,19 @@ impl Asg { /// manipulate the graph; /// this allows those objects to uphold their own invariants /// relative to the state of the graph. - fn add_edge( + fn add_edge, OB: ObjectKind + ObjectRelatable>( &mut self, - from_oi: impl ObjectIndexRelTo, + from_oi: OA, to_oi: ObjectIndex, ctx_span: Option, - ) { - self.graph.add_edge( - from_oi.widen().into(), - to_oi.into(), - (from_oi.src_rel_ty(), OB::rel_ty(), ctx_span), - ); + ) -> Result<(), AsgError> { + from_oi.pre_add_edge(self, to_oi, ctx_span, |asg| { + asg.graph.add_edge( + from_oi.widen().into(), + to_oi.into(), + (from_oi.src_rel_ty(), OB::rel_ty(), ctx_span), + ); + }) } /// Retrieve an object from the graph by [`ObjectIndex`]. @@ -391,5 +393,56 @@ fn diagnostic_node_missing_desc( ] } +/// Mutation of an [`Object`] or its edges on the [`Asg`]. +/// +/// This trait is intended to delegate certain responsibilities to +/// [`ObjectKind`]s so that they may enforce their own invariants with +/// respect to their relationships to other objects on the graph. +/// +/// TODO: It'd be nice if only [`Asg`] were able to invoke methods on this +/// trait, +/// but the current module structure together with Rust's visibility +/// with sibling modules doesn't seem to make that possible. +pub trait AsgObjectMut: ObjectKind { + /// Allow an object to handle or reject the creation of an edge from it + /// to another object. + /// + /// Objects own both their node on the graph and the edges _from_ that + /// node to another object. + /// Phrased another way: + /// they own their data and their relationships. + /// + /// This gives an opportunity for the [`ObjectKind`] associated with the + /// source object to evaluate the proposed relationship. + /// This guarantee allows objects to cache information about these + /// relationships and enforce any associated invariants without + /// worrying about how the object may change out from underneath + /// them. + /// In some cases, + /// this is the only way that an object will know whether an edge has + /// been added, + /// since the [`ObjectIndex`] APIs may not be utilized + /// (e.g. in the case of [`ObjectIndexRelTo`]. + /// + /// This is invoked by [`Asg::add_edge`]. + /// The provided `commit` callback will complete the addition of the + /// edge if provided [`Ok`], + /// and the commit cannot fail. + /// If [`Err`] is provided to `commit`, + /// then [`Asg::add_edge`] will fail with that error. + fn pre_add_edge< + OA: ObjectIndexRelTo, + OB: ObjectKind + ObjectRelatable, + >( + asg: &mut Asg, + _from_oi: &OA, + _to_oi: ObjectIndex, + _ctx_span: Option, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError> { + Ok(commit(asg)) + } +} + #[cfg(test)] mod test; diff --git a/tamer/src/asg/graph/object.rs b/tamer/src/asg/graph/object.rs index 9f36bc79..3c7347ff 100644 --- a/tamer/src/asg/graph/object.rs +++ b/tamer/src/asg/graph/object.rs @@ -112,8 +112,14 @@ //! Since [`ObjectRel`] narrows into an [`ObjectIndex`], //! the system will produce runtime panics if there is ever any attempt to //! follow an edge to an unexpected [`ObjectKind`]. +//! +//! In addition to these static guarantees, +//! [`AsgObjectMut`](super::AsgObjectMut) is utilized by [`Asg`] to +//! consult an object before an edge is added _from_ it, +//! allowing objects to assert ownership over their edges and cache +//! information about them as the graph is being built. -use super::Asg; +use super::{Asg, AsgError}; use crate::{ diagnose::{panic::DiagnosticPanic, Annotate, AnnotatedSpan}, diagnostic_panic, @@ -156,7 +162,7 @@ pub use tpl::Tpl; /// Often-needed exports for [`ObjectKind`]s. pub mod prelude { pub use super::{ - super::{super::error::AsgError, Asg}, + super::{super::error::AsgError, Asg, AsgObjectMut}, Object, ObjectIndex, ObjectIndexRelTo, ObjectKind, ObjectRel, ObjectRelFrom, ObjectRelTo, ObjectRelTy, ObjectRelatable, ObjectTreeRelTo, @@ -606,12 +612,11 @@ impl ObjectIndex { asg: &mut Asg, to_oi: ObjectIndex, ctx_span: Option, - ) -> Self + ) -> Result where Self: ObjectIndexRelTo, { - asg.add_edge(self, to_oi, ctx_span); - self + asg.add_edge(self, to_oi, ctx_span).map(|()| self) } /// Add an edge from `from_oi` to `self` on the provided [`Asg`]. @@ -624,17 +629,16 @@ impl ObjectIndex { /// _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>( + fn add_edge_from>( self, asg: &mut Asg, - from_oi: OF, + from_oi: OA, ctx_span: Option, - ) -> Self + ) -> Result where O: ObjectRelatable, { - asg.add_edge(from_oi, self, ctx_span); - self + asg.add_edge(from_oi, self, ctx_span).map(|()| self) } /// Create an iterator over the [`ObjectIndex`]es of the outgoing edges @@ -781,12 +785,13 @@ impl ObjectIndex { /// objects. /// Forcing objects to be reachable can prevent them from being /// optimized away if they are not used. - pub fn root(self, asg: &mut Asg) -> Self + pub fn root(self, asg: &mut Asg) -> Result where Root: ObjectRelTo, { - asg.root(self.span()).add_edge_to(asg, self, None); - self + asg.root(self.span()) + .add_edge_to(asg, self, None) + .map(|_| self) } /// Whether this object has been rooted in the ASG's [`Root`] object. @@ -863,12 +868,15 @@ impl ObjectIndex { } /// Indicate that the given identifier `oi` is defined by this object. - pub fn defines(self, asg: &mut Asg, oi: ObjectIndex) -> Self + pub fn defines( + self, + asg: &mut Asg, + oi: ObjectIndex, + ) -> Result where Self: ObjectIndexRelTo, { - oi.defined_by(asg, self); - self + oi.defined_by(asg, self).map(|_| self) } /// Describe this expression using a short independent clause. @@ -877,7 +885,11 @@ impl ObjectIndex { /// simple sentence or as part of a compound sentence. /// There should only be one such clause for any given object, /// but that is not enforced here. - pub fn add_desc_short(&self, asg: &mut Asg, clause: SPair) -> Self + pub fn add_desc_short( + &self, + asg: &mut Asg, + clause: SPair, + ) -> Result where O: ObjectRelTo, { diff --git a/tamer/src/asg/graph/object/doc.rs b/tamer/src/asg/graph/object/doc.rs index 29fb903a..6ce6af69 100644 --- a/tamer/src/asg/graph/object/doc.rs +++ b/tamer/src/asg/graph/object/doc.rs @@ -91,3 +91,5 @@ object_rel! { // empty } } + +impl AsgObjectMut for Doc {} diff --git a/tamer/src/asg/graph/object/expr.rs b/tamer/src/asg/graph/object/expr.rs index 6064c0d1..8b5a290b 100644 --- a/tamer/src/asg/graph/object/expr.rs +++ b/tamer/src/asg/graph/object/expr.rs @@ -233,14 +233,18 @@ impl ObjectIndex { self, asg: &mut Asg, expr: Expr, - ) -> ObjectIndex { + ) -> Result, AsgError> { let oi_subexpr = asg.create(expr); oi_subexpr.add_edge_from(asg, self, None) } /// Reference the value of the expression identified by `oi_ident` as if /// it were a subexpression. - pub fn ref_expr(self, asg: &mut Asg, oi_ident: ObjectIndex) -> Self { + pub fn ref_expr( + self, + asg: &mut Asg, + oi_ident: ObjectIndex, + ) -> Result { self.add_edge_to(asg, oi_ident, Some(oi_ident.span())) } @@ -260,7 +264,9 @@ impl ObjectIndex { &self, asg: &mut Asg, oi_container: ObjectIndexTo, - ) -> Self { + ) -> Result { self.add_edge_from(asg, oi_container, None) } } + +impl AsgObjectMut for Expr {} diff --git a/tamer/src/asg/graph/object/ident.rs b/tamer/src/asg/graph/object/ident.rs index 7c492ba3..500035ed 100644 --- a/tamer/src/asg/graph/object/ident.rs +++ b/tamer/src/asg/graph/object/ident.rs @@ -1262,7 +1262,7 @@ impl ObjectIndex { // and use the newly provided `id` and its span. Missing(_) => Ok(Transparent(id)), }) - .map(|ident_oi| ident_oi.add_edge_to(asg, definition, None)) + .and_then(|ident_oi| ident_oi.add_edge_to(asg, definition, None)) } /// Set the fragment associated with a concrete identifier. @@ -1326,7 +1326,7 @@ impl ObjectIndex { &self, asg: &mut Asg, oi_root: impl ObjectIndexRelTo, - ) -> Self { + ) -> Result { self.add_edge_from(asg, oi_root, None) } @@ -1335,7 +1335,7 @@ impl ObjectIndex { &self, asg: &mut Asg, oi_dep: ObjectIndex, - ) -> Self { + ) -> Result { self.add_edge_to(asg, oi_dep, None) } @@ -1400,11 +1400,13 @@ impl ObjectIndex { self, asg: &mut Asg, at: Span, - ) -> ObjectIndex { + ) -> Result, AsgError> { asg.create(Ident::new_abstract(at)) .add_edge_to(asg, self, Some(at)) } } +impl AsgObjectMut for Ident {} + #[cfg(test)] mod test; diff --git a/tamer/src/asg/graph/object/meta.rs b/tamer/src/asg/graph/object/meta.rs index afe17f65..fbc935e8 100644 --- a/tamer/src/asg/graph/object/meta.rs +++ b/tamer/src/asg/graph/object/meta.rs @@ -199,7 +199,11 @@ impl ObjectIndex { /// /// Metavariables with multiple values already represents concatenation /// and a new edge will be added without changing `self`. - pub fn append_lexeme(self, asg: &mut Asg, lexeme: SPair) -> Self { + pub fn append_lexeme( + self, + asg: &mut Asg, + lexeme: SPair, + ) -> Result { use Meta::*; let mut rels = ArrayVec::::new(); @@ -239,10 +243,10 @@ impl ObjectIndex { for rel_lexeme in rels { let oi = asg.create(Meta::Lexeme(rel_lexeme.span(), rel_lexeme)); - self.add_edge_to(asg, oi, None); + self.add_edge_to(asg, oi, None)?; } - self + Ok(self) } pub fn close(self, asg: &mut Asg, close_span: Span) -> Self { @@ -264,7 +268,11 @@ impl ObjectIndex { // from the reference location and therefore contains the reference // [`Span`]; // this is used to provide accurate diagnostic information. - pub fn concat_ref(self, asg: &mut Asg, oi_ref: ObjectIndex) -> Self { + pub fn concat_ref( + self, + asg: &mut Asg, + oi_ref: ObjectIndex, + ) -> Result { use Meta::*; // We cannot mutate the ASG within `map_obj` below because of the @@ -289,7 +297,7 @@ impl ObjectIndex { // we must add the edge before appending the ref since // concatenation will occur during expansion in edge order. if let Some(orig) = pre { - asg.create(orig).add_edge_from(asg, self, None); + asg.create(orig).add_edge_from(asg, self, None)?; } // Having been guaranteed a `ConcatList` above, @@ -298,3 +306,5 @@ impl ObjectIndex { self.add_edge_to(asg, oi_ref, Some(oi_ref.span())) } } + +impl AsgObjectMut for Meta {} diff --git a/tamer/src/asg/graph/object/pkg.rs b/tamer/src/asg/graph/object/pkg.rs index c87507ea..62a8b384 100644 --- a/tamer/src/asg/graph/object/pkg.rs +++ b/tamer/src/asg/graph/object/pkg.rs @@ -134,12 +134,18 @@ impl ObjectIndex { let parent = self.resolve(asg); let oi_import = asg.create(Pkg::new_imported(parent, namespec)?); - Ok(self.add_edge_to(asg, oi_import, Some(namespec.span()))) + self.add_edge_to(asg, oi_import, Some(namespec.span())) } /// Arbitrary text serving as documentation in a literate style. - pub fn append_doc_text(&self, asg: &mut Asg, text: SPair) -> Self { + pub fn append_doc_text( + &self, + asg: &mut Asg, + text: SPair, + ) -> Result { let oi_doc = asg.create(Doc::new_text(text)); self.add_edge_to(asg, oi_doc, None) } } + +impl AsgObjectMut for Pkg {} diff --git a/tamer/src/asg/graph/object/rel.rs b/tamer/src/asg/graph/object/rel.rs index 367370a2..fa0855e0 100644 --- a/tamer/src/asg/graph/object/rel.rs +++ b/tamer/src/asg/graph/object/rel.rs @@ -26,7 +26,10 @@ use super::{ ObjectKind, OiPairObjectInner, Pkg, Root, }; use crate::{ - asg::{graph::object::Tpl, Asg}, + asg::{ + graph::{object::Tpl, AsgObjectMut}, + Asg, AsgError, + }, f::Functor, span::Span, }; @@ -529,7 +532,7 @@ pub trait ObjectTreeRelTo: /// This is used to derive [`ObjectRelTo``], /// which can be used as a trait bound to assert a valid relationship /// between two [`Object`]s. -pub trait ObjectRelatable: ObjectKind { +pub trait ObjectRelatable: ObjectKind + AsgObjectMut { /// Sum type representing a subset of [`Object`] variants that are valid /// targets for edges from [`Self`]. /// @@ -826,6 +829,32 @@ pub trait ObjectIndexRelTo: Sized + Clone + Copy { /// See [`ObjectIndex::widen`] for more information. fn widen(&self) -> ObjectIndex; + /// Request permission to add an edge from `self` to another object. + /// + /// This gives the object ownership over the edges that are created, + /// in addition to the static guarantees provided by + /// [`ObjectIndexRelTo`]. + /// Since [`ObjectIndexRelTo` supports dynamic source objects, + /// this allows calling code to be written in a concise manner that is + /// agnostic to the source type, + /// without sacrificing edge ownership. + /// + /// For more information, + /// see [`AsgObjectMut::pre_add_edge`]. + /// + /// _This should only be called by [`Asg`]_; + /// `commit` is expected to be a continuation that adds the edge to + /// the graph, + /// and the object represented by `self` may modify itself expecting + /// such an edge to be added. + fn pre_add_edge( + &self, + asg: &mut Asg, + to_oi: ObjectIndex, + ctx_span: Option, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError>; + /// Check whether an edge exists from `self` to `to_oi`. fn has_edge_to(&self, asg: &Asg, to_oi: ObjectIndex) -> bool { asg.has_edge(*self, to_oi) @@ -901,6 +930,16 @@ where fn widen(&self) -> ObjectIndex { ObjectIndex::::widen(*self) } + + fn pre_add_edge( + &self, + asg: &mut Asg, + to_oi: ObjectIndex, + ctx_span: Option, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError> { + O::pre_add_edge(asg, self, to_oi, ctx_span, commit) + } } impl ObjectIndexRelTo for ObjectIndexTo { @@ -913,6 +952,30 @@ impl ObjectIndexRelTo for ObjectIndexTo { fn widen(&self) -> ObjectIndex { *self.as_ref() } + + fn pre_add_edge( + &self, + asg: &mut Asg, + to_oi: ObjectIndex, + ctx_span: Option, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError> { + macro_rules! pre_add_edge { + ($ty:ident) => { + $ty::pre_add_edge(asg, self, to_oi, ctx_span, commit) + }; + } + + match self.src_rel_ty() { + ObjectRelTy::Root => pre_add_edge!(Root), + ObjectRelTy::Pkg => pre_add_edge!(Pkg), + ObjectRelTy::Ident => pre_add_edge!(Ident), + ObjectRelTy::Expr => pre_add_edge!(Expr), + ObjectRelTy::Tpl => pre_add_edge!(Tpl), + ObjectRelTy::Meta => pre_add_edge!(Meta), + ObjectRelTy::Doc => pre_add_edge!(Doc), + } + } } impl ObjectIndexRelTo for ObjectIndexToTree { @@ -927,6 +990,18 @@ impl ObjectIndexRelTo for ObjectIndexToTree { Self(oito) => oito.widen(), } } + + fn pre_add_edge( + &self, + asg: &mut Asg, + to_oi: ObjectIndex, + ctx_span: Option, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError> { + match self { + Self(oito) => oito.pre_add_edge(asg, to_oi, ctx_span, commit), + } + } } impl From> for ObjectIndex { diff --git a/tamer/src/asg/graph/object/root.rs b/tamer/src/asg/graph/object/root.rs index 336b8d8a..d9a8fdf5 100644 --- a/tamer/src/asg/graph/object/root.rs +++ b/tamer/src/asg/graph/object/root.rs @@ -66,7 +66,9 @@ impl ObjectIndex { &self, asg: &mut Asg, oi: ObjectIndex, - ) -> ObjectIndex { + ) -> Result, AsgError> { oi.add_edge_from(asg, *self, None) } } + +impl AsgObjectMut for Root {} diff --git a/tamer/src/asg/graph/object/tpl.rs b/tamer/src/asg/graph/object/tpl.rs index 599856c2..29ba83bc 100644 --- a/tamer/src/asg/graph/object/tpl.rs +++ b/tamer/src/asg/graph/object/tpl.rs @@ -98,7 +98,7 @@ impl ObjectIndex { asg: &mut Asg, oi_apply: ObjectIndex, ref_span: Span, - ) -> Self { + ) -> Result { self.add_edge_to(asg, oi_apply, Some(ref_span)) } @@ -116,14 +116,20 @@ impl ObjectIndex { self, asg: &mut Asg, oi_target_parent: OP, - ) -> Self { + ) -> Result { self.add_edge_from(asg, oi_target_parent, None) } /// Arbitrary text serving as documentation in a literate style, /// to be expanded into the application site. - pub fn append_doc_text(&self, asg: &mut Asg, text: SPair) -> Self { + pub fn append_doc_text( + &self, + asg: &mut Asg, + text: SPair, + ) -> Result { let oi_doc = asg.create(Doc::new_text(text)); self.add_edge_to(asg, oi_doc, None) } } + +impl AsgObjectMut for Tpl {}