employer
/
tame
1
0
Fork 0
Code Packages Releases Activity

tamer: asg: New ObjectContainer for Node type 

Working with the graph can be confusing with all of the layers
involved.  This begins to provide a better layer of abstraction that can
encapsulate the concept and enforce invariants.

Since I'm better able to enforce invariants now, this also removes the span
from the diagnostic message, since the invariant is now always enforced with
certainty.  I'm not removing the runtime panic, though; we can revisit that
if future profiling shows that it makes a negative impact.

DEV-13160
main
Mike Gerwitz 2023-01-10 15:06:24 -05:00
parent 8786ee74fa
commit 5e13c93a8f
3 changed files with 143 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
tamer/src/asg/graph.rs
View File

@ -19,6 +19,7 @@
//! Abstract graph as the basis for concrete ASGs.
use super::object::ObjectContainer;
use super::{
AsgError, FragmentText, Ident, IdentKind, Object, ObjectIndex, ObjectKind,
Source, TransitionResult,
@ -48,11 +49,8 @@ pub type AsgResult<T> = Result<T, AsgError>;
/// There are currently no data stored on edges ("edge weights").
pub type AsgEdge = ();
/// Each node of the graph represents an object.
///
/// Enclosed in an [`Option`] to permit moving owned values out of the
/// graph.
pub type Node = Option<Object>;
/// Each node of the graph.
pub type Node = ObjectContainer;
/// Index size for Graph nodes and edges.
type Ix = global::ProgSymSize;
@ -126,14 +124,15 @@ impl Asg {
let mut graph = Graph::with_capacity(objects, edges);
let mut index = Vec::with_capacity(objects);
// Exhaust the first index to be used as a placeholder.
let empty_node = graph.add_node(None);
// Exhaust the first index to be used as a placeholder
// (its value does not matter).
let empty_node = graph.add_node(Object::Root.into());
index.push(empty_node);
// 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(Some(Object::Root));
let root_node = graph.add_node(Object::Root.into());
Self {
graph,
@ -193,7 +192,7 @@ impl Asg {
let sym = ident.symbol();
self.lookup(sym).unwrap_or_else(|| {
let index = self.graph.add_node(Some(Ident::declare(ident).into()));
let index = self.graph.add_node(Ident::declare(ident).into());
self.index_identifier(sym, index);
ObjectIndex::new(index, ident.span())
@ -236,22 +235,12 @@ impl Asg {
where
F: FnOnce(Ident) -> TransitionResult<Ident>,
{
let node = self.graph.node_weight_mut(identi.into()).unwrap();
let container = self.graph.node_weight_mut(identi.into()).unwrap();
let obj = node
.take()
.expect("internal error: missing object")
.unwrap_ident();
f(obj)
.and_then(|obj| {
node.replace(obj.into());
Ok(identi)
})
.or_else(|(orig, err)| {
node.replace(orig.into());
Err(err.into())
})
container
.try_replace_with(f)
.map(|()| identi)
.map_err(Into::into)
}
// TODO: This is transitional;
@ -379,7 +368,7 @@ impl Asg {
pub(super) fn create<O: ObjectKind>(&mut self, obj: O) -> ObjectIndex<O> {
let o = obj.into();
let span = o.span();
let node_id = self.graph.add_node(Some(o));
let node_id = self.graph.add_node(ObjectContainer::from(o.into()));
ObjectIndex::new(node_id, span)
}
@ -393,11 +382,9 @@ impl Asg {
/// It is nevertheless wrapped in an [`Option`] just in case.
#[inline]
pub fn get<O: ObjectKind>(&self, index: ObjectIndex<O>) -> Option<&O> {
self.graph.node_weight(index.into()).map(|node| {
node.as_ref()
.expect("internal error: Asg::get missing Node data")
.into()
})
self.graph
.node_weight(index.into())
.map(ObjectContainer::get)
}
/// Map over an inner [`Object`] referenced by [`ObjectIndex`].
@ -441,24 +428,9 @@ impl Asg {
"invalid ObjectIndex: data are missing from the ASG",
);
// Any function borrowing from the graph ought to also be responsible
// for returning it in all possible code paths,
// as we are.
// And error here means that this must not be the case,
// or that we're breaking encapsulation somewhere.
let cur_obj = obj_container.take().diagnostic_expect(
diagnostic_borrowed_node_desc(index),
"inaccessible ObjectIndex: object has not been returned to the ASG",
);
obj_container.replace_with(f);
let new_obj: Object = f(cur_obj.into()).into();
let new_span = new_obj.span();
// This is where we replace the object that we borrowed,
// as referenced in the above panic.
obj_container.replace(new_obj);
index.overwrite(new_span)
index.overwrite(obj_container.get::<Object>().span())
}
/// Retrieve the [`ObjectIndex`] to which the given `ident` is bound,
@ -511,18 +483,18 @@ impl Asg {
/// (that allows for the invariant to be violated)
/// or direct manipulation of the underlying graph.
pub fn get_ident_obj<O: ObjectKind>(&self, ident: SPair) -> Option<&O> {
self.get_ident_obj_oi::<O>(ident).map(|oi| {
let obj_container =
self.graph.node_weight(oi.into()).diagnostic_expect(
diagnostic_node_missing_desc(oi),
"invalid ObjectIndex: data are missing from the ASG",
);
self.get_ident_obj_oi::<O>(ident)
.map(|oi| self.expect_obj(oi))
}
obj_container.as_ref().diagnostic_expect(
diagnostic_borrowed_node_desc(oi),
"inaccessible ObjectIndex: object has not been returned to the ASG",
).into()
})
pub(super) fn expect_obj<O: ObjectKind>(&self, oi: ObjectIndex<O>) -> &O {
let obj_container =
self.graph.node_weight(oi.into()).diagnostic_expect(
diagnostic_node_missing_desc(oi),
"invalid ObjectIndex: data are missing from the ASG",
);
obj_container.get()
}
/// Attempt to retrieve the [`Object`] to which the given `ident` is bound,
@ -645,23 +617,6 @@ fn diagnostic_node_missing_desc<O: ObjectKind>(
]
}
fn diagnostic_borrowed_node_desc<O: ObjectKind>(
index: ObjectIndex<O>,
) -> Vec<AnnotatedSpan<'static>> {
vec![
index.internal_error(
"this object was borrowed from the graph and \
was not returned",
),
index.help("this means that some operation used take() on the object"),
index.help(" container but never replaced it with an updated object"),
index.help(
" after the operation completed, which should not \
be possible.",
),
]
}
fn diagnostic_opaque_ident_desc(ident: SPair) -> Vec<AnnotatedSpan<'static>> {
vec![
ident.internal_error(

113
tamer/src/asg/object.rs
View File

@ -61,15 +61,15 @@
//! [`ObjectIndex`] is associated with a span derived from the point of its
//! creation to handle this diagnostic situation automatically.
use super::{Expr, Ident};
use super::{Asg, Expr, Ident};
use crate::{
diagnose::Annotate,
diagnose::{panic::DiagnosticPanic, Annotate, AnnotatedSpan},
diagnostic_panic,
f::Functor,
span::{Span, UNKNOWN_SPAN},
};
use petgraph::graph::NodeIndex;
use std::{fmt::Display, marker::PhantomData};
use std::{convert::Infallible, fmt::Display, marker::PhantomData};
/// An object on the ASG.
///
@ -315,3 +315,110 @@ impl<O: ObjectKind> From<ObjectIndex<O>> for Span {
}
}
}
/// A container for an [`Object`] allowing for owned borrowing of data.
///
/// The purpose of allowing this owned borrowing is to permit a functional
/// style of object manipulation,
/// like the rest of the TAMER system,
/// despite the mutable underpinnings of the ASG.
/// This is accomplished by wrapping each object in an [`Option`] so that we
/// can [`Option::take`] its inner value temporarily.
///
/// This container has a critical invariant:
/// the inner [`Option`] must _never_ be [`None`] after a method exits,
/// no matter what branches are taken.
/// Methods operating on owned data enforce this invariant by mapping over
/// data and immediately placing the new value to the container before the
/// method completes.
/// This container will panic if this variant is not upheld.
///
/// TODO: Make this `pub(super)` when [`Asg`]'s public API is cleaned up.
#[derive(Debug, PartialEq)]
pub struct ObjectContainer(Option<Object>);
impl ObjectContainer {
/// Retrieve an immutable reference to the inner [`Object`],
/// narrowed to expected type `O`.
///
/// Panics
/// ======
/// This will panic if the object on the graph is not the expected
/// [`ObjectKind`] `O`.
pub fn get<O: ObjectKind>(&self) -> &O {
let Self(container) = self;
container
.as_ref()
.diagnostic_unwrap(container_oops())
.into()
}
/// Attempt to modify the inner [`Object`],
/// narrowed to expected type `O`,
/// returning any error.
///
/// See also [`Self::replace_with`] if the operation is [`Infallible`].
///
/// Panics
/// ======
/// This will panic if the object on the graph is not the expected
/// [`ObjectKind`] `O`.
pub fn try_replace_with<O: ObjectKind, E>(
&mut self,
f: impl FnOnce(O) -> Result<O, (O, E)>,
) -> Result<(), E> {
let ObjectContainer(container) = self;
let obj = container.take().diagnostic_unwrap(container_oops()).into();
// NB: We must return the object to the container in all code paths!
let result = f(obj)
.map(|obj| {
container.replace(obj.into());
})
.map_err(|(orig, err)| {
container.replace(orig.into());
err
});
debug_assert!(container.is_some());
result
}
/// Modify the inner [`Object`],
/// narrowed to expected type `O`.
///
/// See also [`Self::try_replace_with`] if the operation can fail.
///
/// Panics
/// ======
/// This will panic if the object on the graph is not the expected
/// [`ObjectKind`] `O`.
pub fn replace_with<O: ObjectKind>(&mut self, f: impl FnOnce(O) -> O) {
let _ = self.try_replace_with::<O, (O, Infallible)>(|obj| Ok(f(obj)));
}
}
impl<I: Into<Object>> From<I> for ObjectContainer {
fn from(obj: I) -> Self {
Self(Some(obj.into()))
}
}
fn container_oops() -> Vec<AnnotatedSpan<'static>> {
// This used to be a real span,
// but since this invariant is easily verified and should absolutely
// never occur,
// there's no point in complicating the API.
let span = UNKNOWN_SPAN;
vec![
span.help("this means that some operation used take() on the object"),
span.help(" container but never replaced it with an updated object"),
span.help(
" after the operation completed, which should not \
be possible.",
),
]
}

11
tamer/src/ld/poc.rs
View File

@ -113,8 +113,8 @@ pub fn graphml(package_path: &str, output: &str) -> Result<(), TameldError> {
GraphMl::new(&g)
.pretty_print(true)
.export_node_weights(Box::new(|node| {
let (name, kind, generated) = match node {
Some(Object::Ident(n)) => {
let (name, kind, generated) = match node.get() {
Object::Ident(n) => {
let generated = match n.src() {
Some(src) => src.generated,
None => false,
@ -127,16 +127,11 @@ pub fn graphml(package_path: &str, output: &str) -> Result<(), TameldError> {
)
}
// TODO: We want these filtered.
Some(_) => (
_ => (
String::from("non-ident"),
"non-ident".into(),
"false".into(),
),
None => (
String::from("missing"),
"missing".into(),
"false".into(),
),
};
vec![