employer
/
tame
1
0
Fork 0
Code Packages Releases Activity

tamer: asg::graph::object::rel::DynObjectRel: Store source data 

This is generic over the source, just as the target, defaulting just the
same to `ObjectIndex`.

This allows us to use only the edge information provided rather than having
to perform another lookup on the graph and then assert that we found the
correct edge.  In this case, we're dealing with an `Ident->Expr` edge, of
which there is only one, but in other cases, there may be many such edges,
and it wouldn't be possible to know _which_ was referred to without also
keeping context of the previous edge in the walk.

So, in addition to avoiding more indirection and being more immune to logic
bugs, this also allows us to avoid states in `AsgTreeToXirf` for the purpose
of tracking previous edges in the current path.  And it means that the tree
walk can seed further traversals in conjunction with it, if that is so
needed for deriving sources.

More cleanup will be needed, but this does well to set us up for moving
forward; I was too uncomfortable with having to do the separate
lookup.  This is also a more intuitive API.

But it does have the awkward effect that now I don't need the pair---I just
need the `Object`---but I'm not going to remove it because I suspect I may
need it in the future.  We'll see.

The TODO references the fact that I'm using a convenient `resolve_oi_pairs`
instead of resolving only the target first and then the source only in the
code path that needs it.  I'll want to verify that Rust will properly
optimize to avoid the source resolution in branches that do not need it.

DEV-13708
main
Mike Gerwitz 2023-02-23 22:45:09 -05:00
parent cb5d54b2db
commit 3587d032c3
4 changed files with 90 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -533,7 +533,7 @@ impl Asg {
self.edges_dyn(oi.widen()).map(move |dyn_rel| {
let target_ty = dyn_rel.target_ty();
dyn_rel.narrow::<O>().diagnostic_unwrap(|| {
dyn_rel.narrow_target::<O>().diagnostic_unwrap(|| {
vec![
oi.internal_error(format!(
"encountered invalid outgoing edge type {:?}",
@ -568,6 +568,7 @@ impl Asg {
DynObjectRel::new(
*src_ty,
*target_ty,
oi,
ObjectIndex::<Object>::new(edge.target(), oi),
*ctx_span,
)

92
tamer/src/asg/graph/object/rel.rs
View File

@ -54,25 +54,26 @@ impl Display for ObjectRelTy {
/// A dynamic relationship (edge) from one object to another before it has
/// been narrowed.
///
/// The target of this edge is usually an [`ObjectIndex`],
/// but it is made generic (`T`) to support mapping while retaining useful
/// metadata,
/// The source and target of this edge are usually [`ObjectIndex`]es,
/// but it is made generic (`S, T`) to support mapping while retaining
/// useful metadata,
/// e.g. to resolve an object while retaining the edge information.
#[derive(Debug, PartialEq)]
pub struct DynObjectRel<T = ObjectIndex<Object>>(
pub struct DynObjectRel<S = ObjectIndex<Object>, T = ObjectIndex<Object>>(
(ObjectRelTy, ObjectRelTy),
T,
(S, T),
Option<Span>,
);
impl<T> DynObjectRel<T> {
impl<S, T> DynObjectRel<S, T> {
pub(in super::super) fn new(
from_ty: ObjectRelTy,
to_ty: ObjectRelTy,
x: T,
src: S,
target: T,
ctx_span: Option<Span>,
) -> Self {
Self((from_ty, to_ty), x, ctx_span)
Self((from_ty, to_ty), (src, target), ctx_span)
}
/// The type of the source edge.
@ -89,13 +90,20 @@ impl<T> DynObjectRel<T> {
}
}
/// The source of this relationship.
pub fn source(&self) -> &S {
match self {
Self(_, (oi, _), _) => oi,
}
}
/// The target of this relationship.
///
/// This type generally originates as [`ObjectIndex`] but can be mapped
/// over to retain the structured edge data.
pub fn target(&self) -> &T {
match self {
Self(_, oi, _) => oi,
Self(_, (_, oi), _) => oi,
}
}
@ -109,33 +117,28 @@ impl<T> DynObjectRel<T> {
}
}
impl DynObjectRel<ObjectIndex<Object>> {
/// See [`ObjectIndex::must_narrow_into`].
pub fn must_narrow_into<O: ObjectKind>(&self) -> ObjectIndex<O> {
match self {
Self(_, oi, _) => oi.must_narrow_into(),
}
}
/// Attempt to narrow into the [`ObjectRel`] of `O`.
impl<S> DynObjectRel<S, ObjectIndex<Object>> {
/// Attempt to narrow the target into the [`ObjectRel`] of `O`.
///
/// See [`ObjectRelatable::new_rel_dyn`] for more information.
pub fn narrow<O: ObjectKind + ObjectRelatable>(&self) -> Option<O::Rel> {
pub fn narrow_target<O: ObjectKind + ObjectRelatable>(
&self,
) -> Option<O::Rel> {
O::new_rel_dyn(self.target_ty(), *self.target())
}
/// Pair the inner [`ObjectIndex`] with its resolved [`Object`].
/// Pair the target [`ObjectIndex`] with its resolved [`Object`].
///
/// This allows the [`ObjectIndex`] to be refined alongside the inner
/// [`ObjectKind`] so that callers can make use of the refined
/// [`ObjectIndex`] without having to explicitly narrow themselves.
/// While isn't any more or less safe than the manual alternative,
/// it _does_ defend against logic bugs.
pub fn resolve_oi_pair(
pub fn resolve_target_oi_pair(
self,
asg: &Asg,
) -> DynObjectRel<Object<OiPairObjectInner>> {
self.map(|oi| oi.resolve(asg).pair_oi(oi))
) -> DynObjectRel<S, Object<OiPairObjectInner>> {
self.map(|(soi, toi)| (soi, toi.resolve(asg).pair_oi(toi)))
}
/// Dynamically determine whether this edge represents a cross edge.
@ -165,7 +168,7 @@ impl DynObjectRel<ObjectIndex<Object>> {
match self.source_ty() {
$(
ObjectRelTy::$ty => {
self.narrow::<$ty>().is_some_and(
self.narrow_target::<$ty>().is_some_and(
|rel| rel.is_cross_edge()
)
},
@ -178,10 +181,41 @@ impl DynObjectRel<ObjectIndex<Object>> {
}
}
impl<T, U> Functor<T, U> for DynObjectRel<T> {
type Target = DynObjectRel<U>;
impl<T> DynObjectRel<ObjectIndex<Object>, T> {
/// Pair the source [`ObjectIndex`] with its resolved [`Object`].
///
/// This allows the [`ObjectIndex`] to be refined alongside the inner
/// [`ObjectKind`] so that callers can make use of the refined
/// [`ObjectIndex`] without having to explicitly narrow themselves.
/// While isn't any more or less safe than the manual alternative,
/// it _does_ defend against logic bugs.
pub fn resolve_source_oi_pair(
self,
asg: &Asg,
) -> DynObjectRel<Object<OiPairObjectInner>, T> {
self.map(|(soi, toi)| (soi.resolve(asg).pair_oi(soi), toi))
}
}
fn map(self, f: impl FnOnce(T) -> U) -> Self::Target {
impl DynObjectRel<ObjectIndex<Object>, ObjectIndex<Object>> {
/// Pair the source and target [`ObjectIndex`]es with their respective
/// resolved [`Object`]s.
///
/// See [`Self::resolve_target_oi_pair`] and
/// [`Self::resolve_source_oi_pair`] for more information.
pub fn resolve_oi_pairs(
self,
asg: &Asg,
) -> DynObjectRel<Object<OiPairObjectInner>, Object<OiPairObjectInner>>
{
self.resolve_source_oi_pair(asg).resolve_target_oi_pair(asg)
}
}
impl<S, T, U, V> Functor<(S, T), (U, V)> for DynObjectRel<S, T> {
type Target = DynObjectRel<U, V>;
fn map(self, f: impl FnOnce((S, T)) -> (U, V)) -> Self::Target {
match self {
Self(tys, x, ctx_span) => DynObjectRel(tys, f(x), ctx_span),
}
@ -190,9 +224,9 @@ impl<T, U> Functor<T, U> for DynObjectRel<T> {
impl<T: Display> Display for DynObjectRel<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let Self((from_ty, to_ty), x, _) = self;
let Self((from_ty, to_ty), (s, t), _) = self;
write!(f, "dynamic edge {from_ty}->{to_ty} with {x}",)
write!(f, "dynamic edge {from_ty}->{to_ty} with {s}->{t}",)
}
}

27
tamer/src/asg/graph/visit/test.rs
View File

@ -79,9 +79,10 @@ fn traverses_ontological_tree() {
let sut = tree_reconstruction(&asg);
// We need more concise expressions for the below table of values.
use ObjectRelTy as Ty;
use ObjectRelTy::*;
let d = DynObjectRel::new;
let m = |a: Span, b: Span| a.merge(b).unwrap();
const SU: Span = UNKNOWN_SPAN;
// Note that the `Depth` beings at 1 because the actual root of the
// graph is not emitted.
@ -91,16 +92,20 @@ fn traverses_ontological_tree() {
#[rustfmt::skip]
assert_eq!(
vec![
(d(Ty::Root, Ty::Pkg, m(S1, S11), None ), Depth(1)),
(d(Ty::Pkg, Ty::Ident, S3, None ), Depth(2)),
(d(Ty::Ident, Ty::Expr, m(S2, S7), None ), Depth(3)),
(d(Ty::Expr, Ty::Expr, m(S4, S5), None ), Depth(4)),
(d(Ty::Expr, Ty::Ident, S9, Some(S6)), Depth(4)),
(d(Ty::Pkg, Ty::Ident, S9, None ), Depth(2)),
(d(Ty::Ident, Ty::Expr, m(S8, S10), None ), Depth(3)),
(d(Root, Pkg, SU, m(S1, S11), None ), Depth(1)),
(d(Pkg, Ident, m(S1, S11), S3, None ), Depth(2)),
(d(Ident, Expr, S3, m(S2, S7), None ), Depth(3)),
(d(Expr, Expr, m(S2, S7), m(S4, S5), None ), Depth(4)),
(d(Expr, Ident, m(S2, S7), S9, Some(S6)), Depth(4)),
(d(Pkg, Ident, m(S1, S11), S9, None ), Depth(2)),
(d(Ident, Expr, S9, m(S8, S10), None ), Depth(3)),
],
sut.map(|TreeWalkRel(rel, depth)| {
(rel.map(|oi| oi.resolve(&asg).span()), depth)
}).collect::<Vec<_>>(),
sut.map(|TreeWalkRel(rel, depth)| (
rel.map(|(soi, toi)| (
soi.resolve(&asg).span(),
toi.resolve(&asg).span()
)),
depth
)).collect::<Vec<_>>(),
);
}

24
tamer/src/asg/graph/xmli.rs
View File

@ -37,7 +37,7 @@ use crate::{
visit::{Depth, TreeWalkRel},
Asg, ExprOp,
},
diagnose::{panic::DiagnosticPanic, Annotate},
diagnose::Annotate,
diagnostic_panic, diagnostic_unreachable,
parse::{prelude::*, util::SPair},
span::{Span, UNKNOWN_SPAN},
@ -95,9 +95,11 @@ impl<'a> ParseState for AsgTreeToXirf<'a> {
return Transition(self).incomplete();
}
let pair = dyn_rel.resolve_oi_pair(asg);
// TODO: Verify that the binary does not perform unnecessary
// resolution in branches that do not utilize the source.
let paired_rel = dyn_rel.resolve_oi_pairs(asg);
match pair.target() {
match paired_rel.target() {
Object::Pkg((pkg, _)) => {
let span = pkg.span();
@ -114,24 +116,15 @@ impl<'a> ParseState for AsgTreeToXirf<'a> {
// pass over it for now.
Object::Ident(..) => Transition(self).incomplete(),
Object::Expr((expr, oi)) => match pair.source_ty() {
ObjectRelTy::Ident => {
// We were just told an ident exists,
// so this should not fail.
let ident = oi.ident(asg).diagnostic_unwrap(|| {
vec![expr.internal_error(
"missing ident for this expression",
)]
});
Object::Expr((expr, _)) => match paired_rel.source() {
Object::Ident((ident, _)) => {
toks.push(yields(ident.name(), expr.span()));
Transition(self).ok(stmt(expr, depth))
}
_ => Transition(self).ok(expr_ele(expr, depth)),
},
Object::Root((_, _)) => diagnostic_unreachable!(
Object::Root(..) => diagnostic_unreachable!(
vec![tok_span.error("unexpected Root")],
"tree walk is not expected to emit Root",
),
@ -154,6 +147,7 @@ impl<'a> ParseState for AsgTreeToXirf<'a> {
ObjectRelTy::Root,
ObjectRelTy::Root,
ObjectIndex::new(0.into(), UNKNOWN_SPAN),
ObjectIndex::new(0.into(), UNKNOWN_SPAN),
None,
),
// This is the only part that really matters;