From bdd98a5d924c78e0909842393126f333555da036 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 16 Aug 2023 15:17:00 -0400 Subject: [PATCH] tamer: asg: Require that all template parameters be referenced in body This ensures that each metavariable defined within a template (a template parameter) has, by the time that the template definition is ended, at least one reference to each metavariable. This has practical benefits---ensuring that you haven't forgotten to use a metavariable; ensuring that you clean up when code is removed; and ensuring that you didn't accidentally delete some reference that you didn't intend to (at least in the case of _all_ references...)---but the rationale for it in this commit is a bit different. More on that below. This does introduce the negative effect of making it more difficult to consume inputs without utilizing them, acting more like a relevant type system (in terms of substructural type systems and with respect to metavariables, at least). You can for now reference them in contexts that would reasonably have no effect on the program or be optimized away, but in the future it'd be nice to explicitly state "hey this isn't intended to be used yet", especially if you're creating shells of templates, or trying to maintain BC in a particular situation. But more on that in the future. For now, the real reason for this change is because of how I intend for template expansion to work: by walking the body. Rather than having to check both the parameters of the template and then expand the body separately, we can simply trust that each parameter is referenced. Then, after rebinding metavariable references, we can perform the same check on the expansion template to see if there were arguments provided that do not correspond to parameters. This also adds flexibility with parameters that are used conditionally---we'll be able to have conditionally required parameters in error reporting. More information on this is coming, though; it'll be included in the docs of the commit that introduces the changes. DEV-13163 --- tamer/src/asg/air/meta/test.rs | 20 +++- tamer/src/asg/air/test/scope.rs | 20 +++- tamer/src/asg/air/tpl.rs | 2 +- tamer/src/asg/air/tpl/test.rs | 75 ++++++++++++- tamer/src/asg/error.rs | 33 +++++- tamer/src/asg/graph.rs | 27 +++++ tamer/src/asg/graph/object/ident.rs | 7 ++ tamer/src/asg/graph/object/tpl.rs | 37 ++++++- tamer/src/asg/graph/visit/ontree/test.rs | 135 +++++++++++++---------- tamer/src/nir/abstract_bind.rs | 27 +++++ tamer/src/nir/abstract_bind/test.rs | 34 ++++++ tamer/tests/xmli/template/expected.xml | 10 ++ tamer/tests/xmli/template/src.xml | 10 ++ 13 files changed, 356 insertions(+), 81 deletions(-) diff --git a/tamer/src/asg/air/meta/test.rs b/tamer/src/asg/air/meta/test.rs index fbc18855..2c5aacc2 100644 --- a/tamer/src/asg/air/meta/test.rs +++ b/tamer/src/asg/air/meta/test.rs @@ -34,6 +34,7 @@ use crate::{ Air::{self, *}, }, graph::object::{meta::MetaRel, Meta, Tpl}, + ExprOp, }, parse::{util::spair, Parser}, span::{dummy::*, Span}, @@ -263,14 +264,23 @@ fn assert_concat_list<'a, IT, IE: 'a>( IE: IntoIterator, IE::IntoIter: Debug + DoubleEndedIterator, { - let (ctx, oi_tpl) = air_ctx_from_tpl_body_toks(toks); + let meta_sym = meta_name.into(); + + let (ctx, oi_tpl) = air_ctx_from_tpl_body_toks( + #[rustfmt::skip] + toks.into_iter().chain(vec![ + // Reference the provided metavariable to avoid + // `AsgError::TplUnusedParams`. + ExprStart(ExprOp::Sum, S20), + RefIdent(spair(meta_sym, S21)), + ExprEnd(S22), + ]), + ); + let asg = ctx.asg_ref(); let oi_meta = ctx - .env_scope_lookup_ident_dfn::( - oi_tpl, - spair(meta_name.into(), DUMMY_SPAN), - ) + .env_scope_lookup_ident_dfn::(oi_tpl, spair(meta_sym, DUMMY_SPAN)) .unwrap(); // Meta references are only supported through lexical concatenation. diff --git a/tamer/src/asg/air/test/scope.rs b/tamer/src/asg/air/test/scope.rs index 501d3eaa..b4aabe97 100644 --- a/tamer/src/asg/air/test/scope.rs +++ b/tamer/src/asg/air/test/scope.rs @@ -193,6 +193,7 @@ test_scopes! { // | : ExprStart(ExprOp::Sum, S7), // | : BindIdent(expr_outer), // vd|s : + RefIdent(meta_outer), // | : ExprEnd(S9), // | : // | : TplStart(S10), //---. | : @@ -205,6 +206,7 @@ test_scopes! { // 3| 2| 1: 0 ExprStart(ExprOp::Sum, S15), // | | : BindIdent(expr_inner), // vd|s |s : + RefIdent(meta_inner), // | | : ExprEnd(S17), // | | : TplEnd(S18), //---' | : v,s = EnvScopeKind TplEnd(S19), //–-----' : | @@ -359,15 +361,23 @@ test_scopes! { MetaStart(S4), // | : BindIdent(meta_same_a), // vl|s : <--. MetaEnd(S6), // | : | - TplEnd(S7), //----' : | - // : |s - TplStart(S8), //----. : |a - // ENV: 3 tpl // | : |m - BindIdent(tpl_inner), // |~ : |e + // | : | + ExprStart(ExprOp::Sum, S6), // | : | + RefIdent(meta_same_a), // | : | + ExprEnd(S6), // | : |s + TplEnd(S7), //----' : |a + // : |m + TplStart(S8), //----. : |e + // ENV: 3 tpl // | : | + BindIdent(tpl_inner), // |~ : | // | : | MetaStart(S10), // | : | BindIdent(meta_same_b), // vl|s : <--' MetaEnd(S12), // | : + // | : + ExprStart(ExprOp::Sum, S12), // | : + RefIdent(meta_same_b), // | : + ExprEnd(S12), // | : TplEnd(S13), //----' : ~ = ignored for PkgEnd(S14), //- - - -' these tests ] diff --git a/tamer/src/asg/air/tpl.rs b/tamer/src/asg/air/tpl.rs index 997a37ba..14b97506 100644 --- a/tamer/src/asg/air/tpl.rs +++ b/tamer/src/asg/air/tpl.rs @@ -126,7 +126,7 @@ impl TplState { asg: &mut Asg, close_span: Span, ) -> Result, AsgError> { - let oi = self.oi().close(asg, close_span); + let oi = self.oi().close(asg, close_span)?; match self { Self::Dangling(_) => { diff --git a/tamer/src/asg/air/tpl/test.rs b/tamer/src/asg/air/tpl/test.rs index 757ec0dd..6b599144 100644 --- a/tamer/src/asg/air/tpl/test.rs +++ b/tamer/src/asg/air/tpl/test.rs @@ -512,7 +512,14 @@ fn tpl_with_param() { BindIdent(id_param2), DocIndepClause(param_desc), MetaEnd(S10), - TplEnd(S11), + + // Metavariables must be used by something, + // otherwise we'll receive an error. + ExprStart(ExprOp::Sum, S11), + RefIdent(id_param1), + RefIdent(id_param2), + ExprEnd(S14), + TplEnd(S15), ]; let ctx = air_ctx_from_pkg_body_toks(toks); @@ -521,9 +528,7 @@ fn tpl_with_param() { let oi_tpl = pkg_expect_ident_oi::(&ctx, id_tpl); let tpl = oi_tpl.resolve(&asg); - // The template contains no body - // (only metavariables / params). - assert_eq!(TplShape::Empty, tpl.shape()); + assert_eq!(TplShape::Expr(S11.merge(S14).unwrap()), tpl.shape()); // The template should have an edge to each identifier for each // metavariable. @@ -677,6 +682,8 @@ fn metavars_within_exprs_hoisted_to_parent_tpl() { MetaStart(S4), BindIdent(id_param_outer), MetaEnd(S6), + + RefIdent(id_param_outer), ExprEnd(S7), // Nested template @@ -688,6 +695,8 @@ fn metavars_within_exprs_hoisted_to_parent_tpl() { MetaStart(S11), BindIdent(id_param_inner), MetaEnd(S13), + + RefIdent(id_param_inner), ExprEnd(S14), TplEnd(S15), TplEnd(S16), @@ -932,7 +941,7 @@ fn expressions_referencing_metavars_are_abstract() { } // If we know of at least _one_ abstract reference, -// then it does not matter if we have missing references---​ +// then it does not matter if we have missing references--- // we are abstract. #[test] fn expression_referencing_abstract_with_missing_is_abstract() { @@ -1097,3 +1106,59 @@ fn abstract_doc_clause_cannot_bind_to_non_meta_ref() { .filter_map(|oi_ident| oi_ident.definition_narrow::(asg)) .any(|oi| oi.resolve(asg).span() == S7.merge(S11).unwrap())); } + +/// All template metavariables (params) must hold at least one reference. +/// For rationale, +/// see the parent module's documentation. +#[test] +fn unused_metavars_at_definition_tpl_close_produce_error() { + #[rustfmt::skip] + let toks = [ + TplStart(S1), + BindIdent(spair("_tpl", S2)), + + MetaStart(S3), + BindIdent(spair("@used@", S4)), // <-. + MetaEnd(S5), // | + // | + MetaStart(S6), // | + BindIdent(spair("@unused_a@", S7)), // | + MetaEnd(S8), // | + // | + MetaStart(S9), // | + BindIdent(spair("@unused_b@", S10)), // | + MetaEnd(S11), // | + // | + ExprStart(ExprOp::Sum, S12), // | + RefIdent(spair("@used@", S13)), // --' + ExprEnd(S14), + TplEnd(S15), + ]; + + let mut sut = parse_as_pkg_body(toks); + + assert_eq!( + #[rustfmt::skip] + vec![ + Err(ParseError::StateError(AsgError::TplUnusedParams( + Some(spair("_tpl", S2)), + vec![ + spair("@unused_a@", S7), + spair("@unused_b@", S10), + ], + ))), + ], + sut.by_ref().filter(Result::is_err).collect::>(), + ); + + let ctx = sut.finalize().unwrap().into_private_context(); + + // The template should not have been defined. + // This is admittedly driven by implementation details: + // no other part of AIR's parsing continues with an operation despite + // an error at the time of writing. + // TODO: Should we allow the template to be defined, + // so that the user doesn't get errors saying the template doesn't + // exist? + assert_eq!(None, pkg_lookup(&ctx, spair("_tpl_", S20))); +} diff --git a/tamer/src/asg/error.rs b/tamer/src/asg/error.rs index b3842a21..775353f7 100644 --- a/tamer/src/asg/error.rs +++ b/tamer/src/asg/error.rs @@ -26,7 +26,7 @@ use std::{ use crate::{ diagnose::{Annotate, AnnotatedSpan, Diagnostic}, - fmt::{DisplayWrapper, TtQuote}, + fmt::{AndQualConjList, DisplayWrapper, ListDisplayWrapper, TtQuote}, parse::util::SPair, span::Span, }; @@ -195,6 +195,13 @@ pub enum AsgError { /// The template name is provided if it is known at the time of the /// error. TplShapeExprMulti(Option, ErrorOccurrenceSpan, FirstOccurrenceSpan), + + /// A template has one or more concrete params that are not referred to + /// by any other object. + /// + /// A param is a metavariable identifier; + /// the provided [`SPair`] is the name and span of the identifier. + TplUnusedParams(Option, Vec), } /// A [`Span`] representing the subject of this error. @@ -329,6 +336,19 @@ impl Display for AsgError { "template definition would produce multiple inline \ expressions when expanded" ), + + TplUnusedParams(oname, params) => write!( + f, + "{tpl} has {n} unused {plist}", + tpl = match oname { + Some(name) => format!("template {}", TtQuote::wrap(name)), + None => "anonymous template".into(), + }, + n = params.len(), + plist = AndQualConjList::<"param:", "params:", TtQuote>::wrap( + params + ), + ), } } } @@ -594,6 +614,17 @@ impl Diagnostic for AsgError { ), ]) .collect(), + + TplUnusedParams(oname, params) => oname + .map(|name| name.note("for this template")) + .into_iter() + .chain(params.iter().map(|name| { + name.error(format!( + "param {} is not referenced", + TtQuote::wrap(name) + )) + })) + .collect(), } } } diff --git a/tamer/src/asg/graph.rs b/tamer/src/asg/graph.rs index 6327a059..a5e11bec 100644 --- a/tamer/src/asg/graph.rs +++ b/tamer/src/asg/graph.rs @@ -412,6 +412,33 @@ impl Asg { .map(move |edge| ObjectIndex::::new(edge.source(), oi)) } + /// Incoming edges to `oi`. + /// + /// This is intended to be utilized when + /// [`Self::incoming_edges_filtered`] cannot be used. + /// In particular, + /// this is useful when traversing the graph in reverse, + /// but does not offer the same type benefits; + /// (you must deal with a generic [`Object`] kind). + fn incoming_edges_dyn<'a>( + &'a self, + oi: ObjectIndex, + ) -> impl Iterator + 'a { + self.graph + .edges_directed(oi.into(), Direction::Incoming) + .map(move |edge| { + let (src_ty, target_ty, ref_span) = edge.weight(); + + DynObjectRel::new( + *src_ty, + *target_ty, + oi, + ObjectIndex::::new(edge.target(), oi), + *ref_span, + ) + }) + } + /// Check whether an edge exists from `from` to `to. #[inline] pub fn has_edge( diff --git a/tamer/src/asg/graph/object/ident.rs b/tamer/src/asg/graph/object/ident.rs index 54b62ef0..f8a284e6 100644 --- a/tamer/src/asg/graph/object/ident.rs +++ b/tamer/src/asg/graph/object/ident.rs @@ -1493,6 +1493,13 @@ impl ObjectIndex { asg.create(Ident::new_abstract(at)) .add_cross_edge_to(asg, self, at) } + + /// Whether there exists at least one object that holds a reference to + /// this identifier. + pub fn is_referenced(&self, asg: &Asg) -> bool { + asg.incoming_edges_dyn(ObjectIndex::widen(*self)) + .any(|edge| edge.is_cross_edge()) + } } #[cfg(test)] diff --git a/tamer/src/asg/graph/object/tpl.rs b/tamer/src/asg/graph/object/tpl.rs index 605a372e..27f0b8c8 100644 --- a/tamer/src/asg/graph/object/tpl.rs +++ b/tamer/src/asg/graph/object/tpl.rs @@ -21,7 +21,7 @@ use std::fmt::Display; -use super::{ident::IdentDefinition, prelude::*, Doc, Expr, Ident}; +use super::{ident::IdentDefinition, prelude::*, Doc, Expr, Ident, Meta}; use crate::{asg::graph::ProposedRel, f::Map, parse::util::SPair, span::Span}; /// Template with associated name. @@ -376,12 +376,43 @@ impl ObjectIndex { /// /// This updates the span of the template to encompass the entire /// definition. - pub fn close(self, asg: &mut Asg, close_span: Span) -> Self { + pub fn close( + self, + asg: &mut Asg, + close_span: Span, + ) -> Result { self.map_obj(asg, |tpl| { tpl.map(|open_span: Span| { open_span.merge(close_span).unwrap_or(open_span) }) - }) + }); + + // TODO: Quick-and-dirty ignore of anonymous templates until + // following commits; + // those represent application and we'll need to present errors + // differently. + // This is temporary and will be formalized. + if self.name(asg).is_none() { + return Ok(self); + } + + let mut unused = self + .edges_filtered::(asg) + .filter(|oi| oi.is_bound_to_kind::(asg)) + .filter(|oi| !oi.is_referenced(asg)) + .filter_map(|oi| oi.resolve(asg).name()) // filters out abstract + .collect::>(); + + // Iterator above is not reversable and provides edge in the + // opposite order in which they were added + // (which would be the opposite order in which they were + // lexically defined in the source). + unused.reverse(); + + match unused.len() { + 0 => Ok(self), + _ => Err(AsgError::TplUnusedParams(self.name(asg), unused)), + } } /// Apply a named template `id` to the context of `self`. diff --git a/tamer/src/asg/graph/visit/ontree/test.rs b/tamer/src/asg/graph/visit/ontree/test.rs index f5dec397..af0965f5 100644 --- a/tamer/src/asg/graph/visit/ontree/test.rs +++ b/tamer/src/asg/graph/visit/ontree/test.rs @@ -196,32 +196,28 @@ fn traverses_ontological_tree_tpl_with_sibling_at_increasing_depth() { // unexpected way. #[test] fn traverses_ontological_tree_tpl_apply() { - let name_tpl = "_tpl-to-apply_".into(); - let id_tpl = SPair(name_tpl, S3); - let ref_tpl = SPair(name_tpl, S6); - let id_param = SPair("@param@".into(), S8); - let value_param = SPair("value".into(), S9); + let name_tpl = "_tpl-to-apply_"; #[rustfmt::skip] let toks = vec![ PkgStart(S1, SPair("/pkg".into(), S1)), // The template that will be applied. TplStart(S2), - BindIdent(id_tpl), // <-, + BindIdent(spair(name_tpl, S3)), // <-. // | // This test is light for now, // | // until we develop the ASG further. // | TplEnd(S4), // | // | - // Apply the above template. // | - TplStart(S5), // | - RefIdent(ref_tpl), // | - // | - MetaStart(S7), // | - BindIdent(id_param), // | - MetaLexeme(value_param), // | - MetaEnd(S10), // | - TplEndRef(S11), // notice the `Ref` at the end --' + // Apply the above template. // | a + TplStart(S5), // <-+--.n + RefIdent(spair(name_tpl, S6)), // --' |o + // |n + MetaStart(S7), // |y + BindIdent(spair("@param@", S8)), // |m + MetaLexeme(spair("value", S9)), // |o + MetaEnd(S10), // |u + TplEndRef(S11), // notice the `Ref` at the end -----'s PkgEnd(S12), ]; @@ -355,21 +351,28 @@ fn traverses_ontological_tree_complex_tpl_meta() { // -- Above this line was setup -- // MetaStart(S4), - BindIdent(spair("@param@", S5)), - - // It will be important to observe that ordering - // is respected during traversal, - // otherwise concatenation order will be wrong. - MetaLexeme(spair("foo", S6)), - RefIdent(spair("@other@", S7)), // --. - MetaLexeme(spair("bar", S8)), // | - MetaEnd(S9), // | - // | - MetaStart(S10), // | - BindIdent(spair("@other@", S11)), // <-' - MetaEnd(S12), - TplEnd(S13), - PkgEnd(S14), + BindIdent(spair("@param@", S5)), // <-. + // | + // It will be important to observe that ordering // | + // is respected during traversal, // | + // otherwise concatenation order will be wrong. // | + MetaLexeme(spair("foo", S6)), // | + RefIdent(spair("@other@", S7)), // --. // | + MetaLexeme(spair("bar", S8)), // | // | + MetaEnd(S9), // | // | + // | // | + MetaStart(S10), // | // | + BindIdent(spair("@other@", S11)), // <-' // | + MetaEnd(S12), // | + // | + // Metavariables must all be referenced. // | + // `@other@` is already referenced above, // | + // so only `@param@` is needed. // | + ExprStart(ExprOp::Sum, S13), // | + RefIdent(spair("@param@", S14)), // --' + ExprEnd(S15), + TplEnd(S16), + PkgEnd(S17), ]; // We need more concise expressions for the below table of values. @@ -378,18 +381,20 @@ fn traverses_ontological_tree_complex_tpl_meta() { #[rustfmt::skip] assert_eq!( - // A -|-> B | A span -|-> B span | espan | depth - vec![//-----|-------|-----------|------------|--------|----------------- - (d(Root, Pkg, SU, m(S1, S14), None ), Depth(1)), - (d(Pkg, Ident, m(S1, S14), S3, None ), Depth(2)), - (d(Ident, Tpl, S3, m(S2, S13), None ), Depth(3)), - (d(Tpl, Ident, m(S2, S13), S5, None ), Depth(4)), - (d(Ident, Meta, S5, m(S4, S9), None ), Depth(5)), - (d(Meta, Meta, m(S4, S9), S6, None ), Depth(6)), - /*cross*/ (d(Meta, Ident, m(S4, S9), S11, Some(S7)), Depth(6)), - (d(Meta, Meta, m(S4, S9), S8, None, ), Depth(6)), - (d(Tpl, Ident, m(S2, S13), S11, None ), Depth(4)), - (d(Ident, Meta, S11, m(S10, S12), None ), Depth(5)), + // A -|-> B | A span -|-> B span | espan | depth + vec![//-----|-------|-----------|------------|---------|----------------- + (d(Root, Pkg, SU, m(S1, S17), None ), Depth(1)), + (d(Pkg, Ident, m(S1, S17), S3, None ), Depth(2)), + (d(Ident, Tpl, S3, m(S2, S16), None ), Depth(3)), + (d(Tpl, Ident, m(S2, S16), S5, None ), Depth(4)), + (d(Ident, Meta, S5, m(S4, S9), None ), Depth(5)), + (d(Meta, Meta, m(S4, S9), S6, None ), Depth(6)), + /*cross*/ (d(Meta, Ident, m(S4, S9), S11, Some(S7) ), Depth(6)), + (d(Meta, Meta, m(S4, S9), S8, None, ), Depth(6)), + (d(Tpl, Ident, m(S2, S16), S11, None ), Depth(4)), + (d(Ident, Meta, S11, m(S10, S12), None ), Depth(5)), + (d(Tpl, Expr, m(S2, S16), m(S13, S15), None ), Depth(4)), + /*cross*/ (d(Expr, Ident, m(S13, S15),S5, Some(S14)), Depth(5)), ], tree_reconstruction_report(toks), ); @@ -439,9 +444,14 @@ fn tpl_header_source_order() { // definitions). ExprStart(ExprOp::Sum, S15), BindIdent(spair("sum", S16)), - ExprEnd(S17), - TplEnd(S18), - PkgEnd(S19), + + // Metavariables must be referenced. + RefIdent(spair("@param_before@", S17)), + RefIdent(spair("@param_after_a@", S18)), + RefIdent(spair("@param_after_b@", S19)), + ExprEnd(S20), + TplEnd(S21), + PkgEnd(S22), ]; // We need more concise expressions for the below table of values. @@ -450,22 +460,25 @@ fn tpl_header_source_order() { #[rustfmt::skip] assert_eq!( - // A -|-> B | A span -|-> B span | espan | depth - vec![//-----|-------|-----------|------------|--------|----------------- - (d(Root, Pkg, SU, m(S1, S19), None ), Depth(1)), - (d(Pkg, Ident, m(S1, S19), S3, None ), Depth(2)), - (d(Ident, Tpl, S3, m(S2, S18), None ), Depth(3)), - (d(Tpl, Ident, m(S2, S18), S5, None ), Depth(4)), - (d(Ident, Meta, S5, m(S4, S6), None ), Depth(5)), - // ,-----------------------------------------------------------------------, - (d(Tpl, Ident, m(S2, S18), S10, None ), Depth(4)), - (d(Ident, Meta, S10, m(S9, S11), None ), Depth(5)), - (d(Tpl, Ident, m(S2, S18), S13, None ), Depth(4)), - (d(Ident, Meta, S13, m(S12, S14), None ), Depth(5)), - // '-----------------------------------------------------------------------' - (d(Tpl, Expr, m(S2, S18), m(S7, S8), None ), Depth(4)), - (d(Tpl, Ident, m(S2, S18), S16, None ), Depth(4)), - (d(Ident, Expr, S16, m(S15, S17), None ), Depth(5)), + // A -|-> B | A span -|-> B span | espan | depth + vec![//-----|-------|-----------|------------|---------|----------------- + (d(Root, Pkg, SU, m(S1, S22), None ), Depth(1)), + (d(Pkg, Ident, m(S1, S22), S3, None ), Depth(2)), + (d(Ident, Tpl, S3, m(S2, S21), None ), Depth(3)), + (d(Tpl, Ident, m(S2, S21), S5, None ), Depth(4)), + (d(Ident, Meta, S5, m(S4, S6), None ), Depth(5)), + // ,-------------------------------------------------- ---------------------, + (d(Tpl, Ident, m(S2, S21), S10, None ), Depth(4)), + (d(Ident, Meta, S10, m(S9, S11), None ), Depth(5)), + (d(Tpl, Ident, m(S2, S21), S13, None ), Depth(4)), + (d(Ident, Meta, S13, m(S12, S14), None ), Depth(5)), + // '-------------------------------------------------- ---------------------' + (d(Tpl, Expr, m(S2, S21), m(S7, S8), None ), Depth(4)), + (d(Tpl, Ident, m(S2, S21), S16, None ), Depth(4)), + (d(Ident, Expr, S16, m(S15, S20), None ), Depth(5)), + /*cross*/ (d(Expr, Ident, m(S15, S20),S5, Some(S17)), Depth(6)), + /*cross*/ (d(Expr, Ident, m(S15, S20),S10, Some(S18)), Depth(6)), + /*cross*/ (d(Expr, Ident, m(S15, S20),S13, Some(S19)), Depth(6)), ], // ^ The enclosed Ident->Meta pairs above have been hoisted out of // the body and into the header of `Tpl`. diff --git a/tamer/src/nir/abstract_bind.rs b/tamer/src/nir/abstract_bind.rs index 981133af..137b57fa 100644 --- a/tamer/src/nir/abstract_bind.rs +++ b/tamer/src/nir/abstract_bind.rs @@ -119,6 +119,26 @@ //! [`Nir::BindIdentMeta`] be padded with a single `@` on each side, //! as shown in the examples above. //! +//! Documentation Strings +//! ===================== +//! This also handles translation of metavariable references in description +//! position. +//! For example: +//! +//! ```xml +//! +//! +//! ``` +//! +//! In this case, +//! to support the same convention as described above, +//! we want the lexeme `@bar@` to be interpreted as a _reference_ +//! (to a metavariable `@bar@`), +//! not a documentation string `"@bar@"`. +//! Instead, +//! the documentation ought to be derived at expansion-time from the +//! lexical value of the metavariable identified by `@bar@`. +//! //! Lowering Pipeline Ordering //! ========================== //! This module is an _optional_ syntactic feature of TAME. @@ -165,15 +185,22 @@ impl ParseState for AbstractBindTranslate { _: &mut Self::Context, ) -> TransitionResult { match tok { + // Abstract bindings (inferred) BindIdent(name) if needs_translation(name) => { Transition(self).ok(BindIdentAbstract(name)) } + // Forced-abstract bindings (explicit) BindIdentMeta(name) => validate_meta_name(name) .map(BindIdentMeta) .map(ParseStatus::Object) .transition(self), + // Abstract documentation (inferred) + Desc(clause) if needs_translation(clause) => { + Transition(self).ok(DescRef(clause)) + } + _ => Transition(self).ok(tok), } } diff --git a/tamer/src/nir/abstract_bind/test.rs b/tamer/src/nir/abstract_bind/test.rs index ada82c3d..91aa8563 100644 --- a/tamer/src/nir/abstract_bind/test.rs +++ b/tamer/src/nir/abstract_bind/test.rs @@ -162,6 +162,40 @@ fn rejects_metavariable_without_naming_convention() { ); } +// Similar concept, +// but for `Desc`. +// This is not an abstract bind, +// though, +// making this module a slight misnomer now. +#[test] +fn desc_with_only_metavar_naming_is_translated() { + // This is named as a metavariable. + let name = "@param@".into(); + + assert_tok_translate( + // This documentation string uses a metavariable naming + // convention... + Desc(SPair(name, S2)), + // ...and so is translated, + // so that it is interpreted as a reference rather than a + // documentation string. + DescRef(SPair(name, S2)), + ); +} + +#[test] +fn desc_without_metavar_naming_is_echoed() { + // This is named as a metavariable. + let s = "some string".into(); + + assert_tok_translate( + // This does _not_ follow a metavariable naming convention... + Desc(SPair(s, S2)), + // ...and so is echoed back verbatim. + Desc(SPair(s, S2)), + ); +} + fn assert_tok_translate(tok: Nir, expect: Nir) { #[rustfmt::skip] assert_eq!( diff --git a/tamer/tests/xmli/template/expected.xml b/tamer/tests/xmli/template/expected.xml index a85d5f58..0ea0835d 100644 --- a/tamer/tests/xmli/template/expected.xml +++ b/tamer/tests/xmli/template/expected.xml @@ -186,6 +186,11 @@ @@ -204,6 +209,11 @@ bar + + + + + diff --git a/tamer/tests/xmli/template/src.xml b/tamer/tests/xmli/template/src.xml index d803c694..6b7474c0 100644 --- a/tamer/tests/xmli/template/src.xml +++ b/tamer/tests/xmli/template/src.xml @@ -186,6 +186,11 @@ @@ -204,6 +209,11 @@ bar + + + + +