diff --git a/tamer/build-aux/asg-ontviz.awk b/tamer/build-aux/asg-ontviz.awk index 32637dc2..6bf776e6 100644 --- a/tamer/build-aux/asg-ontviz.awk +++ b/tamer/build-aux/asg-ontviz.awk @@ -32,6 +32,24 @@ # } # } # +# But edges also support arbitrary code definitions: +# +# object_rel! { +# Source -> { +# tree TargetA { +# fn pre_add_edge(...) { +# // ... +# } +# }, +# tree TargetB, +# cross TargetC, +# } +# } +# +# And because of that, +# this script hard-codes the expected level of nesting just to make life +# easier. +# # This script expects to receive a list of files containing such # definitions. # It will output, @@ -81,7 +99,8 @@ BEGINFILE { /^object_rel! {$/, /^}$/ { in_block = 1 } # `Foo -> {` line declares the source of the relation. -in_block && /->/ { +# We hard-code the expected depth to simplify parsing. +in_block && /^ \w+ -> \{$/ { block_src = $1 printf " # `%s` from `%s:%d`\n", block_src, FILENAME, FNR @@ -92,7 +111,7 @@ in_block && /->/ { # A closing curly brace always means that we've finished with the current # source relation, # since we're at the innermost level of nesting. -block_src && /}/ { +block_src && /^ }$/ { block_src = "" print "" } @@ -116,7 +135,12 @@ block_src && /^ *\/\/ empty$/ { # we must independently define each one. # But that's okay; # the output is quite legible. -block_src && $NF ~ /\w+,$/ { +block_src && /^ \w+ \w+(,| \{)$/ { + # Clean up the end of the string _before_ pulling information out of + # fields, + # since the number of fields can vary. + gsub(/(,| \{)$/, "") + # Edge type (cross, tree) ty = $(NF-1) @@ -139,8 +163,6 @@ block_src && $NF ~ /\w+,$/ { break; } - gsub(/,$/, "") - # This may need updating over time as object names in Rust sources # exceed the fixed-width definition here. # This output is intended to form a table that is easy to read and diff --git a/tamer/src/asg/air/test.rs b/tamer/src/asg/air/test.rs index 48ccdf98..8ea14618 100644 --- a/tamer/src/asg/air/test.rs +++ b/tamer/src/asg/air/test.rs @@ -30,7 +30,7 @@ use crate::{ span::dummy::*, }; -type Sut = AirAggregate; +pub type Sut = AirAggregate; use Air::*; use Parsed::Incomplete; @@ -686,16 +686,23 @@ fn resume_previous_parsing_context() { pub fn parse_as_pkg_body>( toks: I, ) -> Parser + Debug> +where + ::IntoIter: Debug, +{ + Sut::parse(as_pkg_body(toks)) +} + +pub fn as_pkg_body>( + toks: I, +) -> impl Iterator + Debug where ::IntoIter: Debug, { use std::iter; - Sut::parse( - iter::once(PkgStart(S1, spair("/incidental-pkg", S1))) - .chain(toks.into_iter()) - .chain(iter::once(PkgEnd(S1))), - ) + iter::once(PkgStart(S1, spair("/incidental-pkg", S1))) + .chain(toks.into_iter()) + .chain(iter::once(PkgEnd(S1))) } pub(super) fn asg_from_pkg_body_toks>( diff --git a/tamer/src/asg/air/tpl/test.rs b/tamer/src/asg/air/tpl/test.rs index d4562a76..ad997c9b 100644 --- a/tamer/src/asg/air/tpl/test.rs +++ b/tamer/src/asg/air/tpl/test.rs @@ -18,6 +18,7 @@ // along with this program. If not, see . use super::*; +use crate::asg::air::test::{as_pkg_body, Sut}; use crate::span::dummy::*; use crate::{ asg::{ @@ -333,18 +334,9 @@ fn tpl_holds_dangling_expressions() { BindIdent(id_tpl), // Dangling expression. - // This would be inlined at an application site, - // and so this changes the shape of the template. ExprStart(ExprOp::Sum, S3), ExprEnd(S4), - - // Dangling - // (TODO: This won't be valid; - // extract into separate test case to check for a new - // AsgError variant.) - ExprStart(ExprOp::Sum, S5), - ExprEnd(S6), - TplEnd(S7), + TplEnd(S5), ]; let ctx = air_ctx_from_pkg_body_toks(toks); @@ -353,12 +345,96 @@ fn tpl_holds_dangling_expressions() { let oi_tpl = pkg_expect_ident_oi::(&ctx, id_tpl); let tpl = oi_tpl.resolve(&asg); - // TODO: Until the above is invalid, - // the second is overwriting the first. - assert_eq!(TplShape::Expr(S5.merge(S6).unwrap()), tpl.shape()); + // The above `Expr` would be inlined at an application site, + // and so this changes the shape of the template. + assert_eq!(TplShape::Expr(S3.merge(S4).unwrap()), tpl.shape()); assert_eq!( - vec![S5.merge(S6).unwrap(), S3.merge(S4).unwrap(),], + vec![S3.merge(S4).unwrap()], + oi_tpl + .edges_filtered::(&asg) + .map(ObjectIndex::cresolve(&asg)) + .map(Expr::span) + .collect::>() + ); +} + +// As of TAMER, +// a new restriction is that templates can't just inline whatever they +// want into their expression expansion context. +#[test] +fn multi_dangling_invalid_expr_shape() { + let id_tpl = spair("_tpl_", S2); + + #[rustfmt::skip] + let toks = [ + TplStart(S1), + BindIdent(id_tpl), + + // We have an expression to be expanded inline. + // We cannot have another. + ExprStart(ExprOp::Sum, S3), + ExprEnd(S4), + + // ...but we're provided another! + // This should error. + ExprStart(ExprOp::Sum, S5), + ExprEnd(S6), + TplEnd(S7), + ]; + + let mut sut = Sut::parse(as_pkg_body(toks)); + + assert_eq!( + #[rustfmt::skip] + vec![ + Ok(Parsed::Incomplete), // PkgStart + Ok(Parsed::Incomplete), // TplStart + Ok(Parsed::Incomplete), // BindIdent + + // This one's okay. + Ok(Parsed::Incomplete), // ExprStart + Ok(Parsed::Incomplete), // ExprEnd + + // We start out okay, + // because an edge for a dangling expression is not added + // until after the expression ends + // (we have until then to bind an identifier). + Ok(Parsed::Incomplete), // ExprStart + // But the ending token will trigger the error. + Err(ParseError::StateError( + AsgError::TplShapeExprMulti( + Some(id_tpl), + S5.merge(S6).unwrap(), + S3.merge(S4).unwrap() + ) + )), + + // RECOVERY: We do not add the edge, + // but the object otherwise continues to exist on the + // graph, + // unreachable. + Ok(Parsed::Incomplete), // TplEnd + Ok(Parsed::Incomplete), // PkgEnd + ], + sut.by_ref().collect::>(), + ); + + let ctx = sut.finalize().unwrap().into_private_context(); + let asg = ctx.asg_ref(); + + let oi_tpl = pkg_expect_ident_oi::(&ctx, id_tpl); + let tpl = oi_tpl.resolve(&asg); + + // The error above should be evidence for this, + // but let's make sure the error didn't somehow cause the shape to + // change. + assert_eq!(TplShape::Expr(S3.merge(S4).unwrap()), tpl.shape()); + + // The template should have only one inline expression; + // it should have discarded the other. + assert_eq!( + vec![S3.merge(S4).unwrap()], oi_tpl .edges_filtered::(&asg) .map(ObjectIndex::cresolve(&asg)) diff --git a/tamer/src/asg/error.rs b/tamer/src/asg/error.rs index 7d383e30..a8bd129d 100644 --- a/tamer/src/asg/error.rs +++ b/tamer/src/asg/error.rs @@ -167,8 +167,26 @@ pub enum AsgError { /// The provided [`Span`] indicates the location of the start of the /// metavariable definition. UnexpectedMeta(Span), + + /// A template already has a shape of + /// [`TplShape::Expr`](super::graph::object::tpl::TplShape::Expr), + /// but another expression was found that would violate this shape + /// constraint. + /// + /// The template name is provided if it is known at the time of the + /// error. + TplShapeExprMulti(Option, ErrorOccurrenceSpan, FirstOccurrenceSpan), } +/// A [`Span`] representing the subject of this error. +type ErrorOccurrenceSpan = Span; + +/// A [`Span`] representing the first occurrence of an object related to the +/// subject of this error. +/// +/// This should be paired with [`ErrorOccurrenceSpan`]. +type FirstOccurrenceSpan = Span; + impl Display for AsgError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { use AsgError::*; @@ -250,6 +268,18 @@ impl Display for AsgError { UnexpectedMeta(_) => { write!(f, "unexpected metavariable definition") } + + TplShapeExprMulti(Some(name), _, _) => write!( + f, + "definition of template {} would produce multiple inline \ + expressions when expanded", + TtQuote::wrap(name), + ), + TplShapeExprMulti(None, _, _) => write!( + f, + "template definition would produce multiple inline \ + expressions when expanded" + ), } } } @@ -445,6 +475,42 @@ impl Diagnostic for AsgError { span.help("metavariables are expected to occur in a template context"), ] } + + // TODO: Perhaps we should have a documentation page that can be + // referenced with examples and further rationale, + // like Rust does. + TplShapeExprMulti(oname, err, first) => oname + .map(|name| name.note("for this template")) + .into_iter() + .chain(vec![ + first.note( + "this is the first expression that would be inlined \ + at an expansion site", + ), + err.error( + "template cannot inline more than one expression \ + into an expansion site", + ), + err.help( + "this restriction is intended to ensure that \ + templates expand in ways that are consistent \ + given any expansion context; consider either:", + ), + err.help( + " - wrapping both expressions in a parent \ + expression; or", + ), + err.help( + " - giving at least one expression an id to prevent \ + inlining", + ), + // in case they were wondering + err.help( + "this restriction did not exist in versions of \ + TAME prior to TAMER", + ), + ]) + .collect(), } } } diff --git a/tamer/src/asg/graph/object/tpl.rs b/tamer/src/asg/graph/object/tpl.rs index e81b97ff..feecee87 100644 --- a/tamer/src/asg/graph/object/tpl.rs +++ b/tamer/src/asg/graph/object/tpl.rs @@ -176,7 +176,22 @@ object_rel! { commit: impl FnOnce(&mut Asg), ) -> Result<(), AsgError> { let span = to_oi.resolve(asg).span(); - from_oi.map_obj(asg, |tpl| tpl.overwrite(TplShape::Expr(span))); + let tpl_name = from_oi + .ident(asg) + .and_then(|oi| oi.resolve(asg).name()); + + from_oi.try_map_obj(asg, |tpl| match tpl.shape() { + TplShape::Expr(first) => { + Err(( + tpl, + AsgError::TplShapeExprMulti(tpl_name, span, first) + )) + }, + + TplShape::Unknown | TplShape::Empty => { + Ok(tpl.overwrite(TplShape::Expr(span))) + }, + })?; Ok(commit(asg)) } diff --git a/tamer/tests/xmli/template/expected.xml b/tamer/tests/xmli/template/expected.xml index 3e725c2c..a85d5f58 100644 --- a/tamer/tests/xmli/template/expected.xml +++ b/tamer/tests/xmli/template/expected.xml @@ -20,19 +20,11 @@ - - - - @@ -100,9 +92,9 @@