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