From a1447309819403d25ba111f31f8e1e28f630f309 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Wed, 28 Jun 2023 16:56:51 -0400 Subject: [PATCH] tamer: nir::abstract_bind: Require @-padding of metavariable names This enforces the naming convention that is utilized to infer whether an identifier binding must be translated to an abstract binding. This does not yet place any restrictions on other characters in identifier names; both the placement of and flexibility of that has yet to be decided. This change is sufficient enough to make abstract binding translation reliable. DEV-13163 --- tamer/src/nir/abstract_bind.rs | 111 +++++++++++++++++++++++++--- tamer/src/nir/abstract_bind/test.rs | 84 ++++++++++++++++++++- 2 files changed, 182 insertions(+), 13 deletions(-) diff --git a/tamer/src/nir/abstract_bind.rs b/tamer/src/nir/abstract_bind.rs index bebe6e05..981133af 100644 --- a/tamer/src/nir/abstract_bind.rs +++ b/tamer/src/nir/abstract_bind.rs @@ -114,22 +114,30 @@ //! metavariable naming convention as abstract bindings. //! This is equivalent to the interpolation behavior. //! -//! This module is therefore an _optional_ syntactic feature of TAME. +//! To support this interpretation, +//! this lowering operation requires that all names of +//! [`Nir::BindIdentMeta`] be padded with a single `@` on each side, +//! as shown in the examples above. +//! +//! Lowering Pipeline Ordering +//! ========================== +//! This module is an _optional_ syntactic feature of TAME. //! If desired, //! this module could be omitted from the lowering pipeline in favor of //! explicitly utilizing interpolation for all abstract identifiers. -//! When utilized, -//! this lowering operation may be intergrated into the lowering pipeline -//! either before or after interpolation. //! -//! TODO: Since this module acts upon a naming convention, -//! shouldn't it also enforce it on definition so that we know that -//! metavariables _will_ always follow that convention? -//! At the time of writing, -//! no part of TAMER yet enforces metavariable naming conventions. +//! Interpolation via [`super::interp`] its own [`Nir::BindIdentAbstract`] +//! tokens, +//! and shorthand template application via [`super::tplshort`] desugars +//! into both the proper `@`-padded naming convention and +//! [`Nir::BindIdentAbstract`]. +//! It should therefore be possible to place this operation in any order +//! relative to those two. use super::Nir; -use crate::{parse::prelude::*, sym::GlobalSymbolResolve}; +use crate::{ + fmt::TtQuote, parse::prelude::*, span::Span, sym::GlobalSymbolResolve, +}; use memchr::memchr; use Nir::*; @@ -161,6 +169,11 @@ impl ParseState for AbstractBindTranslate { Transition(self).ok(BindIdentAbstract(name)) } + BindIdentMeta(name) => validate_meta_name(name) + .map(BindIdentMeta) + .map(ParseStatus::Object) + .transition(self), + _ => Transition(self).ok(tok), } } @@ -170,6 +183,37 @@ impl ParseState for AbstractBindTranslate { } } +/// Validate that the provided name is padded with a single `@` on both +/// sides. +/// +/// This check is necessary to ensure that we can properly infer when a +/// metavariable is in use in a bind position without having to rely on +/// interpolation. +/// +/// TODO: This does not yet place any other restrictions on the name of a +/// metavariable; +/// we'll take care of that when we decide on an approach for other +/// names. +fn validate_meta_name( + meta: SPair, +) -> Result { + let name = meta.symbol().lookup_str(); + + if !name.starts_with('@') { + Err(AbstractBindTranslateError::MetaNamePadMissing( + meta, + meta.span().slice_head(0), + )) + } else if !name.ends_with('@') { + Err(AbstractBindTranslateError::MetaNamePadMissing( + meta, + meta.span().slice_tail(0), + )) + } else { + Ok(meta) + } +} + /// Determine whether the given name requires translation into an abstract /// identifier. /// @@ -200,8 +244,51 @@ fn needs_translation(name: SPair) -> bool { ) } -diagnostic_infallible! { - pub enum AbstractBindTranslateError {} +#[derive(Debug, PartialEq, Eq)] +pub enum AbstractBindTranslateError { + /// A metavariable does not adhere to the naming convention requiring + /// `@`-padding. + /// + /// The provided [`Span`] is the first occurrence of such a violation. + /// If `@` is missing from both the beginning and end of the name, + /// then one of them is chosen. + MetaNamePadMissing(SPair, Span), +} + +impl Display for AbstractBindTranslateError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + use AbstractBindTranslateError::*; + + match self { + MetaNamePadMissing(name, _) => write!( + f, + "metavariable {} must both begin and end with `@`", + TtQuote::wrap(name), + ), + } + } +} + +impl Diagnostic for AbstractBindTranslateError { + fn describe(&self) -> Vec { + use AbstractBindTranslateError::*; + + match self { + MetaNamePadMissing(_, at) => vec![ + at.error("missing `@` here"), + at.help( + "metavariables (such as template parameters) must \ + have names that both begin and end with the \ + character `@`", + ), + at.help( + "this naming requirement is necessary to make curly \ + braces optional when referencing metavariables \ + without requiring interpolation", + ), + ], + } + } } #[cfg(test)] diff --git a/tamer/src/nir/abstract_bind/test.rs b/tamer/src/nir/abstract_bind/test.rs index a377ff27..ada82c3d 100644 --- a/tamer/src/nir/abstract_bind/test.rs +++ b/tamer/src/nir/abstract_bind/test.rs @@ -18,7 +18,7 @@ // along with this program. If not, see . use super::*; -use crate::span::dummy::*; +use crate::span::dummy::{DUMMY_CONTEXT as DC, *}; type Sut = AbstractBindTranslate; use Parsed::Object as O; @@ -80,6 +80,88 @@ fn meta_concrete_bind_with_metavar_naming_ignored() { ); } +// This lowering operation utilizes a naming convention to infer user intent +// and lift the requirement for curly braces; +// they go hand-in-hand. +// To utilize this feature, +// we must also require adherence to the naming convention so that we know +// that our assumptions hold. +// +// We can't check for the opposite--- +// that non-meta identifiers must _not_ follow that convention--- +// because we interpet such occurrences as abstract identifiers. +// In practice, +// users will get an error because the conversion into a reference will +// yield an error when the metavariable does not exist already as a +// reference, +// or a duplicate definition error if it was already defined. +#[test] +fn rejects_metavariable_without_naming_convention() { + let name_a = "@missing-end".into(); + // | | + // | 11 + // | B + // [----------] + // 0 11 + // A + + let a_a = DC.span(10, 12); + let a_b = DC.span(22, 0); // _after_ last char + + let name_b = "missing-start@".into(); + // | | + // 0 | + // A | + // [------------] + // 0 13 + // A + let b_a = DC.span(10, 14); + let b_b = DC.span(10, 0); // _before_ first char + + let name_c = "missing-both".into(); + // | | + // 0 | + // B | + // [----------] + // 0 11 + // A + let c_a = DC.span(10, 12); + let c_b = DC.span(10, 0); // _before_ first char + + // Each of these will result in slightly different failures. + #[rustfmt::skip] + let toks = [ + BindIdentMeta(SPair(name_a, a_a)), + BindIdentMeta(SPair(name_b, b_a)), + BindIdentMeta(SPair(name_c, c_a)), + ]; + + assert_eq!( + #[rustfmt::skip] + vec![ + Err(ParseError::StateError( + AbstractBindTranslateError::MetaNamePadMissing( + SPair(name_a, a_a), + a_b, + ), + )), + Err(ParseError::StateError( + AbstractBindTranslateError::MetaNamePadMissing( + SPair(name_b, b_a), + b_b, + ), + )), + Err(ParseError::StateError( + AbstractBindTranslateError::MetaNamePadMissing( + SPair(name_c, c_a), + c_b, + ), + )), + ], + Sut::parse(toks.into_iter()).collect::>>(), + ); +} + fn assert_tok_translate(tok: Nir, expect: Nir) { #[rustfmt::skip] assert_eq!(