tamer: asg::air::object::tpl: Reject multi-expression shape
This enforces the new constraint that templates expanding into an `Expr` context must only inline a single `Expr`. Perhaps in the future we'll support explicit splicing, like `,@` in Lisp. But this new restriction is intended for two purposes: - To make templates more predictable (if you have a list of expressions inlined then they will act differently depending on the type of expression that they are inlined into, which means that more defensive programming would otherwise be required); and - To make expansion easier, since we're going to have to set aside an expansion workspace ahead of time to ensure ordering (Petgraph can't replace edges in-place). If we support multi-expansion, we'd have to handle associativity in all expression contexts. This'll become more clear in future commits. It's nice to see all this hard work coming together now, though; it's easy now to perform static analysis on the system, and any part of the graph construction can throw errors with rich diagnostic information and still recover properly. And, importantly, the system enforces its own state, and the compiler helps us with that (the previous commits). DEV-13163main
parent
aec721f4fa
commit
c19ecba6ef
|
@ -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
|
||||
|
|
|
@ -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<I: IntoIterator<Item = Air>>(
|
||||
toks: I,
|
||||
) -> Parser<Sut, impl Iterator<Item = Air> + Debug>
|
||||
where
|
||||
<I as IntoIterator>::IntoIter: Debug,
|
||||
{
|
||||
Sut::parse(as_pkg_body(toks))
|
||||
}
|
||||
|
||||
pub fn as_pkg_body<I: IntoIterator<Item = Air>>(
|
||||
toks: I,
|
||||
) -> impl Iterator<Item = Air> + Debug
|
||||
where
|
||||
<I as IntoIterator>::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<I: IntoIterator<Item = Air>>(
|
||||
|
|
|
@ -18,6 +18,7 @@
|
|||
// along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
|
||||
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::<Tpl>(&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::<Expr>(&asg)
|
||||
.map(ObjectIndex::cresolve(&asg))
|
||||
.map(Expr::span)
|
||||
.collect::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
|
||||
// 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::<Vec<_>>(),
|
||||
);
|
||||
|
||||
let ctx = sut.finalize().unwrap().into_private_context();
|
||||
let asg = ctx.asg_ref();
|
||||
|
||||
let oi_tpl = pkg_expect_ident_oi::<Tpl>(&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::<Expr>(&asg)
|
||||
.map(ObjectIndex::cresolve(&asg))
|
||||
|
|
|
@ -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<SPair>, 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(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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))
|
||||
}
|
||||
|
|
|
@ -20,19 +20,11 @@
|
|||
<c:sum>
|
||||
<c:product />
|
||||
</c:sum>
|
||||
|
||||
<c:product>
|
||||
<c:sum />
|
||||
</c:product>
|
||||
</template>
|
||||
|
||||
<template name="_with-static-mix_"
|
||||
desc="Both identified and unidentified that may or may
|
||||
not be reachable in expansion context">
|
||||
<c:sum>
|
||||
<c:product />
|
||||
</c:sum>
|
||||
|
||||
<c:product>
|
||||
<c:sum />
|
||||
</c:product>
|
||||
|
@ -48,9 +40,9 @@
|
|||
|
||||
<rate yields="tplStaticMix" />
|
||||
|
||||
<c:sum>
|
||||
<rate yields="tplStaticMixEnd">
|
||||
<c:product />
|
||||
</c:sum>
|
||||
</rate>
|
||||
</template>
|
||||
|
||||
|
||||
|
@ -100,9 +92,9 @@
|
|||
|
||||
<template name="_short-hand-nullary-body_" desc="Nullary with body" />
|
||||
<apply-template name="_short-hand-nullary-body_">
|
||||
<with-param name="@values@" value="___dsgr-bfe___" />
|
||||
<with-param name="@values@" value="___dsgr-bb5___" />
|
||||
</apply-template>
|
||||
<template name="___dsgr-bfe___"
|
||||
<template name="___dsgr-bb5___"
|
||||
desc="Desugared body of shorthand template application of `_short-hand-nullary-body_`">
|
||||
<c:product>
|
||||
<c:sum />
|
||||
|
@ -113,9 +105,9 @@
|
|||
<apply-template name="_short-hand-nary-body_">
|
||||
<with-param name="@bar@" value="baz" />
|
||||
<with-param name="@baz@" value="quux" />
|
||||
<with-param name="@values@" value="___dsgr-cb5___" />
|
||||
<with-param name="@values@" value="___dsgr-c6c___" />
|
||||
</apply-template>
|
||||
<template name="___dsgr-cb5___"
|
||||
<template name="___dsgr-c6c___"
|
||||
desc="Desugared body of shorthand template application of `_short-hand-nary-body_`">
|
||||
<c:sum>
|
||||
<c:product />
|
||||
|
@ -125,9 +117,9 @@
|
|||
<template name="_short-hand-nullary-outer_"
|
||||
desc="Outer template holding an inner" />
|
||||
<apply-template name="_short-hand-nullary-outer_">
|
||||
<with-param name="@values@" value="___dsgr-d99___" />
|
||||
<with-param name="@values@" value="___dsgr-d50___" />
|
||||
</apply-template>
|
||||
<template name="___dsgr-d99___"
|
||||
<template name="___dsgr-d50___"
|
||||
desc="Desugared body of shorthand template application of `_short-hand-nullary-outer_`">
|
||||
<template name="_short-hand-nullary-inner-dfn-inner_"
|
||||
desc="Inner template applied inner" />
|
||||
|
@ -137,9 +129,9 @@
|
|||
<template name="_short-hand-nullary-inner-dfn-outer_"
|
||||
desc="Define template outer but apply inner" />
|
||||
<apply-template name="_short-hand-nullary-outer_">
|
||||
<with-param name="@values@" value="___dsgr-eed___" />
|
||||
<with-param name="@values@" value="___dsgr-ea4___" />
|
||||
</apply-template>
|
||||
<template name="___dsgr-eed___"
|
||||
<template name="___dsgr-ea4___"
|
||||
desc="Desugared body of shorthand template application of `_short-hand-nullary-outer_`">
|
||||
<apply-template name="_short-hand-nullary-inner-dfn-outer_" />
|
||||
</template>
|
||||
|
@ -148,9 +140,9 @@
|
|||
desc="Unary with body" />
|
||||
<apply-template name="_short-hand-unary-with-body_">
|
||||
<with-param name="@foo@" value="bar" />
|
||||
<with-param name="@values@" value="___dsgr-fb4___" />
|
||||
<with-param name="@values@" value="___dsgr-f6b___" />
|
||||
</apply-template>
|
||||
<template name="___dsgr-fb4___"
|
||||
<template name="___dsgr-f6b___"
|
||||
desc="Desugared body of shorthand template application of `_short-hand-unary-with-body_`">
|
||||
<template name="_short-hand-unary-with-body-inner_"
|
||||
desc="Inner template" />
|
||||
|
|
|
@ -20,19 +20,11 @@
|
|||
<c:sum>
|
||||
<c:product />
|
||||
</c:sum>
|
||||
|
||||
<c:product>
|
||||
<c:sum />
|
||||
</c:product>
|
||||
</template>
|
||||
|
||||
<template name="_with-static-mix_"
|
||||
desc="Both identified and unidentified that may or may
|
||||
not be reachable in expansion context">
|
||||
<c:sum>
|
||||
<c:product />
|
||||
</c:sum>
|
||||
|
||||
<c:product> <!-- depth N -->
|
||||
<c:sum /> <!-- depth N+1 -->
|
||||
</c:product>
|
||||
|
@ -48,9 +40,9 @@
|
|||
|
||||
<rate yields="tplStaticMix" /> <!-- begins at depth N+1 -->
|
||||
|
||||
<c:sum>
|
||||
<rate yields="tplStaticMixEnd">
|
||||
<c:product />
|
||||
</c:sum>
|
||||
</rate>
|
||||
</template>
|
||||
|
||||
Short-hand template application.
|
||||
|
|
Loading…
Reference in New Issue