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?)", + ); }