tamer::asg::graph::object::ident::Ident::name: Produce Option

This prepares to make the name of an `Ident` optional to support abstract
identifiers derived from metavariables.

This is an unfortunate change to have to prepare for, since it complicates
how Idents are interpreted, but the alternative (a new object type) is not
good either.  We'll see how this evolves.

DEV-13163

tamer/src/asg/graph/object.rs

@@ -810,13 +810,11 @@ impl<O: ObjectKind> ObjectIndex<O> {
     ///   on the graph,
     ///     like common subexpression elimination,
     ///   in which case it's best not to rely on following edges in reverse.
-    pub fn ident<'a>(&self, asg: &'a Asg) -> Option<&'a Ident>
+    pub fn ident<'a>(&self, asg: &'a Asg) -> Option<ObjectIndex<Ident>>
     where
         O: ObjectRelFrom<Ident>,
     {
-        self.incoming_edges_filtered(asg)
+        self.incoming_edges_filtered(asg).next()
-            .map(ObjectIndex::cresolve(asg))
-            .next()
     }
 
     /// Describe this expression using a short independent clause.

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

@@ -170,14 +170,18 @@ impl Display for Ident {
 }
 
 impl Ident {
-    /// Identifier name.
+    /// Concrete identifier name.
-    pub fn name(&self) -> SPair {
+    ///
+    /// Note: This [`Option`] is in preparation for identifiers that may not
+    ///   yet have names,
+    ///     awaiting expansion of a metavariable.
+    pub fn name(&self) -> Option<SPair> {
         match self {
             Missing(name)
             | Opaque(name, ..)
             | Extern(name, ..)
             | IdentFragment(name, ..)
-            | Transparent(name) => *name,
+            | Transparent(name) => Some(*name),
         }
     }
 
@@ -357,10 +361,11 @@ impl Ident {
 
             Missing(name) => Ok(Opaque(name.overwrite(span), kind, src)),
 
-            // TODO: Remove guards and catch-all for exhaustiveness check.
+            // TODO: Remove guards for better exhaustiveness check
-            _ => {
+            Opaque(name, _, _)
-                let err = TransitionError::Redeclare(self.name(), span);
+            | IdentFragment(name, _, _, _)
-
+            | Transparent(name) => {
+                let err = TransitionError::Redeclare(name, span);
                 Err((self, err))
             }
         }
@@ -381,7 +386,7 @@ impl Ident {
     /// At present,
     ///   both [`Ident::Missing`] and [`Ident::Extern`] are
     ///   considered to be unresolved.
-    pub fn resolved(&self) -> Result<&Ident, UnresolvedError> {
+    pub fn resolved(&self) -> Result<(&Ident, SPair), UnresolvedError> {
         match self {
             Missing(name) => Err(UnresolvedError::Missing(*name)),
 
@@ -389,7 +394,9 @@ impl Ident {
                 Err(UnresolvedError::Extern(*name, kind.clone()))
             }
 
-            Opaque(..) | IdentFragment(..) | Transparent(..) => Ok(self),
+            Opaque(name, ..)
+            | IdentFragment(name, ..)
+            | Transparent(name, ..) => Ok((self, *name)),
         }
     }
 
@@ -414,21 +421,26 @@ impl Ident {
         kind: IdentKind,
         src: Source,
     ) -> TransitionResult<Ident> {
-        match self.kind() {
+        match self {
-            None => Ok(Extern(self.name().overwrite(span), kind, src)),
+            Missing(name) | Transparent(name) => {
-            Some(cur_kind) => {
+                Ok(Extern(name.overwrite(span), kind, src))
+            }
+
+            Opaque(name, ref cur_kind, _)
+            | Extern(name, ref cur_kind, _)
+            | IdentFragment(name, ref cur_kind, _, _) => {
                 if cur_kind != &kind {
                     let err = TransitionError::ExternResolution(
-                        self.name(),
+                        name,
                         cur_kind.clone(),
                         (kind, span),
                     );
 
-                    return Err((self, err));
+                    Err((self, err))
+                } else {
+                    // Resolved successfully, so keep what we already have.
+                    Ok(self)
                 }
-
-                // Resolved successfully, so keep what we already have.
-                Ok(self)
             }
         }
     }
@@ -457,6 +469,9 @@ impl Ident {
             IdentFragment(_, _, ref src, ..) if src.override_ => Ok(self),
 
             // These represent the prologue and epilogue of maps.
+            //
+            // TODO: Is this arm still needed after having eliminated their
+            //   fragments from xmlo files?
             IdentFragment(
                 _,
                 IdentKind::MapHead
@@ -466,8 +481,10 @@ impl Ident {
                 ..,
             ) => Ok(self),
 
-            _ => {
+            Missing(name)
-                let name = self.name();
+            | Extern(name, _, _)
+            | IdentFragment(name, _, _, _)
+            | Transparent(name) => {
                 Err((self, TransitionError::BadFragmentDest(name)))
             }
         }
@@ -1167,6 +1184,19 @@ impl ObjectIndex<Ident> {
     ) -> Self {
         self.add_edge_to(asg, oi_dep, None)
     }
+
+    /// Retrieve either the concrete name of the identifier or the name of
+    ///   the metavariable that will be used to produce it.
+    pub fn name_or_meta(&self, asg: &Asg) -> SPair {
+        let ident = self.resolve(asg);
+
+        ident.name().unwrap_or_else(|| {
+            diagnostic_todo!(
+                vec![ident.span().internal_error("for this abstract ident")],
+                "metavariable lookup not yet supported",
+            )
+        })
+    }
 }
 
 #[cfg(test)]

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

@@ -30,20 +30,20 @@ fn ident_name() {
     let name = "name".into();
     let spair = SPair(name, S1);
 
-    assert_eq!(spair, Ident::Missing(spair).name());
+    assert_eq!(Some(spair), Ident::Missing(spair).name());
 
     assert_eq!(
-        spair,
+        Some(spair),
         Ident::Opaque(spair, IdentKind::Meta, Source::default()).name()
     );
 
     assert_eq!(
-        spair,
+        Some(spair),
         Ident::Extern(spair, IdentKind::Meta, Source::default()).name()
     );
 
     assert_eq!(
-        spair,
+        Some(spair),
         Ident::IdentFragment(
             spair,
             IdentKind::Meta,
@@ -182,7 +182,10 @@ fn resolved_on_ident() {
             .unwrap()
             .resolved()
             .unwrap(),
-        &Ident::Opaque(SPair(sym, S2), kind.clone(), src.clone()),
+        (
+            &Ident::Opaque(SPair(sym, S2), kind.clone(), src.clone()),
+            SPair(sym, S2)
+        ),
     );
 }
 
@@ -403,7 +406,10 @@ fn resolved_on_fragment() {
 
     assert_eq!(
         ident.set_fragment(text.clone()).unwrap().resolved(),
-        Ok(&Ident::IdentFragment(SPair(sym, S2), kind, src, text)),
+        Ok((
+            &Ident::IdentFragment(SPair(sym, S2), kind, src, text),
+            SPair(sym, S2),
+        )),
     );
 }
 

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

@@ -875,8 +875,10 @@ pub trait ObjectIndexRelTo<OB: ObjectRelatable>: Sized + Clone + Copy {
         Self: ObjectIndexRelTo<Ident>,
     {
         // Rust fails to infer OB with `self.edges_rel_to` as of 2023-03
-        ObjectIndexRelTo::<Ident>::edges_rel_to(self, asg)
+        ObjectIndexRelTo::<Ident>::edges_rel_to(self, asg).find(|oi| {
-            .find(|oi| oi.resolve(asg).name().symbol() == name.symbol())
+            oi.resolve(asg).name().map(|name| name.symbol())
+                == Some(name.symbol())
+        })
     }
 }
 

tamer/src/asg/graph/xmli.rs

@@ -282,8 +282,8 @@ impl<'a> TreeContext<'a> {
         depth: Depth,
     ) -> Option<Xirf> {
         match src {
-            Object::Ident((ident, _)) => {
+            Object::Ident((ident, oi_ident)) => {
-                self.emit_expr_ident(expr, ident, depth)
+                self.emit_expr_ident(expr, *oi_ident, ident, depth)
             }
             Object::Expr((pexpr, _)) => match (pexpr.op(), expr.op()) {
                 (ExprOp::Conj | ExprOp::Disj, ExprOp::Eq) => {
@@ -315,6 +315,7 @@ impl<'a> TreeContext<'a> {
     fn emit_expr_ident(
         &mut self,
         expr: &Expr,
+        oi_ident: ObjectIndex<Ident>,
         ident: &Ident,
         depth: Depth,
     ) -> Option<Xirf> {
@@ -331,8 +332,9 @@ impl<'a> TreeContext<'a> {
             }
         };
 
+        let name = oi_ident.name_or_meta(self.asg);
         let ispan = ident.span();
-        self.push(Xirf::attr(ident_qname, ident.name(), (ispan, ispan)));
+        self.push(Xirf::attr(ident_qname, name, (ispan, ispan)));
 
         Some(Xirf::open(
             qname,
@@ -353,21 +355,15 @@ impl<'a> TreeContext<'a> {
         let mut edges = oi_expr.edges_filtered::<Ident>(self.asg);
 
         // note: the edges are reversed (TODO?)
-        let value = edges
+        let value = edges.next().diagnostic_expect(
-            .next()
+            || vec![oi_expr.note("for this match")],
-            .diagnostic_expect(
+            "missing @value ref",
-                || vec![oi_expr.note("for this match")],
+        );
-                "missing @value ref",
-            )
-            .resolve(self.asg);
 
-        let on = edges
+        let on = edges.next().diagnostic_expect(
-            .next()
+            || vec![oi_expr.note("for this match")],
-            .diagnostic_expect(
+            "missing @on ref",
-                || vec![oi_expr.note("for this match")],
+        );
-                "missing @on ref",
-            )
-            .resolve(self.asg);
 
         if let Some(unexpected) = edges.next() {
             diagnostic_panic!(
@@ -376,8 +372,8 @@ impl<'a> TreeContext<'a> {
             );
         }
 
-        self.push(attr_value(value.name()));
+        self.push(attr_value(value.name_or_meta(self.asg)));
-        self.push(attr_on(on.name()));
+        self.push(attr_on(on.name_or_meta(self.asg)));
 
         Xirf::open(QN_MATCH, OpenSpan::without_name_span(expr.span()), depth)
     }
@@ -391,9 +387,9 @@ impl<'a> TreeContext<'a> {
         depth: Depth,
     ) -> Option<Xirf> {
         match src {
-            Object::Ident((ident, _)) => {
+            Object::Ident((_, oi_ident)) => {
                 self.tpl_apply = None;
-                self.push(attr_name(ident.name()));
+                self.push(attr_name(oi_ident.name_or_meta(self.asg)));
 
                 Some(Xirf::open(
                     QN_TEMPLATE,
@@ -438,7 +434,7 @@ impl<'a> TreeContext<'a> {
                     "cannot derive name of template for application",
                 );
 
-                self.push(attr_name(apply_tpl.resolve(self.asg).name()));
+                self.push(attr_name(apply_tpl.name_or_meta(self.asg)));
 
                 Some(Xirf::open(
                     QN_APPLY_TEMPLATE,
@@ -466,13 +462,12 @@ impl<'a> TreeContext<'a> {
         oi_meta: ObjectIndex<Meta>,
         depth: Depth,
     ) -> Option<Xirf> {
-        let pname =
+        let pname = oi_meta
-            oi_meta
+            .ident(self.asg)
-                .ident(self.asg)
+            .map(|oi| oi.name_or_meta(self.asg))
-                .map(Ident::name)
+            .diagnostic_unwrap(|| {
-                .diagnostic_unwrap(|| {
+                vec![meta.internal_error("missing param name")]
-                    vec![meta.internal_error("missing param name")]
+            });
-                });
 
         self.push(attr_name(pname));
 
@@ -493,7 +488,7 @@ impl<'a> TreeContext<'a> {
         oi_meta: ObjectIndex<Meta>,
         depth: Depth,
     ) -> Option<Xirf> {
-        let pname = oi_meta.ident(self.asg).map(Ident::name)
+        let pname = oi_meta.ident(self.asg).map(|oi| oi.name_or_meta(self.asg))
             .diagnostic_unwrap(|| vec![meta.internal_error(
                 "anonymous metavariables are not supported as template arguments"
             )]);

tamer/src/ld/xmle/lower/test.rs

@@ -105,12 +105,13 @@ fn graph_sort() -> SortResult<()> {
     let asg = asg_from_toks(toks);
     let sections = sort(&asg, StubSections { pushed: Vec::new() })?;
 
-    let expected = vec![
+    let expected = [
         // Post-order
         name_a_dep_dep,
         name_a_dep,
         name_a,
     ]
+    .map(Some)
     .into_iter()
     .collect::<Vec<_>>();
 

tamer/src/ld/xmle/section.rs

@@ -113,10 +113,10 @@ impl<'a> XmleSections<'a> for Sections<'a> {
     fn push(&mut self, ident: &'a Ident) -> PushResult {
         self.deps.push(ident);
 
-        let name = ident.name();
         let frag = ident.fragment();
+        let (resolved, name) = ident.resolved()?;
 
-        match ident.resolved()?.kind() {
+        match resolved.kind() {
             Some(kind) => match kind {
                 IdentKind::Cgen(..)
                 | IdentKind::Gen(..)
@@ -156,7 +156,7 @@ impl<'a> XmleSections<'a> for Sections<'a> {
                 // compiler bug and there is no use in trying to be nice
                 // about a situation where something went terribly, horribly
                 // wrong.
-                return Err(SectionsError::MissingObjectKind(ident.name()));
+                return Err(SectionsError::MissingObjectKind(name));
             }
         }
 

tamer/src/ld/xmle/xir/test.rs

@@ -256,7 +256,7 @@ fn test_writes_deps() -> TestResult {
 
         assert_eq!(
             attrs.find(QN_NAME).map(|a| a.value()),
-            Some(ident.name().symbol()),
+            ident.name().map(|name| name.symbol()),
         );
 
         assert_eq!(