From e14854a555414d6e948a30eed99f09efaa1789cd Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 26 Jul 2023 16:09:17 -0400 Subject: [PATCH] tamer: asg::air::tpl: Resolve shape against inner template application Things are starting to get interesting, and this shows how caching information about template shape (rather than having to query the graph any time we want to discover it) makes it easy to compose shapes. This does not yet handle the unknown case. Before I do that, I'll want to do some refactoring to address duplication in the `tpl` module. DEV-13163 --- tamer/src/asg/air/tpl.rs | 1 + tamer/src/asg/air/tpl/test.rs | 168 ++++++++++++++++++++++++++++++ tamer/src/asg/graph/object/tpl.rs | 97 ++++++++++++++--- 3 files changed, 252 insertions(+), 14 deletions(-) diff --git a/tamer/src/asg/air/tpl.rs b/tamer/src/asg/air/tpl.rs index 5bcab3e4..9e5f0931 100644 --- a/tamer/src/asg/air/tpl.rs +++ b/tamer/src/asg/air/tpl.rs @@ -229,6 +229,7 @@ impl ParseState for AirTplAggregate { match ctx.expansion_oi() { Some(oi_target) => tpl .oi() + .close(ctx.asg_mut(), span) .expand_into(ctx.asg_mut(), oi_target) .map(|_| ()) .transition(Toplevel(tpl.anonymous_reachable())), diff --git a/tamer/src/asg/air/tpl/test.rs b/tamer/src/asg/air/tpl/test.rs index ad997c9b..4af29492 100644 --- a/tamer/src/asg/air/tpl/test.rs +++ b/tamer/src/asg/air/tpl/test.rs @@ -802,6 +802,174 @@ fn tpl_apply_nested_missing() { ); } +#[test] +fn tpl_inner_apply_inherit_shape() { + let id_tpl_outer = spair("_tpl-outer_", S2); + + #[rustfmt::skip] + let toks = [ + TplStart(S1), + BindIdent(id_tpl_outer), + + // Inner template application has an Expr shape. + TplStart(S3), + ExprStart(ExprOp::Sum, S4), + ExprEnd(S5), + TplEndRef(S6), + TplEnd(S7), + ]; + + let ctx = air_ctx_from_pkg_body_toks(toks); + let asg = ctx.asg_ref(); + + let oi_tpl_outer = pkg_expect_ident_oi::(&ctx, id_tpl_outer); + let oi_tpl_inner = oi_tpl_outer.edges_filtered::(&asg).next().unwrap(); + + // The inner template's expression should determine its shape. + assert_eq!( + TplShape::Expr(S4.merge(S5).unwrap()), + oi_tpl_inner.resolve(&asg).shape() + ); + + // Since the outer template applies the inner, + // it inherits the inner's shape. + // (Imagine pasting the body of the inner template inline.) + assert_eq!( + TplShape::Expr(S3.merge(S6).unwrap()), + oi_tpl_outer.resolve(&asg).shape() + ); +} + +// It's okay to have sibling template applications that aren't Exprs. +#[test] +fn tpl_inner_apply_expr_alongside_empty() { + let id_tpl_outer = spair("_tpl-outer_", S2); + + #[rustfmt::skip] + let toks = [ + TplStart(S1), + BindIdent(id_tpl_outer), + + // Inner template application has an Expr shape. + TplStart(S3), + ExprStart(ExprOp::Sum, S4), + ExprEnd(S5), + TplEndRef(S6), + + // But this is empty. + TplStart(S7), + TplEndRef(S8), + TplEnd(S9), + ]; + + let ctx = air_ctx_from_pkg_body_toks(toks); + let asg = ctx.asg_ref(); + + let oi_tpl_outer = pkg_expect_ident_oi::(&ctx, id_tpl_outer); + + // The Expr shape takes precedence. + assert_eq!( + TplShape::Expr(S3.merge(S6).unwrap()), + oi_tpl_outer.resolve(&asg).shape() + ); +} + +// Template applications with Expr shapes yield the same errors as if the +// bodies were just pasted inline, +// with the exception of the spans that are reported. +// The spans we provide to the user should be in the context of the template +// being defined; +// if we inherit the shape from the template, +// then the span would be for the expression _inside_ that template, +// which would compose to the innermost application's expression, +// recursively. +// And while that would certainly make for an impressive display if you +// understood what was going on, +// that breaks every layer of abstraction that was built and is not what +// we want to provide. +#[test] +fn tpl_inner_apply_expr_alongside_another_apply_expr() { + let id_tpl_outer = spair("_tpl-outer_", S2); + + #[rustfmt::skip] + let toks = [ + TplStart(S1), + BindIdent(id_tpl_outer), + + // Inner template application has an Expr shape. + TplStart(S3), + ExprStart(ExprOp::Sum, S4), + ExprEnd(S5), + TplEndRef(S6), + + // As does this one, + // which should produce an error. + TplStart(S7), + ExprStart(ExprOp::Sum, S8), + ExprEnd(S9), + TplEndRef(S10), + TplEnd(S11), + ]; + + let mut sut = Sut::parse(as_pkg_body(toks)); + + assert_eq!( + #[rustfmt::skip] + vec![ + Ok(Parsed::Incomplete), // PkgStart + Ok(Parsed::Incomplete), // TplStart + Ok(Parsed::Incomplete), // BindIdent + + // This one's okay and changes the template's shape. + Ok(Parsed::Incomplete), // TplStart + Ok(Parsed::Incomplete), // ExprStart + Ok(Parsed::Incomplete), // ExprEnd + Ok(Parsed::Incomplete), // TplEnd + + // But this one runs afoul of that new shape. + Ok(Parsed::Incomplete), // TplStart + Ok(Parsed::Incomplete), // ExprStart + Ok(Parsed::Incomplete), // ExprEnd + Err(ParseError::StateError( + // These spans are relative to the application itself. + // This is important to present the appropriate level of + // abstraction; + // see the comment for this test case. + AsgError::TplShapeExprMulti( + Some(id_tpl_outer), + S7.merge(S10).unwrap(), + S3.merge(S6).unwrap() + ) + )), + // RECOVERY: We ignore the template by not adding the edge. + Ok(Parsed::Incomplete), // TplEnd >LA + Ok(Parsed::Incomplete), // TplEnd >(), + ); + + let ctx = sut.finalize().unwrap().into_private_context(); + let asg = ctx.asg_ref(); + + let oi_tpl_outer = pkg_expect_ident_oi::(&ctx, id_tpl_outer); + + assert_eq!( + TplShape::Expr(S3.merge(S6).unwrap()), + oi_tpl_outer.resolve(&asg).shape() + ); + + // The second template application should have been omitted. + assert_eq!( + vec![S3.merge(S6).unwrap()], + oi_tpl_outer + .edges_filtered::(&asg) + .map(ObjectIndex::cresolve(&asg)) + .map(Tpl::span) + .collect::>() + ); +} + #[test] fn tpl_doc_short_desc() { let id_tpl = spair("foo", S2); diff --git a/tamer/src/asg/graph/object/tpl.rs b/tamer/src/asg/graph/object/tpl.rs index feecee87..1e631ee5 100644 --- a/tamer/src/asg/graph/object/tpl.rs +++ b/tamer/src/asg/graph/object/tpl.rs @@ -150,6 +150,41 @@ pub enum TplShape { Expr(Span), } +impl TplShape { + fn try_adapt_to( + self, + other: TplShape, + tpl_name: Option, + ) -> Result { + match (self, other) { + (TplShape::Expr(first_span), TplShape::Expr(bad_span)) => { + Err(AsgError::TplShapeExprMulti(tpl_name, bad_span, first_span)) + } + + // Higher levels of specificity take precedence. + (shape @ TplShape::Expr(_), TplShape::Empty) + | (TplShape::Empty, shape @ TplShape::Expr(_)) + | (shape @ TplShape::Empty, TplShape::Empty) => Ok(shape), + + // Unknown is not yet handled. + ( + TplShape::Unknown, + TplShape::Empty | TplShape::Unknown | TplShape::Expr(_), + ) + | (TplShape::Empty | TplShape::Expr(_), TplShape::Unknown) => { + todo!("TplShape::Unknown") + } + } + } + + fn overwrite_span_if_any(self, span: Span) -> Self { + match self { + TplShape::Empty | TplShape::Unknown => self, + TplShape::Expr(_) => TplShape::Expr(span), + } + } +} + impl Display for TplShape { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { // phrase as "template with ..." @@ -175,22 +210,18 @@ object_rel! { _ctx_span: Option, commit: impl FnOnce(&mut Asg), ) -> Result<(), AsgError> { + let tpl_name = from_oi.name(asg); let span = to_oi.resolve(asg).span(); - let tpl_name = from_oi - .ident(asg) - .and_then(|oi| oi.resolve(asg).name()); - from_oi.try_map_obj(asg, |tpl| match tpl.shape() { - TplShape::Expr(first) => { - Err(( - tpl, - AsgError::TplShapeExprMulti(tpl_name, span, first) - )) - }, + from_oi.try_map_obj(asg, |tpl| { + let new = tpl + .shape() + .try_adapt_to(TplShape::Expr(span), tpl_name); - TplShape::Unknown | TplShape::Empty => { - Ok(tpl.overwrite(TplShape::Expr(span))) - }, + match new { + Ok(shape) => Ok(tpl.overwrite(shape)), + Err(e) => Err((tpl, e)), + } })?; Ok(commit(asg)) @@ -202,7 +233,35 @@ object_rel! { dyn Ident, // Template application. - tree Tpl, + tree Tpl { + fn pre_add_edge( + asg: &mut Asg, + from_oi: ObjectIndex, + to_oi: ObjectIndex, + _ctx_span: Option, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError> { + let tpl_name = from_oi.name(asg); + let apply = to_oi.resolve(asg); + let apply_shape = apply + .shape() + .overwrite_span_if_any(apply.span()); + + // TODO: Refactor; very similar to Expr edge above. + from_oi.try_map_obj(asg, |tpl| { + let new = tpl + .shape() + .try_adapt_to(apply_shape, tpl_name); + + match new { + Ok(shape) => Ok(tpl.overwrite(shape)), + Err(e) => Err((tpl, e)), + } + })?; + + Ok(commit(asg)) + } + }, // Short template description and arbitrary documentation to be // expanded into the application site. @@ -211,6 +270,16 @@ object_rel! { } impl ObjectIndex { + /// Name of template, + /// if any. + /// + /// A template may either be anonymous, + /// or it may not yet have a name because it is still under + /// construction. + pub fn name(&self, asg: &Asg) -> Option { + self.ident(asg).and_then(|oi| oi.resolve(asg).name()) + } + /// Attempt to complete a template definition. /// /// This updates the span of the template to encompass the entire