From 85b08eb45ec2b5c56e40216b8ac90851acab619d Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 18 Jul 2023 11:17:51 -0400 Subject: [PATCH] tamer: nir::interp: Do not include original specification in generated desc This is a really obvious problem in retrospect, which makes me feel rather silly. The output was useful, but I don't have time to deal with this any further right now. The comments in the commit explain the problem---that the output ends up being interpolated as part of the fixpoint test, in an incorrect context, and so the code that we generate is invalid. Also goes to show why the fixpoint tests are important. (Yes, they're still disabled for meta-interp, I'm trying to get them enabled.) DEV-13163 --- tamer/src/nir/interp.rs | 36 +++++++++++++---------- tamer/src/nir/interp/test.rs | 29 ++++++++---------- tamer/src/sym/prefill.rs | 2 ++ tamer/tests/xmli/meta-interp/expected.xml | 16 +++++----- 4 files changed, 44 insertions(+), 39 deletions(-) diff --git a/tamer/src/nir/interp.rs b/tamer/src/nir/interp.rs index 98104ebf..cc8d6bc8 100644 --- a/tamer/src/nir/interp.rs +++ b/tamer/src/nir/interp.rs @@ -41,7 +41,7 @@ //! //! ```xml //! +//! desc="Generated from interpolated string"> //! foo //! //! baz @@ -69,6 +69,12 @@ //! then it is interpreted as a literal within the context of the template //! system and is echoed back unchanged. //! +//! There is currently no way to escape `{` within a string. +//! Such a feature will be considered in the future, +//! but for the meantime, +//! this can be worked around by using metavariables that expand into the +//! desired literal. +//! //! Desugared Spans //! --------------- //! [`Span`]s for the generated tokens are derived from the specification @@ -107,8 +113,8 @@ use crate::{ parse::{prelude::*, util::SPair, NoContext}, span::Span, sym::{ - st::quick_contains_byte, GlobalSymbolIntern, GlobalSymbolResolve, - SymbolId, + st::{quick_contains_byte, raw::S_GEN_FROM_INTERP}, + GlobalSymbolIntern, GlobalSymbolResolve, SymbolId, }, }; use std::{error::Error, fmt::Display}; @@ -286,25 +292,25 @@ impl ParseState for InterpState { .with_lookahead(tok) } + // Note: This historically generated a description containing + // the interpolated string, + // which was useful when looking at generated code. + // But this ends up producing output that is not a fixpoint, + // because if you run it back through the compiler, + // it needs interpolation again, + // but now in an incorrect context. + // We can revisit this + // (see commit introducing this comment) + // when we introduce escaping of some form, + // if it's worth doing. GenDesc(sym, gen_ident) => { let s = sym.lookup_str(); - // Description is not interned since there's no use in - // wasting time hashing something that will not be - // referenced - // (it's just informative for a human). - // Note that this means that tests cannot compare SymbolId. - let gen_desc = format!( - "Generated from interpolated string {}", - TtQuote::wrap(s) - ) - .clone_uninterned(); - // Begin parsing in a _literal_ context, // since interpolation is most commonly utilized with literal // prefixes. Transition(ParseLiteralAt(s, gen_ident, 0)) - .ok(Nir::Desc(SPair(gen_desc, span))) + .ok(Nir::Desc(SPair(S_GEN_FROM_INTERP, span))) .with_lookahead(tok) } diff --git a/tamer/src/nir/interp/test.rs b/tamer/src/nir/interp/test.rs index 2ef87562..a205bdc9 100644 --- a/tamer/src/nir/interp/test.rs +++ b/tamer/src/nir/interp/test.rs @@ -22,7 +22,6 @@ use crate::{ nir::NirEntity, parse::{Parsed, ParsedResult, Parser}, span::dummy::{DUMMY_CONTEXT as DC, *}, - sym::GlobalSymbolResolve, }; use std::assert_matches::assert_matches; use Parsed::*; @@ -81,7 +80,6 @@ fn does_not_desugar_text() { fn expect_expanded_header( sut: &mut Parser>, - given_val: &str, span: Span, ) -> SymbolId { let GenIdentSymbolId(expect_name) = gen_tpl_param_ident_at_offset(span); @@ -103,9 +101,8 @@ fn expect_expanded_header( ); assert_matches!( sut.next(), - Some(Ok(Object(Nir::Desc(SPair(desc_str, desc_span))))) - if desc_str.lookup_str().contains(given_val) - && desc_span == span + Some(Ok(Object(Nir::Desc(SPair(S_GEN_FROM_INTERP, desc_span))))) + if desc_span == span ); expect_name_sym @@ -133,7 +130,7 @@ fn desugars_spec_with_only_var() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( Ok(vec![ @@ -180,7 +177,7 @@ fn concrete_bind_ident_desugars_into_abstract_bind_after_interpolation() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( Ok(vec![ @@ -222,7 +219,7 @@ fn desugars_literal_with_ending_var() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( Ok(vec![ @@ -269,7 +266,7 @@ fn desugars_var_with_ending_literal() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( Ok(vec![ @@ -306,7 +303,7 @@ fn desugars_many_vars_and_literals() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( Ok(vec![ @@ -356,7 +353,7 @@ fn proper_multibyte_handling() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( Ok(vec![ @@ -397,7 +394,7 @@ fn desugars_adjacent_interpolated_vars() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( Ok(vec![ @@ -432,7 +429,7 @@ fn error_missing_closing_interp_delim() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( vec![ @@ -478,7 +475,7 @@ fn error_nested_delim() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( vec![ @@ -527,7 +524,7 @@ fn error_empty_interp() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( vec![ @@ -569,7 +566,7 @@ fn error_close_before_open() { let mut sut = Sut::parse(toks.into_iter()); - let expect_name = expect_expanded_header(&mut sut, given_val, a); + let expect_name = expect_expanded_header(&mut sut, a); assert_eq!( vec![ diff --git a/tamer/src/sym/prefill.rs b/tamer/src/sym/prefill.rs index 3b340fec..c0420a62 100644 --- a/tamer/src/sym/prefill.rs +++ b/tamer/src/sym/prefill.rs @@ -733,6 +733,8 @@ pub mod st { URI_LV_TPL: uri "http://www.lovullo.com/rater/apply-template", URI_LV_WORKSHEET: uri "http://www.lovullo.com/rater/worksheet", + S_GEN_FROM_INTERP: str "Generated from interpolated string", + // Common whitespace. // // _This does not represent all forms of whitespace!_ diff --git a/tamer/tests/xmli/meta-interp/expected.xml b/tamer/tests/xmli/meta-interp/expected.xml index 8560526d..fb98b1e9 100644 --- a/tamer/tests/xmli/meta-interp/expected.xml +++ b/tamer/tests/xmli/meta-interp/expected.xml @@ -15,21 +15,21 @@