diff --git a/tamer/src/asg/air/expr.rs b/tamer/src/asg/air/expr.rs index ca87a9eb..ae135770 100644 --- a/tamer/src/asg/air/expr.rs +++ b/tamer/src/asg/air/expr.rs @@ -31,7 +31,6 @@ use super::{ }; use crate::{ asg::{graph::object::ObjectIndexToTree, ObjectKind}, - f::Map, parse::prelude::*, }; @@ -102,9 +101,7 @@ impl ParseState for AirExprAggregate { } (BuildingExpr(es, oi), AirExpr(ExprEnd(end))) => { - let _ = oi.map_obj(ctx.asg_mut(), |expr| { - expr.map(|span| span.merge(end).unwrap_or(span)) - }); + oi.close(ctx.asg_mut(), end); let dangling = es.is_dangling(); let oi_root = ctx.dangling_expr_oi(); diff --git a/tamer/src/asg/air/expr/test.rs b/tamer/src/asg/air/expr/test.rs index 314c042a..87237df5 100644 --- a/tamer/src/asg/air/expr/test.rs +++ b/tamer/src/asg/air/expr/test.rs @@ -18,6 +18,7 @@ // along with this program. If not, see . use super::*; +use crate::convert::ExpectInto; use crate::span::dummy::*; use crate::{ asg::{ @@ -30,7 +31,10 @@ use crate::{ Air::*, AirAggregate, }, - graph::object::{expr::ExprRel, Doc, ObjectRel}, + graph::object::{ + expr::{ExprRel, MetaState}, + Doc, ObjectRel, + }, ExprOp, Ident, }, parse::util::spair, @@ -191,6 +195,7 @@ fn expr_non_empty_ident_root() { let expr_a = pkg_expect_ident_obj::(&ctx, id_a); assert_eq!(expr_a.span(), S1.merge(S6).unwrap()); + assert_eq!(expr_a.meta_state(), MetaState::Concrete); // Identifiers should reference the same expression. let expr_b = pkg_expect_ident_obj::(&ctx, id_b); @@ -888,3 +893,69 @@ fn abstract_bind_without_dangling_container() { let _ = sut.finalize().unwrap(); } + +// We cannot be confident that something is concrete until we know what type +// of object is being referenced. +// If there is any level of uncertainty, +// we defer that decision. +#[test] +fn expr_referencing_missing_without_abstract_is_unknown() { + #[rustfmt::skip] + let toks = [ + ExprStart(ExprOp::Sum, S1), + BindIdent(spair("expr", S2)), + + // This identifier does not exist, + // and so we can't know whether it is a metavariable, + // and so we can't know whether we are concrete. + RefIdent(spair("missing", S3)), + + // We count the number of missing references, + // so despite this being the same reference, + // it counts. + RefIdent(spair("missing", S4)), + + RefIdent(spair("another_missing", S5)), + ExprEnd(S6), + ]; + + let ctx = air_ctx_from_pkg_body_toks(toks); + + let expr = pkg_expect_ident_obj::(&ctx, spair("expr", S20)); + + // We have _three_ references above that are not defined. + assert_eq!(expr.meta_state(), MetaState::MaybeConcrete(3.unwrap_into())); +} + +// Same as above, +// but throw in a known reference to show that it doesn't override the +// missing one. +#[test] +fn expr_referencing_missing_with_concrete_without_abstract_is_unknown() { + #[rustfmt::skip] + let toks = [ + ExprStart(ExprOp::Sum, S1), + BindIdent(spair("known", S2)), // <-. + ExprEnd(S3), // | + // | + ExprStart(ExprOp::Sum, S4), // | + BindIdent(spair("expr", S5)), // | + // | + // For this reason we are missing. // | + RefIdent(spair("missing", S6)), // | + // | + // But this one is known; // | + // let's make sure it does not // | + // override the previous inference. // | + RefIdent(spair("known", S7)), // --' + ExprEnd(S8), + ]; + + let ctx = air_ctx_from_pkg_body_toks(toks); + + let expr = pkg_expect_ident_obj::(&ctx, spair("expr", S20)); + + // One is known, + // so only one is missing. + assert_eq!(expr.meta_state(), MetaState::MaybeConcrete(1.unwrap_into())); +} diff --git a/tamer/src/asg/air/test.rs b/tamer/src/asg/air/test.rs index 8ea14618..73a44bae 100644 --- a/tamer/src/asg/air/test.rs +++ b/tamer/src/asg/air/test.rs @@ -791,3 +791,15 @@ pub fn pkg_expect_ident_obj>( ) -> &O { pkg_expect_ident_oi(ctx, name).resolve(ctx.asg_ref()) } + +pub fn expect_ident_obj>( + ctx: &::Context, + env: impl ObjectIndexRelTo, + name: SPair, +) -> &O { + ctx.env_scope_lookup::(env, name) + .expect("missing requested Ident `{name}`") + .definition_narrow::(ctx.asg_ref()) + .expect("missing `{name}` definition") + .resolve(ctx.asg_ref()) +} diff --git a/tamer/src/asg/air/tpl/test.rs b/tamer/src/asg/air/tpl/test.rs index 6f0742a6..2b0bc797 100644 --- a/tamer/src/asg/air/tpl/test.rs +++ b/tamer/src/asg/air/tpl/test.rs @@ -20,7 +20,8 @@ mod apply; use super::*; -use crate::asg::air::test::{as_pkg_body, Sut}; +use crate::asg::air::test::{as_pkg_body, expect_ident_obj, Sut}; +use crate::convert::ExpectInto; use crate::span::dummy::*; use crate::{ asg::{ @@ -33,7 +34,7 @@ use crate::{ }, Air::*, }, - graph::object::{tpl::TplShape, Doc, Meta, ObjectRel}, + graph::object::{expr::MetaState, tpl::TplShape, Doc, Meta, ObjectRel}, Expr, ExprOp, Ident, }, parse::util::spair, @@ -779,11 +780,18 @@ fn expr_abstract_bind_produces_cross_edge_from_ident_to_meta() { assert_eq!(id_meta, oi_ident.name_or_meta(asg)); // The identifier should be bound to the expression. - let oi_expr = oi_ident + let expr = oi_ident .definition_narrow::(asg) - .expect("abstract identifier did not bind to Expr"); + .expect("abstract identifier did not bind to Expr") + .resolve(asg); - assert_eq!(S3.merge(S5).unwrap(), oi_expr.resolve(asg).span()); + assert_eq!(S3.merge(S5).unwrap(), expr.span()); + + // The abstract identifier references the expression, + // but the expression does not contain it. + // Consequently, + // the expression is still concrete. + assert_eq!(expr.meta_state(), MetaState::Concrete); // Finally, // the expression should not be considered dangling and so we should @@ -804,3 +812,158 @@ fn expr_abstract_bind_produces_cross_edge_from_ident_to_meta() { // it all ends up expanding into the same thing in the end. assert_eq!(TplShape::Empty, oi_tpl.resolve(&asg).shape()); } + +// Just because an expression is defined within a template does not mean +// that it abstract; +// expressions without metavariable references are concrete. +#[test] +fn expressions_within_tpls_without_metavars_are_concrete() { + #[rustfmt::skip] + let toks = [ + TplStart(S1), + BindIdent(spair("_tpl_", S2)), + + // This expression should be concrete, + // since it references no metavariables. + ExprStart(ExprOp::Sum, S3), + BindIdent(spair("expr", S4)), + ExprEnd(S5), + TplEnd(S7), + ]; + + let ctx = air_ctx_from_pkg_body_toks(toks); + let oi_tpl = pkg_expect_ident_oi::(&ctx, spair("_tpl_", S7)); + + let expr = expect_ident_obj::(&ctx, oi_tpl, spair("expr", S8)); + assert_eq!(expr.meta_state(), MetaState::Concrete); +} + +#[test] +fn expressions_referencing_metavars_are_abstract() { + #[rustfmt::skip] + let toks = [ + TplStart(S1), + BindIdent(spair("_tpl_", S2)), + + // TODO: At the time of writing, + // we do not yet notify objects when a + // missing Ident has received a definition. + // Until that time, + // this will remain unresolved. + ExprStart(ExprOp::Sum, S3), + BindIdent(spair("expr_pre", S4)), + + // This has yet to be defined, + // and so is `Missing`. + RefIdent(spair("@param@", S5)), // --. + ExprEnd(S6), // | + // | + MetaStart(S7), // | + BindIdent(spair("@param@", S8)), // <-: + // | + // We'll give this metavar a value just to // | + // show that it doesn't matter that we // | + // _could_ make the expression concrete; // | + // expansion is never performed until // | + // it is explicitly requested. // | + MetaLexeme(spair("value", S9)), // | + MetaEnd(S10), // | + // | + ExprStart(ExprOp::Sum, S11), // | + BindIdent(spair("expr_post", S12)), // <-+--. + // | | + // This reference causes the parent // | | + // expression to become abstract since // | | + // it requires expansion. // | | + RefIdent(spair("@param@", S13)), // --' | + ExprEnd(S14), // | + // | + ExprStart(ExprOp::Sum, S15), // | + BindIdent(spair("expr_conc", S16)), // | + // | + // Even though we reference an abstract // | + // expression, // | + // that expression is not our child // | + // and therefore does not affect // | + // whether this expression is abstract. // | + RefIdent(spair("expr_post", S17)), // -----' + ExprEnd(S18), + TplEnd(S19), + ]; + + let ctx = air_ctx_from_pkg_body_toks(toks); + let oi_tpl = pkg_expect_ident_oi::(&ctx, spair("_tpl_", S20)); + + // Both `expr_pre` and `expr_post` expressions are abstract, + // containing a reference to a metavariable. + // Intuitively, + // we don't know what we're referencing until that metavariable is + // replaced with a lexical value during template application. + let expr_post = + expect_ident_obj::(&ctx, oi_tpl, spair("expr_post", S21)); + assert_eq!(expr_post.meta_state(), MetaState::Abstract); + + // ...but in the case of expr_pre, + // we don't yet notify the object on Ident resolution and so we do not + // yet know that it ought to be abstract. + let expr_pre = + expect_ident_obj::(&ctx, oi_tpl, spair("expr_pre", S22)); + assert_eq!( + expr_pre.meta_state(), + MetaState::MaybeConcrete(1_u16.unwrap_into()) + ); + + // But an expression that _references_ that abstract expression does not + // itself become abstract. + // That is: + // we depend on `expr_post` having been computed before we can + // reference it, + // but that dependency is not affected by whether `expr_post` is concrete + // or abstract. + // It is certainly required that `expr_post` be made concrete before it + // can be lowered into the target, + // but provided that occurs + // (and there would be a compilation failure if it didn't), + // we are unaffected. + let expr_conc = + expect_ident_obj::(&ctx, oi_tpl, spair("expr_conc", S23)); + assert_eq!(expr_conc.meta_state(), MetaState::Concrete); +} + +// If we know of at least _one_ abstract reference, +// then it does not matter if we have missing references---​ +// we are abstract. +#[test] +fn expression_referencing_abstract_with_missing_is_abstract() { + #[rustfmt::skip] + let toks = [ + TplStart(S1), + BindIdent(spair("_tpl_", S2)), + + MetaStart(S3), + BindIdent(spair("@param@", S4)), // <-. + MetaEnd(S5), // | + // | + ExprStart(ExprOp::Sum, S7), // | + BindIdent(spair("expr", S8)), // | + // | + // This reference causes the parent // | + // expression to become abstract since // | + // it requires expansion. // | + RefIdent(spair("@param@", S9)), // --' + + // This reference is unknown, + // but we're still just abstract, + // since it takes only a single abstract reference to make + // that determination. + RefIdent(spair("missing", S10)), + ExprEnd(S11), + TplEnd(S12), + ]; + + let ctx = air_ctx_from_pkg_body_toks(toks); + let oi_tpl = pkg_expect_ident_oi::(&ctx, spair("_tpl_", S20)); + + let expr = expect_ident_obj::(&ctx, oi_tpl, spair("expr", S21)); + assert_eq!(expr.meta_state(), MetaState::Abstract); +} diff --git a/tamer/src/asg/graph/object/expr.rs b/tamer/src/asg/graph/object/expr.rs index 5c79aa1e..49bb33e8 100644 --- a/tamer/src/asg/graph/object/expr.rs +++ b/tamer/src/asg/graph/object/expr.rs @@ -27,9 +27,14 @@ //! the value that they represent without affecting the meaning of the //! program. -use super::{prelude::*, Doc, Ident, ObjectIndexToTree, Tpl}; -use crate::{num::Dim, span::Span}; -use std::fmt::Display; +use super::{ + ident::IdentDefinition, prelude::*, Doc, Ident, ObjectIndexToTree, Tpl, +}; +use crate::{ + asg::graph::ProposedRel, diagnose::panic::DiagnosticPanic, num::Dim, + parse::prelude::Annotate, span::Span, +}; +use std::{fmt::Display, num::NonZeroU16}; #[cfg(doc)] use super::ObjectKind; @@ -43,6 +48,7 @@ use super::ObjectKind; pub struct Expr { op: ExprOp, dim: ExprDim, + meta: MetaState, span: Span, } @@ -51,6 +57,7 @@ impl Expr { Self { op, dim: ExprDim::default(), + meta: MetaState::default(), span, } } @@ -66,10 +73,28 @@ impl Expr { Expr { op, .. } => *op, } } + + /// Whether an expression is concrete, abstract, or not yet known. + /// + /// Note that, + /// since [`Ident`]s reference expressions, + /// an abstract identifier is able to reference a concrete + /// expression. + /// This may not be intuitive when looking at the source XML notation, + /// or when looking at AIR, + /// since both are structured to appear as though the expression + /// parents the identifier; + /// this is not the case. + pub fn meta_state(&self) -> MetaState { + match self { + Expr { meta, .. } => *meta, + } + } } impl_mono_map! { Span => Expr { span, .. }, + MetaState => Expr { meta, .. }, } impl From<&Expr> for Span { @@ -84,10 +109,11 @@ impl Display for Expr { Self { op, dim, + meta, // intentional: exhaustiveness check to bring attention to // this when fields change span: _span, - } => write!(f, "{op} expression with {dim}"), + } => write!(f, "{meta} {op} expression with {dim}"), } } } @@ -226,13 +252,158 @@ impl Display for DimState { } } +/// The state of an [`Expr`] in a metalanguage context. +/// +/// Intuitively, +/// an expression is [`MetaState::Concrete`] if and only if template +/// expansion would act as an identity function. +/// +/// This does not cache edges that contributed to these decisions, +/// since all such edges are direct children and can be quickly and easily +/// discovered by iterating over those edges. +#[derive(Debug, PartialEq, Eq, Clone, Copy, Default)] +pub enum MetaState { + /// Neither the expression nor its children references any + /// metavariables. + #[default] + Concrete, + + /// Either the expression or one of its children references some + /// metavariable. + /// + /// This does not store information about the location of those + /// references; + /// it is expected that they will be located during a walk of the + /// graph during e.g. template expansion. + /// + /// Note that a metavariable reference is behind an [`Ident`]. + Abstract, + + /// There are a number of references to [`Ident`]s that are missing + /// definitions, + /// but no abstract references have yet been found. + /// + /// As soon as a single abstract reference is encountered, + /// [`Self::Abstract`] is able to be inferred and this distinction no + /// longer matters. + /// + /// The choice of [`u16`] for this count is a compromise: + /// [`u8`] is too small for aggressive out-of-order code generation, + /// and [`u32`] is a lot of space to waste for every [`Expr`] for + /// something that is very unlikely to ever occur. + /// [`u16`] is plenty large enough to put the burden on a code + /// generation tool to either break up expressions, + /// or to order dependencies + /// (the former is relatively trivial for any tool). + MaybeConcrete(NonZeroU16), +} + +impl MetaState { + /// Cache the existence of an abstract identifier. + /// + /// This will always result in [`Self::Abstract`], + /// no matter what the current state of `self`. + fn found_abstract(self) -> Self { + // At the time of writing, + // abstract takes precedence over all other states. + // However, + // please keep the exhaustive check here, + // as it will draw our attention to this for new variants just in + // case that assumption changes. + match self { + Self::Concrete | Self::Abstract | Self::MaybeConcrete(_) => { + Self::Abstract + } + } + } + + /// Cache the existence of an identifier that is not known to be either + /// concrete or abstract. + /// + /// The [`Span`] `at` is used only for diagnostics if storage limits are + /// exceeded + /// (see [`Self::MaybeConcrete`]). + /// The system does not cache information about edges; + /// it maintains only a count that can be decremented as edges are + /// resolved in the future. + fn found_missing(self, at: Span) -> Self { + match self { + Self::Concrete => Self::MaybeConcrete(NonZeroU16::MIN), + Self::Abstract => Self::Abstract, + Self::MaybeConcrete(x) => { + Self::MaybeConcrete(x.checked_add(1).diagnostic_unwrap(|| { + vec![ + at.internal_error("missing identifier limit exceeded"), + at.help( + "either move this reference into another \ + expression or move dependencies before this \ + expression", + ), + at.help( + "it is not expected that this limit be reached \ + except by code generation", + ), + ] + })) + } + } + } +} + +impl Display for MetaState { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + MetaState::Concrete => write!(f, "concrete"), + MetaState::Abstract => write!(f, "abstract"), + MetaState::MaybeConcrete(n) => { + write!(f, "possibly-concrete ({n}-Missing)") + } + } + } +} + object_rel! { /// An expression is inherently a tree, /// however it may contain references to other identifiers which /// represent their own trees. /// Any [`Ident`] reference is a cross edge. Expr -> { - cross Ident, + cross Ident { + fn pre_add_edge( + asg: &mut Asg, + rel: ProposedRel, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError> { + match rel.to_oi.definition(asg) { + // Metavariable references mean that the source + // expression will require expansion. + Some(IdentDefinition::Meta(_)) => { + rel.from_oi.map_obj_inner( + asg, + |meta: MetaState| meta.found_abstract() + ); + }, + + // Non-meta identifiers are just references. + // We don't care what they are as long as they're not + // metavariables. + Some(IdentDefinition::Expr(_) | IdentDefinition::Tpl(_)) => (), + + None => { + rel.from_oi.map_obj_inner(asg, |meta: MetaState| { + // This is a cross edge and so this span must be + // available, but the types provided don't + // guarantee that. + let span = rel.ref_span.unwrap_or(rel.to_oi.span()); + meta.found_missing(span) + }); + } + }; + + Ok(commit(asg)) + } + }, + tree Expr, tree Doc, @@ -242,6 +413,12 @@ object_rel! { } impl ObjectIndex { + /// Finalize an expression's definition by updating its span to + /// encompass the entire (lexical) definition. + pub fn close(self, asg: &mut Asg, end: Span) -> Self { + self.map_obj_inner(asg, |span: Span| span.merge(end).unwrap_or(span)) + } + /// Create a new subexpression as the next child of this expression and /// return the [`ObjectIndex`] of the new subexpression. ///