From 85892caeb2053a0aa3567350c67f52a368cccbf6 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 13 Jul 2023 15:20:39 -0400 Subject: [PATCH] tamer: asg: Root abstract identifiers in active container I'm not sure how I overlooked this previously, and I didn't notice until trying to generate xmli output. I think I distracted myself with the use of dangling status, which was not appropriate, and that has since changed so that we have a dedicated concept. This introduces the term "instantiation", or more specifically "lexical instantiation". This is more specific and meaningful than simply "expansion", which is what occurs during instantiation. I'll try to adjust terminology and make things more consistent as I go. DEV-13163 --- tamer/src/asg/air.rs | 103 ++++++++++++++++++++++++++-------- tamer/src/asg/air/expr.rs | 23 +++++--- tamer/src/asg/air/tpl/test.rs | 38 +++++++++---- 3 files changed, 124 insertions(+), 40 deletions(-) diff --git a/tamer/src/asg/air.rs b/tamer/src/asg/air.rs index 15c33ac6..f28fbb64 100644 --- a/tamer/src/asg/air.rs +++ b/tamer/src/asg/air.rs @@ -433,6 +433,42 @@ impl AirAggregate { Root(_) => Filter, } } + + /// Whether the active context represents an object that can be + /// lexically instantiated. + /// + /// Only containers that support _instantiation_ are able to contain + /// abstract identifiers. + /// Instantiation triggers expansion, + /// which resolves metavariables and makes all abstract objects + /// therein concrete. + fn is_lexically_instantiatible(&self) -> bool { + use AirAggregate::*; + + match self { + Uninit => false, + + // These objects cannot be instantiated, + // and so the abstract identifiers that they own would never + // be able to be made concrete. + Root(_) => false, + Pkg(_) => false, + PkgExpr(_) => false, + + // Templates are the metalinguistic abstraction and are, + // at the time of writing, + // the only containers capable of instantiation. + PkgTpl(_) => true, + + // Metavariables cannot own identifiers + // (they can only reference them). + PkgMeta(_) => false, + + // If an object is opaque to us then we cannot possibly look + // into it to see what needs expansion. + PkgOpaque(_) => false, + } + } } /// Behavior of an environment boundary when crossing environment upward @@ -851,11 +887,33 @@ impl AirAggregateCtx { /// /// A value of [`None`] indicates that no bindings are permitted in the /// current context. - fn rooting_oi(&self) -> Option> { + fn rooting_oi(&self) -> Option<(&AirAggregate, ObjectIndexToTree)> { self.stack .iter() .rev() - .find_map(|st| st.active_rooting_oi()) + .find_map(|st| st.active_rooting_oi().map(|oi| (st, oi))) + } + + /// The active container (rooting context) for _abstract_ [`Ident`]s. + /// + /// Only containers that support _instantiation_ are able to contain + /// abstract identifiers. + /// Instantiation triggers expansion, + /// which resolves metavariables and makes all abstract objects + /// therein concrete. + /// + /// This utilizes [`Self::rooting_oi`] to determine the active rooting + /// context. + /// If that context does not support instantiation, + /// [`None`] is returned. + /// This method will _not_ continue looking further up the stack for a + /// context that is able to be instantiated, + /// since that would change the parent of the binding. + fn instantiable_rooting_oi( + &self, + ) -> Option<(&AirAggregate, ObjectIndexToTree)> { + self.rooting_oi() + .filter(|(st, _)| st.is_lexically_instantiatible()) } /// The active dangling expression context for [`Expr`]s. @@ -943,7 +1001,7 @@ impl AirAggregateCtx { &mut self, binding_name: SPair, ) -> Result, AsgError> { - let oi_root = self + let (_, oi_root) = self .rooting_oi() .ok_or(AsgError::InvalidBindContext(binding_name))?; @@ -967,18 +1025,22 @@ impl AirAggregateCtx { &mut self, meta_name: SPair, ) -> Result, AsgError> { - // To help mitigate potentially cryptic errors down the line, - // let's try to be helpful and notify the user when - // they're trying to do something that almost certainly - // will not succeed. - match self.dangling_expr_oi() { - // The container does not support dangling expressions - // and so there is no chance that this expression will - // be expanded in the future. + match self.instantiable_rooting_oi() { + // The container cannot be instantiated and so there is no + // chance that this expression will be expanded in the future. None => { + // Since we do not have an abstract container, + // the nearest container (if any) is presumably concrete, + // so let's reference that in the hope of making the error + // more informative. + // Note that this _does_ re-search the stack, + // but this is an error case that should seldom occur. + // If it's a problem, + // we can have `instantiable_rooting_oi` retain + // information. let rooting_span = self .rooting_oi() - .map(|oi| oi.widen().resolve(self.asg_ref()).span()); + .map(|(_, oi)| oi.widen().resolve(self.asg_ref()).span()); // Note that we _discard_ the attempted bind token // and so remain in a dangling state. @@ -988,18 +1050,15 @@ impl AirAggregateCtx { )) } - // We don't care what our container is, - // only that the above check passed. - // That is: - // the above check is entirely optional and intended - // only as a debugging aid for users. - Some(_) => { + // We root a new identifier in the instantiable container, + // but we do not index it, + // since its name is not known until instantiation. + Some((_, oi_root)) => { let oi_meta_ident = self.lookup_lexical_or_missing(meta_name); - let oi_abstract = oi_meta_ident - .new_abstract_ident(self.asg_mut(), meta_name.span()); - - Ok(oi_abstract) + Ok(oi_meta_ident + .new_abstract_ident(self.asg_mut(), meta_name.span()) + .add_edge_from(self.asg_mut(), oi_root, None)) } } } diff --git a/tamer/src/asg/air/expr.rs b/tamer/src/asg/air/expr.rs index a9715f22..a0ba8801 100644 --- a/tamer/src/asg/air/expr.rs +++ b/tamer/src/asg/air/expr.rs @@ -139,15 +139,22 @@ impl ParseState for AirExprAggregate { } (BuildingExpr(es, oi), AirBind(BindIdentAbstract(meta_name))) => { - // Note that we are still dangling, - // since the identifier is abstract and is therefore not - // yet reachable via its yet-to-be-determined identifier. - ctx.defines_abstract(meta_name) - .and_then(|oi_ident| { + let result = + ctx.defines_abstract(meta_name).and_then(|oi_ident| { oi_ident.bind_definition(ctx.asg_mut(), meta_name, oi) - }) - .map(|_| ()) - .transition(BuildingExpr(es, oi)) + }); + + // It is important that we do not mark this expression as + // reachable unless we successfully bind the identifier. + // Even though the identifier is abstract, + // we want to mimic the concrete structure of the graph. + match result { + Ok(oi_ident) => { + Transition(BuildingExpr(es.reachable_by(oi_ident), oi)) + .incomplete() + } + Err(e) => Transition(BuildingExpr(es, oi)).err(e), + } } (BuildingExpr(es, oi), AirBind(RefIdent(name))) => { diff --git a/tamer/src/asg/air/tpl/test.rs b/tamer/src/asg/air/tpl/test.rs index 94eb1289..d553dd11 100644 --- a/tamer/src/asg/air/tpl/test.rs +++ b/tamer/src/asg/air/tpl/test.rs @@ -816,18 +816,18 @@ fn expr_abstract_bind_produces_cross_edge_from_ident_to_meta() { let ctx = air_ctx_from_pkg_body_toks(toks); let asg = ctx.asg_ref(); - // The expression is dangling and so we must find it through a - // traversal. + // The expression cannot be indexed by a named (concrete) identifier + // and so we must find it through a traversal. + // The abstract identifier should be bound to our rooting context, + // which is the template, + // despite not having a name; + // this mirrors the same structure that the template body will + // assume upon instantiation. let oi_tpl = pkg_expect_ident_oi::(&ctx, id_tpl); - let oi_expr = oi_tpl - .edges_filtered::(asg) + let oi_ident = oi_tpl + .edges_filtered::(asg) .next() - .expect("could not locate child Expr"); - - // The abstract identifier that should have been bound to the expression. - let oi_ident = oi_expr - .ident(asg) - .expect("abstract identifier was not bound to Expr"); + .expect("abstract identifier is not rooted in template"); assert_eq!( None, @@ -837,4 +837,22 @@ fn expr_abstract_bind_produces_cross_edge_from_ident_to_meta() { // The metavariable referenced by the abstract identifier assert_eq!(id_meta, oi_ident.name_or_meta(asg)); + + // The identifier should be bound to the expression. + let oi_expr = oi_ident + .definition::(asg) + .expect("abstract identifier did not bind to Expr"); + + assert_eq!(S3.merge(S5).unwrap(), oi_expr.resolve(asg).span()); + + // Finally, + // the expression should not be considered dangling and so we should + // not have an edge directly from the template to the Expr. + // If that were to happen, + // then we'd end up duplicating the expression. + assert!( + oi_tpl.edges_filtered::(asg).next().is_none(), + "Tpl must not have an edge directly to Expr \ + (is it considered dangling?)", + ); }