employer
/
tame
1
0
Fork 0
Code Packages Releases Activity

Revert "tamer: nir::desugar::interp: Include attribute name in derived param name" 

Also: Revert "tamer: nir::desugar::interp: Token {SPair=>Attr}"

This reverts commit 7fd60d6cdafaedc19642a3f10dfddfa7c7ae8f53.
This reverts commit 12a008c66414c3d628097e503a98c80687e3c088.

This has been quite a tortured experience, trying to figure out how to best
fit desugaring into the existing system.  The truth is that it ultimately
failed because I was not sticking with my intuition---I was trying to get
things out quickly by compromising on the design, and in the end, it saved
me nothing.

But I wouldn't say that it was a waste of time---the path was a dead end,
but it was full of experiences.

More to come, but interpolation is back to operating on NIR directly, and I
chose to treat it as a source-to-source mapping and not represent it using
the type system---interpolation can be an optional feature when writing TAME
frontends (the principal one being the XML-based one), and it's up to later
checks to assert that identifiers match a given domain.

I am disappointed by the additional context we lose here, but that can
always be introduced in the future differently, e.g. by maintaining a
dictionary of additional context for spans that can be later referenced for
diagnostic purposes.  But let's worry about that in the future; it doesn't
make sense to further complicate IRs for such a thing.

DEV-13346
main
Mike Gerwitz 2022-11-28 10:35:21 -05:00
parent 9da6cb439f
commit 76beb117f9
3 changed files with 97 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
tamer/src/nir/desugar/interp.rs
View File

@ -17,7 +17,7 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.
//! Interpolation parser for desugaring attributes for NIR.
//! Interpolation parser for desugaring NIR.
//!
//! String interpolation occurs for attributes containing curly braces
//! (`{` and `}`)
@ -103,17 +103,16 @@ use super::super::{PlainNir, PlainNirSymbol};
use crate::{
diagnose::{AnnotatedSpan, Diagnostic},
fmt::{DisplayWrapper, TtQuote},
parse::{prelude::*, util::expand::Expansion, NoContext},
parse::{
prelude::*,
util::{expand::Expansion, SPair},
NoContext,
},
span::Span,
sym::{
st::quick_contains_byte, GlobalSymbolIntern, GlobalSymbolResolve,
SymbolId,
},
xir::{
attr::{Attr, AttrSpan},
fmt::XmlAttr,
QName,
},
};
use std::{error::Error, fmt::Display};
@ -209,8 +208,8 @@ impl Display for InterpState {
}
impl ParseState for InterpState {
type Token = Attr;
type Object = Expansion<Self::Token, PlainNir>;
type Token = SPair;
type Object = Expansion<SPair, PlainNir>;
type Error = InterpError;
fn parse_token(
@ -218,9 +217,7 @@ impl ParseState for InterpState {
tok: Self::Token,
_: NoContext,
) -> TransitionResult<Self> {
let Attr(attr_qname, sym, AttrSpan(_, span)) = tok;
match self {
match (self, tok.into()) {
// When receiving a new symbol,
// we must make a quick determination as to whether it
// requires desugaring.
@ -230,26 +227,24 @@ impl ParseState for InterpState {
// filter out non-interpolated strings quickly,
// before we start to parse.
// Symbols that require no interpoolation are simply echoed back.
Ready => {
(Ready, (sym, span)) => {
if needs_interpolation(sym) {
Self::begin_expansion(attr_qname, sym, span)
.with_lookahead(tok)
Self::begin_expansion(sym, span)
} else {
// No desugaring is needed.
Self::yield_tok(tok)
Self::yield_symbol(sym, span)
}
}
// The outermost parsing context is that of the literal,
// where a sequence of characters up to `{` stand for
// themselves.
ParseLiteralAt(s, gen_param, offset) => {
(ParseLiteralAt(s, gen_param, offset), (sym, span)) => {
if offset == s.len() {
// We've reached the end of the specification string.
// Since we're in the outermost (literal) context,
// we're safe to complete.
return Self::end_expansion(s, gen_param, span)
.with_lookahead(tok);
return Self::end_expansion(s, gen_param, sym, span);
}
// Note that this is the position _relative to the offset_,
@ -261,7 +256,7 @@ impl ParseState for InterpState {
Some(0) => {
Transition(ParseInterpAt(s, gen_param, offset + 1))
.incomplete()
.with_lookahead(tok)
.with_lookahead((sym, span).into())
}
// Everything from the offset until the curly brace is a
@ -278,7 +273,7 @@ impl ParseState for InterpState {
Transition(ParseInterpAt(s, gen_param, end + 1))
.ok(Expanded(text))
.with_lookahead(tok)
.with_lookahead((sym, span).into())
}
// The remainder of the specification is a literal.
@ -294,7 +289,7 @@ impl ParseState for InterpState {
// we'll complete parsing next pass.
Transition(ParseLiteralAt(s, gen_param, s.len()))
.ok(Expanded(text))
.with_lookahead(tok)
.with_lookahead((sym, span).into())
}
}
}
@ -304,7 +299,7 @@ impl ParseState for InterpState {
// This is an inner context that cannot complete without being
// explicitly closed,
// and cannot not be nested.
ParseInterpAt(s, gen_param, offset) => {
(ParseInterpAt(s, gen_param, offset), (sym, span)) => {
// TODO: Make sure offset exists, avoid panic
// TODO: Prevent nested `{`.
@ -331,7 +326,7 @@ impl ParseState for InterpState {
// back in a literal context.
Transition(ParseLiteralAt(s, gen_param, end + 1))
.ok(Expanded(param_value))
.with_lookahead(tok)
.with_lookahead((sym, span).into())
}
None => todo!("missing closing '}}'"),
@ -343,8 +338,8 @@ impl ParseState for InterpState {
// (the interpolation specification)
// with a metavariable referencing the parameter that we just
// generated.
FinishSym(_, GenIdentSymbolId(gen_param)) => {
Self::yield_tok(tok.replace_value_derived(gen_param))
(FinishSym(_, GenIdentSymbolId(gen_param)), (_, span)) => {
Self::yield_symbol(gen_param, span)
}
}
}
@ -367,8 +362,8 @@ impl InterpState {
///
/// This transitions back to [`Ready`] and finally releases the
/// lookahead symbol.
fn yield_tok(tok: Attr) -> TransitionResult<Self> {
Transition(Ready).ok(DoneExpanding(tok))
fn yield_symbol(sym: SymbolId, span: Span) -> TransitionResult<Self> {
Transition(Ready).ok(DoneExpanding((sym, span).into()))
}
/// Begin expansion of an interpolation specification by generating a
@ -376,11 +371,7 @@ impl InterpState {
///
/// For more information on identifier generation,
/// see [`gen_tpl_param_ident_at_offset`].
fn begin_expansion(
attr_qname: QName,
sym: SymbolId,
span: Span,
) -> TransitionResult<Self> {
fn begin_expansion(sym: SymbolId, span: Span) -> TransitionResult<Self> {
let gen_param = gen_tpl_param_ident_at_offset(span);
// Description is not interned since there's no use in
@ -389,9 +380,8 @@ impl InterpState {
// (it's just informative for a human).
// Note that this means that tests cannot compare SymbolId.
let gen_desc = format!(
"Generated from interpolated string {s} for attribute {fmt_attr}",
s = TtQuote::wrap(sym),
fmt_attr = XmlAttr::wrap(attr_qname),
"Generated from interpolated string {}",
TtQuote::wrap(sym)
)
.clone_uninterned();
@ -407,6 +397,7 @@ impl InterpState {
// prefixes.
Transition(ParseLiteralAt(sym.lookup_str(), gen_param, 0))
.ok(Expanded(open))
.with_lookahead((sym, span).into())
}
/// Complete expansion of an interpolation specification string.
@ -416,6 +407,7 @@ impl InterpState {
fn end_expansion(
s: SpecSlice,
gen_param: GenIdentSymbolId,
sym: SymbolId,
span: Span,
) -> TransitionResult<Self> {
let close = PlainNir::TplParamClose(span);
@ -424,7 +416,9 @@ impl InterpState {
// which is to perform the final replacement of the original
// symbol that we've been fed
// (the specification string).
Transition(FinishSym(s, gen_param)).ok(Expanded(close))
Transition(FinishSym(s, gen_param))
.ok(Expanded(close))
.with_lookahead((sym, span).into())
}
}
@ -473,18 +467,6 @@ fn needs_interpolation(val: SymbolId) -> bool {
/// and serves as a unique string that can be used to track down this code
/// that generates it.
///
/// TODO: Ideally this would also contain the name of the attribute from
/// which it is derived,
/// but at the time of writing,
/// the system is not far enough along to ensure that the domain of
/// possible values for attributes is a subset of template param
/// names,
/// and that XIR properly enforces attribute names via its
/// underlying third-party reader.
/// But until then,
/// the generated description _is_ safe to contain the attribute name
/// and can be used as a guide for debugging.
///
/// Hygiene is not a concern since identifiers cannot be redeclared,
/// so conflicts with manually-created identifiers will result in a
/// compilation error

107
tamer/src/nir/desugar/interp/test.rs
View File

@ -19,7 +19,6 @@
use super::*;
use crate::{
convert::ExpectInto,
nir::PlainNirSymbol,
parse::Parsed,
span::dummy::{DUMMY_CONTEXT as DC, *},
@ -40,13 +39,11 @@ fn does_not_desugar_literal_only() {
// `@bar@` is a metavariable,
// but it's also a literal because it's not enclosed in braces.
for literal in ["foo", "@bar@"] {
let name = "foo".unwrap_into();
let sym = literal.into();
let toks = vec![Attr::new(name, sym, (S1, S2))];
let toks = vec![SPair(sym, S1)];
// Attr should be unchanged.
assert_eq!(
Ok(vec![Object(DoneExpanding(Attr::new(name, sym, (S1, S2))))]),
Ok(vec![Object(DoneExpanding(SPair(sym, S1)))]),
Sut::parse(toks.into_iter()).collect(),
"literal `{literal}` must not desugar",
);
@ -72,12 +69,11 @@ fn desugars_literal_with_ending_var() {
let b = DC.span(10, 3);
let c = DC.span(14, 5);
let name = "foo".unwrap_into();
let given_sym = Attr::new(name, given_val.into(), (S1, a));
let given_sym = SPair(given_val.into(), a);
let toks = vec![given_sym];
let GenIdentSymbolId(expect_pname) = gen_tpl_param_ident_at_offset(a);
let expect_dfn = PlainNirSymbol::Todo(expect_pname.into(), a);
let GenIdentSymbolId(expect_name) = gen_tpl_param_ident_at_offset(a);
let expect_dfn = PlainNirSymbol::Todo(expect_name.into(), a);
let expect_text = PlainNirSymbol::Todo("foo".into(), b);
let expect_param = PlainNirSymbol::Todo("@bar@".into(), c);
@ -125,13 +121,10 @@ fn desugars_literal_with_ending_var() {
// we replace the original provided attribute
// (the interpolation specification)
// with a metavariable reference to the generated parameter.
assert_eq!(
assert_matches!(
sut.next(),
Some(Ok(Object(DoneExpanding(Attr::new(
name,
expect_pname,
(S1, a)
)))))
Some(Ok(Object(DoneExpanding(SPair(given_replace, given_span)))))
if given_replace == expect_name && given_span == a
);
assert_eq!(sut.next(), None);
@ -155,12 +148,11 @@ fn desugars_var_with_ending_literal() {
let b = DC.span(21, 5);
let c = DC.span(27, 3);
let name = "foo".unwrap_into();
let given_sym = Attr::new(name, given_val.into(), (S1, a));
let given_sym = SPair(given_val.into(), a);
let toks = vec![given_sym];
let GenIdentSymbolId(expect_pname) = gen_tpl_param_ident_at_offset(a);
let expect_dfn = PlainNirSymbol::Todo(expect_pname.into(), a);
let GenIdentSymbolId(expect_name) = gen_tpl_param_ident_at_offset(a);
let expect_dfn = PlainNirSymbol::Todo(expect_name.into(), a);
let expect_param = PlainNirSymbol::Todo("@foo@".into(), b);
let expect_text = PlainNirSymbol::Todo("bar".into(), c);
@ -181,14 +173,27 @@ fn desugars_var_with_ending_literal() {
);
assert_eq!(
Ok(vec![
Object(Expanded(PlainNir::TplParamValue(expect_param))),
Object(Expanded(PlainNir::TplParamText(expect_text))),
Object(Expanded(PlainNir::TplParamClose(a))),
Object(DoneExpanding(Attr::new(name, expect_pname, (S1, a)))),
]),
sut.collect(),
sut.next(),
Some(Ok(Object(Expanded(PlainNir::TplParamValue(expect_param))))),
);
assert_eq!(
sut.next(),
Some(Ok(Object(Expanded(PlainNir::TplParamText(expect_text)))))
);
assert_eq!(
sut.next(),
Some(Ok(Object(Expanded(PlainNir::TplParamClose(a)))))
);
assert_matches!(
sut.next(),
Some(Ok(Object(DoneExpanding(SPair(given_replace, given_span)))))
if given_replace == expect_name && given_span == a
);
assert_eq!(sut.next(), None);
}
// Combination of the above two tests.
@ -210,12 +215,11 @@ fn desugars_many_vars_and_literals() {
let d = DC.span(40, 3);
let e = DC.span(44, 6);
let name = "foo".unwrap_into();
let given_sym = Attr::new(name, given_val.into(), (S1, a));
let given_sym = SPair(given_val.into(), a);
let toks = vec![given_sym];
let GenIdentSymbolId(expect_pname) = gen_tpl_param_ident_at_offset(a);
let expect_dfn = PlainNirSymbol::Todo(expect_pname.into(), a);
let GenIdentSymbolId(expect_name) = gen_tpl_param_ident_at_offset(a);
let expect_dfn = PlainNirSymbol::Todo(expect_name.into(), a);
let expect_text1 = PlainNirSymbol::Todo("foo".into(), b);
let expect_param1 = PlainNirSymbol::Todo("@bar@".into(), c);
let expect_text2 = PlainNirSymbol::Todo("baz".into(), d);
@ -248,11 +252,22 @@ fn desugars_many_vars_and_literals() {
// offsets.
Object(Expanded(PlainNir::TplParamText(expect_text2))),
Object(Expanded(PlainNir::TplParamValue(expect_param2))),
Object(Expanded(PlainNir::TplParamClose(a))),
Object(DoneExpanding(Attr::new(name, expect_pname, (S1, a)))),
]),
sut.collect(),
sut.by_ref().take(4).collect(),
);
assert_eq!(
sut.next(),
Some(Ok(Object(Expanded(PlainNir::TplParamClose(a)))))
);
assert_matches!(
sut.next(),
Some(Ok(Object(DoneExpanding(SPair(given_replace, given_span)))))
if given_replace == expect_name && given_span == a
);
assert_eq!(sut.next(), None);
}
// Adjacent vars with empty literal between them.
@ -271,12 +286,11 @@ fn desugars_adjacent_interpolated_vars() {
let c = DC.span(48, 5);
let d = DC.span(55, 5);
let name = "hopefullyuniqattrname".unwrap_into();
let given_sym = Attr::new(name, given_val.into(), (S1, a));
let given_sym = SPair(given_val.into(), a);
let toks = vec![given_sym];
let GenIdentSymbolId(expect_pname) = gen_tpl_param_ident_at_offset(a);
let expect_dfn = PlainNirSymbol::Todo(expect_pname.into(), a);
let GenIdentSymbolId(expect_name) = gen_tpl_param_ident_at_offset(a);
let expect_dfn = PlainNirSymbol::Todo(expect_name.into(), a);
let expect_param1 = PlainNirSymbol::Todo("@foo@".into(), b);
let expect_param2 = PlainNirSymbol::Todo("@bar@".into(), c);
let expect_param3 = PlainNirSymbol::Todo("@baz@".into(), d);
@ -294,18 +308,29 @@ fn desugars_adjacent_interpolated_vars() {
PlainNirSymbol::Todo(desc_str, desc_span)
))))) if dfn == expect_dfn
&& desc_str.lookup_str().contains(given_val)
&& desc_str.lookup_str().contains(&name.to_string())
&& desc_span == a
);
// These are the three adjacent vars.
assert_eq!(
Ok(vec![
Object(Expanded(PlainNir::TplParamValue(expect_param1))),
Object(Expanded(PlainNir::TplParamValue(expect_param2))),
Object(Expanded(PlainNir::TplParamValue(expect_param3))),
Object(Expanded(PlainNir::TplParamClose(a))),
Object(DoneExpanding(Attr::new(name, expect_pname, (S1, a)))),
]),
sut.collect(),
sut.by_ref().take(3).collect(),
);
assert_eq!(
sut.next(),
Some(Ok(Object(Expanded(PlainNir::TplParamClose(a)))))
);
assert_matches!(
sut.next(),
Some(Ok(Object(DoneExpanding(SPair(given_replace, given_span)))))
if given_replace == expect_name && given_span == a
);
assert_eq!(sut.next(), None);
}

14
tamer/src/xir/attr.rs
View File

@ -157,20 +157,6 @@ impl Attr {
Attr(.., span) => span,
}
}
/// Replace the attribute's value with a new one that has been derived
/// from the original.
///
/// _This does not update the value span!_
/// The intent is for this operation to be used during term rewriting,
/// where the replaced value is _derived from_ the original and so
/// should retain the original span to provide the proper context to
/// the user.
pub fn replace_value_derived(self, value_new: SymbolId) -> Self {
match self {
Self(name, _, span) => Self(name, value_new, span),
}
}
}
impl Token for Attr {