tamer: asg::graph::object::tpl: Infer shape from referenced identifiers

Oh, identifiers.  What a mess you make of the graph.  As indirection tends
to do, I suppose.

This still needs some cleanup; I wanted to get something working first.  In
particular:

  - Common operations need refactoring;
  - Having all this code in the edge definition makes for a mess;
  - `Ident` references really ought not be returned by `Ident::definition`
    since an identifier will never be a definition.

...I'm sure there's more.

DEV-13163

tamer/src/asg/air/tpl/test/apply.rs

@@ -149,18 +149,18 @@ fn tpl_apply_nested_missing() {
 
           // Inner template application (Missing)
           TplStart(S3),
-            RefIdent(ref_tpl_inner_pre),
+            RefIdent(ref_tpl_inner_pre),               // -.
-          TplEndRef(S5),
+          TplEndRef(S5),                               //  |
-
+                                                       //  |
-          // Define the template above
+          // Define the template above                 //  |
-          TplStart(S6),
+          TplStart(S6),                                //  |
-            BindIdent(id_tpl_inner),
+            BindIdent(id_tpl_inner),                   // <:
-          TplEnd(S8),
+          TplEnd(S8),                                  //  |
-
+                                                       //  |
-          // Apply again,
+          // Apply again,                              //  |
-          //   this time _after_ having been defined.
+          //   this time _after_ having been defined.  //  |
-          TplStart(S9),
+          TplStart(S9),                                //  |
-            RefIdent(ref_tpl_inner_post),
+            RefIdent(ref_tpl_inner_post),              // -'
           TplEndRef(S11),
         TplEnd(S12),
     ];
@@ -171,11 +171,17 @@ fn tpl_apply_nested_missing() {
     let oi_tpl_outer = pkg_expect_ident_oi::<Tpl>(&ctx, id_tpl_outer);
     assert_eq!(S1.merge(S12).unwrap(), oi_tpl_outer.resolve(&asg).span());
 
-    // We apply two template,
+    // TODO: It should be the case that we apply two templates,
-    //   both of which are empty,
+    //     both of which are empty,
-    //   and so the outer shape is still empty.
+    //     and so the outer shape is still empty.
+    //   But until we notify templates of `Missing` ident resolution,
+    //     we don't know the shape of the template by the time we reach the
+    //     first reference.
+    //   Consequently,
+    //     `TplShape::Unknown` must take precedence to reflect this
+    //     uncertainty.
     let tpl_outer = oi_tpl_outer.resolve(&asg);
-    assert_eq!(TplShape::Empty, tpl_outer.shape());
+    assert_eq!(TplShape::Unknown(S3.merge(S5).unwrap()), tpl_outer.shape());
 
     // The inner template should be contained within the outer and so not
     //   globally resolvable.
@@ -378,3 +384,146 @@ fn tpl_inner_apply_expr_alongside_another_apply_expr() {
             .collect::<Vec<_>>()
     );
 }
+
+// A simpler version of the above test that asserts against the explicit
+//   case of sibling expressions in a template body,
+//     with no wrapping or indirection.
+#[test]
+fn tpl_multi_expr() {
+    #[rustfmt::skip]
+    let toks = [
+        TplStart(S1),
+          BindIdent(spair("_tpl_", S2)),
+
+          // First one is okay.
+          ExprStart(ExprOp::Sum, S3),
+          ExprEnd(S4),
+
+          // But the second is in error and will be dropped.
+          ExprStart(ExprOp::Sum, S5),
+          ExprEnd(S6),
+        TplEnd(S7),
+    ];
+
+    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), // ExprStart
+                Ok(Parsed::Incomplete), // ExprEnd
+
+                // But this one runs afoul of that new shape.
+                Ok(Parsed::Incomplete), // ExprStart
+                Err(ParseError::StateError(
+                    AsgError::TplShapeExprMulti(
+                        Some(spair("_tpl_", S2)),
+                        S5.merge(S6).unwrap(),
+                        S3.merge(S4).unwrap(),
+                    )
+                )),
+                // RECOVERY: We ignore the expression by not adding its
+                //   edge.
+              Ok(Parsed::Incomplete), // TplEnd
+            Ok(Parsed::Incomplete), // PkgEnd
+        ],
+        sut.by_ref().collect::<Vec<_>>(),
+    );
+
+    let ctx = sut.finalize().unwrap().into_private_context();
+    let asg = ctx.asg_ref();
+
+    let oi_tpl = pkg_expect_ident_oi::<Tpl>(&ctx, spair("_tpl_", S8));
+
+    assert_eq!(
+        TplShape::Expr(S3.merge(S4).unwrap()),
+        oi_tpl.resolve(&asg).shape()
+    );
+
+    // The second Expr should have been omitted.
+    assert_eq!(
+        vec![S3.merge(S4).unwrap()],
+        oi_tpl
+            .edges_filtered::<Expr>(&asg)
+            .map(ObjectIndex::cresolve(&asg))
+            .map(Expr::span)
+            .collect::<Vec<_>>()
+    );
+}
+
+// Identifiers are a layer of indirection,
+//   but the object that they identify should influence the shape of the
+//   template all the same.
+#[test]
+fn tpl_ref_expr_shape() {
+    #[rustfmt::skip]
+    let toks = [
+        TplStart(S1),
+          BindIdent(spair("_tpl-pre_", S2)),
+
+          RefIdent(spair("expr", S3)),                // --,
+        TplEnd(S4),                                   //   |
+                                                      //   | U
+        ExprStart(ExprOp::Sum, S5),                   //   |
+          BindIdent(spair("expr", S6)),               // <=:
+        ExprEnd(S7),                                  //   |
+                                                      //   | E
+        TplStart(S8),                                 //   |
+          BindIdent(spair("_tpl-post_", S9)),         // <-+-.
+                                                      //   | |
+          RefIdent(spair("expr", S10)),               // --' |
+        TplEnd(S11),                                  //     |
+                                                      //     |
+        // Similarly,                                 //     |
+        //   if we have a template that references    //     |
+        //   one of the above templates,              //     |
+        //     it too should have the same            //     | E
+        //     Expr shape,                            //     |
+        //       since the reference represents       //     |
+        //       application.                         //     |
+        TplStart(S12),                                //     |
+          BindIdent(spair("_tpl-post-tpl-ref_", S13)),//     |
+                                                      //     |
+          RefIdent(spair("_tpl-post_", S14)),         // ----'
+        TplEnd(S14),
+    ];
+
+    let ctx = air_ctx_from_pkg_body_toks(toks);
+    let asg = ctx.asg_ref();
+
+    let oi_tpl_pre = pkg_expect_ident_oi::<Tpl>(&ctx, spair("_tpl-pre_", S15));
+    let oi_tpl_post =
+        pkg_expect_ident_oi::<Tpl>(&ctx, spair("_tpl-post_", S16));
+    let oi_tpl_post_tpl_ref =
+        pkg_expect_ident_oi::<Tpl>(&ctx, spair("_tpl-post-tpl-ref_", S17));
+
+    // TODO: We haven't yet handled notification when an identifier has been
+    //   resolved,
+    //     so the shape will be unknown until then.
+    assert_eq!(TplShape::Unknown(S3), oi_tpl_pre.resolve(&asg).shape(),);
+
+    // But the template defined _after_ the expression will be immediately
+    //   resolved and its shape known.
+    assert_eq!(
+        // Note that the span is that of the _reference_,
+        //   since that is what appears within the template body and that
+        //   which we want to emphasize in diagnostics.
+        TplShape::Expr(S10),
+        oi_tpl_post.resolve(&asg).shape(),
+    );
+
+    // And the final template applies the preceding one,
+    //   and should inherit its shape since.
+    assert_eq!(
+        // Just as above,
+        //   the span is the topmost _reference_,
+        //     which is the application.
+        TplShape::Expr(S14),
+        oi_tpl_post_tpl_ref.resolve(&asg).shape(),
+    );
+}

tamer/src/asg/graph.rs

@@ -533,7 +533,6 @@ impl<OA: ObjectRelatable, OB: ObjectRelatable> AsgRelMut<OB> for OA {
         _rel: ProposedRel<Self, OB>,
         commit: impl FnOnce(&mut Asg),
     ) -> Result<(), AsgError> {
-        let _ = _rel.ctx_span; // TODO: remove when used (dead_code)
         Ok(commit(asg))
     }
 }

tamer/src/asg/graph/object/ident.rs

@@ -1291,6 +1291,8 @@ impl ObjectIndex<Ident> {
         &self,
         asg: &Asg,
     ) -> Option<<Ident as ObjectRelatable>::Rel> {
+        // XXX: This could return an abstract binding metavar reference
+        //   depending on undefined edge ordering!
         self.edges(asg).next()
     }
 

tamer/src/asg/graph/object/tpl.rs

@@ -21,8 +21,13 @@
 
 use std::fmt::Display;
 
-use super::{prelude::*, Doc, Expr, Ident};
+use super::{ident::IdentRel, prelude::*, Doc, Expr, Ident};
-use crate::{asg::graph::ProposedRel, f::Map, parse::util::SPair, span::Span};
+use crate::{
+    asg::graph::ProposedRel,
+    f::Map,
+    parse::{prelude::Annotate, util::SPair},
+    span::Span,
+};
 
 /// Template with associated name.
 #[derive(Debug, PartialEq, Eq)]
@@ -123,7 +128,10 @@ pub enum TplShape {
     ///   completed.
     /// Note that a definition is not complete until all missing identifiers
     ///   have been defined.
-    Unknown,
+    ///
+    /// The associated span represents the location that resulted in
+    ///   uncertainty.
+    Unknown(Span),
 
     /// The template can be expanded inline into a single [`Expr`].
     ///
@@ -158,18 +166,35 @@ impl TplShape {
             )),
 
             // Higher levels of specificity take precedence.
-            (shape @ TplShape::Expr(_), TplShape::Empty)
+            // This pattern is designed to be very clear in what shape takes
-            | (TplShape::Empty, shape @ TplShape::Expr(_))
+            //   precedence over another.
-            | (shape @ TplShape::Empty, TplShape::Empty) => Ok(shape),
+            // It should be clear enough that there is no value in writing
-
+            //   unit test against this method since those tests' examples
-            // Unknown is not yet handled.
+            //   would simply reiterate this table
+            //     (but tests for AIR should still be written to test more
+            //        complex interactions).
+            #[rustfmt::skip]
             (
-                TplShape::Unknown,
+                TplShape::Empty,
-                TplShape::Empty | TplShape::Unknown | TplShape::Expr(_),
+                give_precedence_to @ (
+                    TplShape::Empty
+                    | TplShape::Unknown(_)
+                    | TplShape::Expr(_)
+                ),
             )
-            | (TplShape::Empty | TplShape::Expr(_), TplShape::Unknown) => {
+            | (
-                todo!("TplShape::Unknown")
+                TplShape::Unknown(_),
-            }
+                give_precedence_to @ TplShape::Expr(_),
+            )
+            | (
+                give_precedence_to @ TplShape::Unknown(_),
+                TplShape::Empty | TplShape::Unknown(_),
+            )
+            | (
+                give_precedence_to @ TplShape::Expr(_),
+                TplShape::Empty | TplShape::Unknown(_),
+            )
+            => Ok(give_precedence_to),
         }
     }
 
@@ -183,7 +208,8 @@ impl TplShape {
     ///   its own body.
     fn overwrite_span_if_any(self, span: Span) -> Self {
         match self {
-            TplShape::Empty | TplShape::Unknown => self,
+            TplShape::Empty => self,
+            TplShape::Unknown(_) => TplShape::Unknown(span),
             TplShape::Expr(_) => TplShape::Expr(span),
         }
     }
@@ -204,8 +230,8 @@ impl Display for TplShape {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         // phrase as "template with ..."
         match self {
-            TplShape::Unknown => write!(f, "unknown shape"),
             TplShape::Empty => write!(f, "empty shape"),
+            TplShape::Unknown(_) => write!(f, "unknown shape"),
             TplShape::Expr(_) => write!(f, "shape of a single expression"),
         }
     }
@@ -237,7 +263,78 @@ object_rel! {
 
         // Identifiers are used for both references and identifiers that
         //   will expand into an application site.
-        dyn Ident,
+        dyn Ident {
+            fn pre_add_edge(
+                asg: &mut Asg,
+                rel: ProposedRel<Self, Ident>,
+                commit: impl FnOnce(&mut Asg),
+            ) -> Result<(), AsgError> {
+                let tpl_name = rel.from_oi.name(asg);
+
+                match (rel.ctx_span, rel.to_oi.definition(asg)) {
+                    // Missing definition results in shape uncertainty that
+                    //   will have to be resolved when (if) a definition
+                    //   becomes available.
+                    (Some(ref_span), None) => {
+                        rel.from_oi.try_map_obj_inner(
+                            asg,
+                            try_adapt_to(TplShape::Unknown(ref_span), tpl_name),
+                        )?;
+                    }
+
+                    // TAME is referentally transparent,
+                    //   so a reference to an Expr is no different than
+                    //   inlining that Expr.
+                    (Some(ref_span), Some(IdentRel::Expr(_))) => {
+                        rel.from_oi.try_map_obj_inner(
+                            asg,
+                            try_adapt_to(TplShape::Expr(ref_span), tpl_name),
+                        )?;
+                    }
+
+                    // This is the same as the `Tpl` tree edge below,
+                    //   but a named template instead of an anonymous one.
+                    (Some(ref_span), Some(IdentRel::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),
+                        )?;
+                    }
+
+                    // TODO: Filter this out (Ident -> Ident)
+                    (Some(span), Some(IdentRel::Ident(_))) => {
+                        diagnostic_todo!(
+                            vec![span.internal_error("while parsing this reference")],
+                            "opaque identifier or abstract binding"
+                        )
+                    }
+
+                    // The mere _existence_ of metavariables (template
+                    //   params) do not influence the expansion shape.
+                    (Some(_), Some(IdentRel::Meta(_))) => (),
+
+                    // Lack of span means that this is not a cross edge,
+                    //   and so not a reference;
+                    //     this means that the object is identified and will
+                    //     be hoisted into the rooting context of the
+                    //     application site,
+                    //       which does not impact template shape.
+                    // TODO: Let's make that span assumption explicit in the
+                    //   `ProposeRel` abstraction.
+                    (None, _) => (),
+                }
+
+                Ok(commit(asg))
+            }
+        },
 
         // Template application.
         tree Tpl {