From 62884b2e6818cea73a66fda3ca984b7c40831945 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 31 Jul 2023 15:13:14 -0400 Subject: [PATCH] tamer: asg::graph::object::tpl: Refactor common application shape inference This also hoists `?` out of the inner match arms to make control flow easier to follow. I try to balance convenience with brutal explicitness, to make control flow more obvious and make it easier to reason about what the system is doing at every point in the program. DEV-13163 --- tamer/src/asg/graph/object/tpl.rs | 64 +++++++++++++++++-------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/tamer/src/asg/graph/object/tpl.rs b/tamer/src/asg/graph/object/tpl.rs index 559aa9b6..78f34580 100644 --- a/tamer/src/asg/graph/object/tpl.rs +++ b/tamer/src/asg/graph/object/tpl.rs @@ -274,7 +274,7 @@ object_rel! { rel.from_oi.try_map_obj_inner( asg, try_adapt_to(TplShape::Unknown(ref_span), tpl_name), - )?; + ).map(|_| ()) } // TAME is referentally transparent, @@ -284,29 +284,18 @@ object_rel! { rel.from_oi.try_map_obj_inner( asg, try_adapt_to(TplShape::Expr(ref_span), tpl_name), - )?; + ).map(|_| ()) } // This is the same as the `Tpl` tree edge below, // but a named template instead of an anonymous one. (Some(ref_span), Some(IdentDefinition::Tpl(to_oi))) => { - // TODO: Factor common logic between this and the - // `Tpl->Tpl` edge below. - let tpl_name = to_oi.name(asg); - let apply = to_oi.resolve(asg); - let apply_shape = apply - .shape() - .overwrite_span_if_any(ref_span); - - rel.from_oi.try_map_obj_inner( - asg, - try_adapt_to(apply_shape, tpl_name), - )?; + apply_tpl_shape(asg, rel.from_oi, to_oi, Some(ref_span)) } // The mere _existence_ of metavariables (template // params) do not influence the expansion shape. - (Some(_), Some(IdentDefinition::Meta(_))) => (), + (Some(_), Some(IdentDefinition::Meta(_))) => Ok(()), // Lack of span means that this is not a cross edge, // and so not a reference; @@ -316,8 +305,8 @@ object_rel! { // which does not impact template shape. // TODO: Let's make that span assumption explicit in the // `ProposeRel` abstraction. - (None, _) => (), - } + (None, _) => Ok(()), + }?; Ok(commit(asg)) } @@ -330,17 +319,7 @@ object_rel! { rel: ProposedRel, commit: impl FnOnce(&mut Asg), ) -> Result<(), AsgError> { - let tpl_name = rel.from_oi.name(asg); - let apply = rel.to_oi.resolve(asg); - let apply_shape = apply - .shape() - .overwrite_span_if_any(apply.span()); - - rel.from_oi.try_map_obj_inner( - asg, - try_adapt_to(apply_shape, tpl_name), - )?; - + apply_tpl_shape(asg, rel.from_oi, rel.to_oi, None)?; Ok(commit(asg)) } }, @@ -351,6 +330,35 @@ object_rel! { } } +/// Apply the shape of a template to a parent template. +/// +/// Phrased another way: +/// given a template application `oi_apply` in the body of a parent +/// template `oi_parent`, +/// infer the appropriate shape forĀ `oi_parent`. +/// +/// If `oi_apply` is behind a reference, +/// `ref_span` may be used to override the diagnostic span stored within +/// the shape. +/// Since this is used to report shape errors to the user, +/// this span should _always_ be within the context of `oi_parent`. +fn apply_tpl_shape( + asg: &mut Asg, + oi_parent: ObjectIndex, + oi_apply: ObjectIndex, + ref_span: Option, +) -> Result<(), AsgError> { + let tpl_name = oi_parent.name(asg); + let apply = oi_apply.resolve(asg); + let apply_shape = apply + .shape() + .overwrite_span_if_any(ref_span.unwrap_or(apply.span())); + + oi_parent.try_map_obj_inner(asg, try_adapt_to(apply_shape, tpl_name))?; + + Ok(()) +} + impl ObjectIndex { /// Name of template, /// if any.