From 828d8918a3c79d23d2df6343e2ae0222b86f515a Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Mon, 26 Jun 2023 15:00:51 -0400 Subject: [PATCH] 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 | 6 +-- tamer/src/asg/graph/object/ident.rs | 68 +++++++++++++++++------- tamer/src/asg/graph/object/ident/test.rs | 18 ++++--- tamer/src/asg/graph/object/rel.rs | 6 ++- tamer/src/asg/graph/xmli.rs | 55 +++++++++---------- tamer/src/ld/xmle/lower/test.rs | 3 +- tamer/src/ld/xmle/section.rs | 6 +-- tamer/src/ld/xmle/xir/test.rs | 2 +- 8 files changed, 98 insertions(+), 66 deletions(-) diff --git a/tamer/src/asg/graph/object.rs b/tamer/src/asg/graph/object.rs index 011f443d..475a4031 100644 --- a/tamer/src/asg/graph/object.rs +++ b/tamer/src/asg/graph/object.rs @@ -810,13 +810,11 @@ impl ObjectIndex { /// 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> where O: ObjectRelFrom, { - self.incoming_edges_filtered(asg) - .map(ObjectIndex::cresolve(asg)) - .next() + self.incoming_edges_filtered(asg).next() } /// Describe this expression using a short independent clause. diff --git a/tamer/src/asg/graph/object/ident.rs b/tamer/src/asg/graph/object/ident.rs index 5bc73af4..65494548 100644 --- a/tamer/src/asg/graph/object/ident.rs +++ b/tamer/src/asg/graph/object/ident.rs @@ -170,14 +170,18 @@ impl Display for Ident { } impl Ident { - /// Identifier name. - pub fn name(&self) -> SPair { + /// Concrete identifier name. + /// + /// Note: This [`Option`] is in preparation for identifiers that may not + /// yet have names, + /// awaiting expansion of a metavariable. + pub fn name(&self) -> Option { 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. - _ => { - let err = TransitionError::Redeclare(self.name(), span); - + // TODO: Remove guards for better exhaustiveness check + Opaque(name, _, _) + | 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 { - match self.kind() { - None => Ok(Extern(self.name().overwrite(span), kind, src)), - Some(cur_kind) => { + match self { + Missing(name) | Transparent(name) => { + 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), - _ => { - let name = self.name(); + Missing(name) + | Extern(name, _, _) + | IdentFragment(name, _, _, _) + | Transparent(name) => { Err((self, TransitionError::BadFragmentDest(name))) } } @@ -1167,6 +1184,19 @@ impl ObjectIndex { ) -> 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)] diff --git a/tamer/src/asg/graph/object/ident/test.rs b/tamer/src/asg/graph/object/ident/test.rs index dc5c1ed6..46252a3a 100644 --- a/tamer/src/asg/graph/object/ident/test.rs +++ b/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), + )), ); } diff --git a/tamer/src/asg/graph/object/rel.rs b/tamer/src/asg/graph/object/rel.rs index 6a68fdf5..b6b2062f 100644 --- a/tamer/src/asg/graph/object/rel.rs +++ b/tamer/src/asg/graph/object/rel.rs @@ -875,8 +875,10 @@ pub trait ObjectIndexRelTo: Sized + Clone + Copy { Self: ObjectIndexRelTo, { // Rust fails to infer OB with `self.edges_rel_to` as of 2023-03 - ObjectIndexRelTo::::edges_rel_to(self, asg) - .find(|oi| oi.resolve(asg).name().symbol() == name.symbol()) + ObjectIndexRelTo::::edges_rel_to(self, asg).find(|oi| { + oi.resolve(asg).name().map(|name| name.symbol()) + == Some(name.symbol()) + }) } } diff --git a/tamer/src/asg/graph/xmli.rs b/tamer/src/asg/graph/xmli.rs index c4b05005..9cc3d105 100644 --- a/tamer/src/asg/graph/xmli.rs +++ b/tamer/src/asg/graph/xmli.rs @@ -282,8 +282,8 @@ impl<'a> TreeContext<'a> { depth: Depth, ) -> Option { match src { - Object::Ident((ident, _)) => { - self.emit_expr_ident(expr, ident, depth) + Object::Ident((ident, oi_ident)) => { + 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, depth: Depth, ) -> Option { @@ -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::(self.asg); // note: the edges are reversed (TODO?) - let value = edges - .next() - .diagnostic_expect( - || vec![oi_expr.note("for this match")], - "missing @value ref", - ) - .resolve(self.asg); + let value = edges.next().diagnostic_expect( + || vec![oi_expr.note("for this match")], + "missing @value ref", + ); - let on = edges - .next() - .diagnostic_expect( - || vec![oi_expr.note("for this match")], - "missing @on ref", - ) - .resolve(self.asg); + let on = edges.next().diagnostic_expect( + || vec![oi_expr.note("for this match")], + "missing @on ref", + ); 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_on(on.name())); + self.push(attr_value(value.name_or_meta(self.asg))); + 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 { 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, depth: Depth, ) -> Option { - let pname = - oi_meta - .ident(self.asg) - .map(Ident::name) - .diagnostic_unwrap(|| { - vec![meta.internal_error("missing param name")] - }); + let pname = oi_meta + .ident(self.asg) + .map(|oi| oi.name_or_meta(self.asg)) + .diagnostic_unwrap(|| { + vec![meta.internal_error("missing param name")] + }); self.push(attr_name(pname)); @@ -493,7 +488,7 @@ impl<'a> TreeContext<'a> { oi_meta: ObjectIndex, depth: Depth, ) -> Option { - 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" )]); diff --git a/tamer/src/ld/xmle/lower/test.rs b/tamer/src/ld/xmle/lower/test.rs index 2ed69dde..071ee35b 100644 --- a/tamer/src/ld/xmle/lower/test.rs +++ b/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::>(); diff --git a/tamer/src/ld/xmle/section.rs b/tamer/src/ld/xmle/section.rs index 01b25375..531c6a03 100644 --- a/tamer/src/ld/xmle/section.rs +++ b/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)); } } diff --git a/tamer/src/ld/xmle/xir/test.rs b/tamer/src/ld/xmle/xir/test.rs index a09093f3..9a829bcc 100644 --- a/tamer/src/ld/xmle/xir/test.rs +++ b/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!(