From a1aa3b236dbadd9e35340c5f5f94856b6012271b Mon Sep 17 00:00:00 2001 From: takeokunn Date: Mon, 3 Aug 2026 22:23:46 +0900 Subject: [PATCH 1/2] fix(syntax): implement Hy's string prefixes, shebang and bracket strings Hy string prefixes were unimplemented across the board -- `f"..."`, `r"..."`, `b"..."`, `rb"..."`, `t"..."`. The atom scanner ran past the opening quote and stopped at a delimiter *inside* the literal, so `r"a)b"` closed its enclosing list early. f-strings are one instance of this, not the whole bug: it accounts for **417 of 469** baseline parse failures. Two more, both silent: `#!` shebangs were not stripped (393 files parsed at exit 0 with the shebang as two junk atoms), and bracket strings `#[[...]]` were parsed as *code* -- `#[[(defn evil [] 1)]]` yielded a real `defn` node inside a raw string, which is the shape where a lint rule fires on text that is not code. `#[delim[...]delim]` was silent too, not a loud failure as first reported. Measured against HEAD with the same code, over 2824 readable files: **493 failures -> 116**, with 378 newly parsing and 1 newly failing. Every failure class strictly decreased. Against **Hy 1.3.0's own reader** as external oracle, files we refuse that Hy accepts fell from **308 to 15**, with zero new over-refusals; all 15 already failed at HEAD with byte-identical errors. The one newly-failing file is a true positive -- real Hy refuses it too (`LexException: invalid string prefix`). It previously "passed" only because the f-string was never scanned. f-strings are modelled as **one opaque atom**, deliberately. `read_fcomponent` calls `parse_one_form`, so the interpolations are arbitrary code and finding the closing quote needs a sub-reader -- `f"{(str "}")}"` is legal. But `ExpressionKind` is only Root/List/Atom, so interleaved text-and-forms needs a new node kind rippling through the formatter, edit engine and every rule; and children are *editable*, so the formatter would reindent inside the literal segments between interpolations. Exposing them trades the `#[[...]]` silent-corruption class for a new one. Blind beats wrong, and today the whole file is invisible. A surprise worth recording: **format specs are nearly unreachable.** `:` is not in `NON_IDENT`, so `f"{x:>10}"` interpolates the *symbol* `x:>10`, and `f"{x:>{width}}"` is a LexException. You need `f"{x :>10}"`. `~` as a reader prefix is deliberately **not** included, though it was implemented and measured. Several formatter paths open a child list without writing that child's prefixes, so a prefixed list is emitted with its prefix deleted -- making `~` a prefix aimed that at the most common construct in Hy macro code, and `edit format` newly changed the meaning of 14 files. PR #100 has since fixed that family; `~` is the obvious next step, and `hy_unquote_is_not_yet_a_reader_prefix` pins the known-wrong reading so whoever does it finds the reasoning. Other dialects: **42142 file-parses, zero differences**, full-tree hash pre- vs post-change across all nine plus the permissive `Unknown` reader. A unit test pins `has_prefixed_strings`/`has_bracket_strings` false and `#[[a]]` as a *list* for all ten. Rebased across #96, #98 and #100, all of which edit the same files. The `classify_reader_macro` dispatch table was resolved arm by arm rather than by side: base had the group `Unknown | Lfe | Hy | Carp`, #98 pulled `Lfe` out and this change pulls `Hy` out, so each side's group line still named the other's dialect. Taking either side would have silently deleted a dialect's reader support. A 29-case probe confirms every recent PR's behaviour survives -- Janet's `'`, Clojure's spaced `#:ns {...}`, LFE's `#B(`/`#M(`/`#S(`/`#"`/`#\`/`|...|`, and #100's `MissingReaderForm` on `(a ')`. One pre-existing issue found and not touched: 13 of the 15 remaining over-refusals are the `,`-as-`Unquote` divergence colliding with #100. Hy's empty tuple is `(,)`, and a trailing `,` before `}` is common, so those are now hard refusals. `classify_hy`'s comment claims `,` "occurs at a token start essentially never" -- the corpus says 13 files, including `hylang/simalq`, `kanaka/mal` and Hy's own `contrib/walk.hy`. Live on main today, and the obvious next Hy item. --- packages/core/syntax/src/sexpr/parser.rs | 70 ++- .../core/syntax/src/sexpr/reader_policy.rs | 471 +++++++++++++++++- .../core/syntax/src/sexpr/tests/parser.rs | 382 +++++++++++++- 3 files changed, 918 insertions(+), 5 deletions(-) diff --git a/packages/core/syntax/src/sexpr/parser.rs b/packages/core/syntax/src/sexpr/parser.rs index bf2e9210..775f1827 100644 --- a/packages/core/syntax/src/sexpr/parser.rs +++ b/packages/core/syntax/src/sexpr/parser.rs @@ -2,7 +2,9 @@ use thiserror::Error; use crate::dialect::Dialect; -use super::reader_policy::{BarQuoting, DialectReaderPolicy, LongStringExtent, ReaderMacro}; +use super::reader_policy::{ + BarQuoting, DialectReaderPolicy, HyStringExtent, LongStringExtent, ReaderMacro, +}; use super::tree::{Comment, Node, NodeKind, ReaderPrefix, ReaderPrefixes, SyntaxTree}; use super::types::{ByteOffset, ByteSpan, Delimiter, NodeId}; @@ -437,6 +439,61 @@ impl<'a> Parser<'a> { } } + fn atom_hy_string_with_prefixes( + &mut self, + prefixes: Vec, + ) -> std::result::Result<(), ParseError> { + let start = self.pos; + let width = self.hy_string_width(start.get())?; + self.advance_by(width); + self.push_atom(prefixes, start, self.pos); + Ok(()) + } + + /// Byte width of the Hy string literal at `start`, prefix and both + /// delimiters included. + /// + /// Unterminated is refused rather than read to EOF, for the same reason + /// the Janet long string is: an atom that swallows the rest of the file is + /// silent corruption, and Hy refuses it too — `chars()` raises + /// `PrematureEndOfInput` when the stream ends inside a string body. + /// + /// The two `Refused` bytes are the ones Hy names explicitly: a `]` inside + /// a bracket string's delimiter ("Ran into a ']' where it wasn't + /// expected") and an undoubled `}` in f-string literal text ("single '}' + /// is not allowed"). Both map onto the delimiter error this reader already + /// has, rather than a new variant, because that is exactly what they are. + fn hy_string_width(&self, start: usize) -> std::result::Result { + match self.policy.hy_string_extent(self.bytes, start) { + Some(HyStringExtent::Closed { width }) => Ok(width), + Some(HyStringExtent::Refused { position, byte }) => Err(ParseError::UnexpectedClose { + delimiter: char::from(byte), + position, + }), + Some(HyStringExtent::Unterminated) | None => Err(ParseError::UnterminatedString(start)), + } + } + + /// Whether a Hy string literal that needs the dedicated scanner starts here. + /// + /// A bare `"` in Hy is left to [`Self::atom_string_with_prefixes`]: it has + /// no prefix and cannot interpolate, and that path already applies the + /// same escape rule. Only a prefixed string (which may be an f-string) and + /// a `#[` bracket string need the sub-reader. + fn at_hy_string(&self) -> bool { + let pos = self.pos.get(); + if self + .policy + .hy_string_prefix_width(self.bytes, pos) + .is_some() + { + return true; + } + self.policy.has_bracket_strings() + && self.bytes.get(pos) == Some(&b'#') + && self.bytes.get(pos + 1) == Some(&b'[') + } + fn atom_with_prefixes( &mut self, prefixes: Vec, @@ -699,6 +756,7 @@ impl<'a> Parser<'a> { b'`' if self.policy.has_long_strings() => { self.atom_long_string_with_prefixes(prefixes)?; } + _ if self.at_hy_string() => self.atom_hy_string_with_prefixes(prefixes)?, _ => self.atom_with_prefixes(prefixes)?, } Ok(()) @@ -889,6 +947,16 @@ impl<'a> Parser<'a> { let width = self.long_string_width(self.pos.get())?; self.advance_by(width); } + // `#_` is Hy's datum comment, so unlike the Janet arm + // above this one is reached constantly: `#_ f"{x}"` + // and `#_ #[[...]]` both land here. Both paths call + // the same `hy_string_extent`, which is what keeps the + // recording and scanning readers from disagreeing + // about where the literal ends. + _ if self.at_hy_string() => { + let width = self.hy_string_width(self.pos.get())?; + self.advance_by(width); + } _ => self.skip_atom()?, } } diff --git a/packages/core/syntax/src/sexpr/reader_policy.rs b/packages/core/syntax/src/sexpr/reader_policy.rs index f1a5abe4..428939c1 100644 --- a/packages/core/syntax/src/sexpr/reader_policy.rs +++ b/packages/core/syntax/src/sexpr/reader_policy.rs @@ -42,6 +42,51 @@ pub(super) enum LongStringExtent { Unterminated, } +/// How far a Hy string literal reaches from its opening delimiter. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum HyStringExtent { + /// Total byte width of the literal, prefix and both delimiters included. + Closed { width: usize }, + /// An opening delimiter with no matching close before EOF. + Unterminated, + /// A byte Hy's own reader refuses outright: a `]` inside a bracket + /// string's delimiter, or an undoubled `}` in an f-string's literal text. + Refused { position: usize, byte: u8 }, +} + +/// The bytes a Hy string prefix may be built from (`hy_reader.prefixed_string`). +const HY_STRING_PREFIX_BYTES: &[u8] = b"bfrt"; + +/// How deeply a Hy f-string may nest string literals inside its `{...}` +/// interpolations before the reader refuses it. +/// +/// `f"{f"{x}"}"` is legal and reads recursively, so the scanner recurses too, +/// and a bound keeps adversarial input from exhausting the stack. Real code +/// does not go past two or three; 32 is far above anything the 2825-file +/// corpus contains and far below a stack limit. +const MAX_HY_STRING_NESTING: usize = 32; + +/// What ends a Hy string body. +#[derive(Debug, Clone, Copy)] +enum HyCloser<'a> { + /// An unescaped `"`. + Quote, + /// `]`, the bracket string's delimiter, then `]`. + Bracket(&'a [u8]), +} + +/// The outcome of scanning one Hy string body. +#[derive(Debug, Clone, Copy)] +enum HyScan { + /// Offset just past the closing delimiter. + End(usize), + Unterminated, + Refused { + position: usize, + byte: u8, + }, +} + /// Dialect-specific lexical decisions shared by normal parsing and discarded /// form scanning. Keeping these decisions in one place prevents the two paths /// from disagreeing about the extent of a reader form. @@ -102,6 +147,25 @@ impl DialectReaderPolicy { // offset 0 keeps a stray `#!` anywhere else the reader error it // has always been. Dialect::EmacsLisp if pos == 0 && bytes.starts_with(b"#!") => Some(2), + // Hy strips a shebang the same way, and under exactly the same + // restriction. `HyReader.parse` peeks the first two characters of + // the *stream* and, when `skip_shebang` is set, consumes to the + // first newline; `hy.importer` and `hy2py` both set it, so this is + // what reading a `.hy` file means. Offset 0 only: Hy's own peek + // happens before any character is consumed, and `\n#!/usr/bin/env + // hy` is rejected with "reader macro '#!/usr/bin/env' is not + // defined", so a `#!` anywhere else stays the error it has always + // been. + // + // Reading it as a line comment rather than stripping the line + // keeps every later byte offset unchanged, which matters because + // every rewrite in this workspace is a span replacement over the + // original string. + // + // Without this, 393 of 2825 real `.hy` files parsed at exit 0 with + // the shebang split into two junk atoms -- `#!/usr/bin/env` and + // `hy` -- sitting at top level as if they were code. + Dialect::Hy if pos == 0 && bytes.starts_with(b"#!") => Some(2), _ => None, } } @@ -242,6 +306,69 @@ impl DialectReaderPolicy { Some(LongStringExtent::Unterminated) } + /// Whether an identifier written immediately before `"` is a Python-style + /// string prefix rather than a symbol. + /// + /// Hy only. `HyReader.read_default` (`hy/reader/hy_reader.py`) reads an + /// identifier and then, if the very next character is `"`, hands that + /// identifier to `prefixed_string` *as a prefix* instead of returning it + /// as a symbol. So `r"a)b"` is one string literal, not the symbol `r` + /// followed by anything. + /// + /// Missing this rule was the single largest reader defect for Hy: the atom + /// scanner ran past the opening quote, stopped at the `)` *inside* the + /// literal, and reported a stray closing delimiter. 417 of 469 parse + /// failures over a 2825-file corpus were this one rule. + pub(super) const fn has_prefixed_strings(self) -> bool { + matches!(self.dialect, Dialect::Hy) + } + + /// Whether `#[` opens a bracket string. + /// + /// Hy only, and unconditionally: `tag_dispatch` finds no identifier after + /// the `#` (because `[` ends one), takes `[` as the tag, and dispatches to + /// `bracketed_string`. There is no other reading of `#[` in Hy. + /// + /// This is why the arm matters so much more than its frequency suggests: + /// `#[[...]]` was being read as a `HashLiteral` prefix on a nested bracket + /// *list*, so `#[[(defn evil [] 1)]]` produced a real `defn` node — inside + /// what is actually a raw string. Any of the 320 lint rules could fire on + /// text that is not code. + pub(super) const fn has_bracket_strings(self) -> bool { + matches!(self.dialect, Dialect::Hy) + } + + /// The width of a valid non-empty Hy string prefix at `pos`, when one is + /// immediately followed by `"`. + /// + /// `prefixed_string` accepts a prefix whose characters are distinct, are a + /// proper subset of `bfrt`, and include at most one of `b`/`f`/`t`. That + /// admits `r`, `b`, `f`, `t` and the two-character pairs that add `r`, and + /// rejects `bf`, `ff` and `bfrt`. A longer identifier such as `foo"` is a + /// Hy error rather than a string; this returns `None` for it so the atom + /// scanner keeps its existing behaviour instead of the reader inventing a + /// literal where Hy has none. + pub(super) fn hy_string_prefix_width(self, bytes: &[u8], pos: usize) -> Option { + if !self.has_prefixed_strings() { + return None; + } + hy_string_prefix_width_at(bytes, pos) + } + + /// How far the Hy string literal starting at `pos` reaches, if one starts + /// there. + /// + /// Covers both spellings: a quoted string with an optional prefix, and a + /// `#[delim[...]delim]` bracket string. The whole literal, interpolations + /// included, is one opaque atom -- see [`hy_string_extent_at`] for why the + /// interpolated forms are deliberately not exposed as children. + pub(super) fn hy_string_extent(self, bytes: &[u8], pos: usize) -> Option { + if !self.has_prefixed_strings() { + return None; + } + hy_string_extent_at(bytes, pos, MAX_HY_STRING_NESTING) + } + /// How many bytes introduce a character literal at `pos`, if one starts /// there. /// @@ -302,10 +429,9 @@ impl DialectReaderPolicy { let third = bytes.get(pos + 2).copied(); match self.dialect { - Dialect::Unknown | Dialect::Hy | Dialect::Carp => { - self.classify_legacy(byte, next, third) - } + Dialect::Unknown | Dialect::Carp => self.classify_legacy(byte, next, third), Dialect::Lfe => self.classify_lfe(bytes, pos), + Dialect::Hy => self.classify_hy(byte, next, third), Dialect::CommonLisp => self.classify_common_lisp(bytes, pos), Dialect::EmacsLisp => self.classify_emacs_lisp(bytes, pos), Dialect::Scheme | Dialect::Racket => self.classify_scheme(bytes, pos), @@ -436,6 +562,61 @@ impl DialectReaderPolicy { self.classify_legacy(byte, next, third) } + /// Hy's reader macros. + /// + /// Split out of [`Self::classify_legacy`] so `#[` can stop being a + /// `HashLiteral` prefix. In Hy it opens a bracket string, so the shared arm + /// turned a raw string's contents into real nodes; + /// [`Self::has_bracket_strings`] describes what that cost. `#(` and `#{` + /// keep the shared reading: they are Hy's tuple and set literals, which + /// really do contain forms. + /// + /// ### Why `~` is *not* a prefix here yet + /// + /// `~` is Hy's unquote and `~@` its unquote-splice (`@reader_for("~")` + /// returns `(unquote ...)`, or `(unquote-splice ...)` when an `@` + /// follows), so a Hy `~foo` coming out as the bare atom `~foo` is wrong: + /// `QuoteState`'s quasiquote counter never comes back down, and every form + /// inside a Hy `` ` `` is treated as data. It suppresses findings rather + /// than inventing them. + /// + /// Adding the two arms is a three-line change and it *was* implemented and + /// measured. It is left out because it is not safe to ship on its own: + /// several formatter paths open a child list by pushing `delimiter.open()` + /// directly, without first writing that child's reader prefixes, so a + /// prefixed list reaching one of them is emitted with its prefix deleted. + /// `format_body_clause` and `format_sequence_list` are two; there are more. + /// + /// That bug is already live — `#(...)` in a Hy `cond` loses its `#` today, + /// turning a tuple into a call — but it is rare, because `#(` in clause + /// position is rare. Making `~` a prefix would aim it straight at the + /// single most common construct in Hy macro code. Measured over the 2825 + /// file corpus with Hy's reader as the oracle, `edit format` newly changed + /// the *meaning* of 14 files that it had previously formatted correctly, + /// every one of them a dropped `~`/`~@`. + /// + /// So this waits on a formatter fix. It is a bug family across five files + /// in `sexpr::formatter`, it affects every dialect, and it needs its own + /// golden review — not a rider on a reader change. + /// + /// Two smaller divergences are also knowingly left alone: + /// + /// * `,` still reads as `Unquote`. In Hy it is an ordinary identifier + /// character — `(foo a, b)` reads the symbol `a,` — so a leading `,` is + /// mis-shaped. It occurs at a token start essentially never. + /// * `#;`, `#+` and `#-` are not Hy reader macros at all. + const fn classify_hy( + self, + byte: u8, + next: Option, + third: Option, + ) -> Option { + match (byte, next) { + (b'#', Some(b'[')) => None, + _ => self.classify_legacy(byte, next, third), + } + } + fn classify_common_lisp(self, bytes: &[u8], pos: usize) -> Option { let byte = *bytes.get(pos)?; let next = bytes.get(pos + 1).copied(); @@ -767,6 +948,290 @@ impl DialectReaderPolicy { } } +/// Whether `byte` ends a Hy identifier (`HyReader.NON_IDENT`). +/// +/// Note what is *absent*: `#`, `,`, `:`, `!` and `@` are all ordinary +/// identifier constituents in Hy, which is why `a,` is the single symbol `a,` +/// and why `f"{x:>10}"` interpolates the symbol `x:>10` rather than applying a +/// format spec. +const fn is_hy_non_ident(byte: u8) -> bool { + byte.is_ascii_whitespace() + || matches!( + byte, + b'(' | b')' | b'[' | b']' | b'{' | b'}' | b';' | b'"' | b'\'' | b'`' | b'~' + ) +} + +/// The width of a valid non-empty Hy string prefix at `pos` followed by `"`. +fn hy_string_prefix_width_at(bytes: &[u8], pos: usize) -> Option { + // A valid prefix is one or two bytes: `prefixed_string` requires distinct + // characters, a *proper* subset of `bfrt`, and at most one of `b`/`f`/`t`. + for width in 1..=2usize { + let candidate = bytes.get(pos..pos + width)?; + if bytes.get(pos + width) != Some(&b'"') { + continue; + } + if !candidate + .iter() + .all(|byte| HY_STRING_PREFIX_BYTES.contains(byte)) + { + continue; + } + if candidate[0] == *candidate.last().expect("candidate is non-empty") && width == 2 { + // Duplicate characters: `prefix_chars` would be shorter than + // `prefix`, which `prefixed_string` rejects (`ff"..."`). + continue; + } + if candidate.iter().filter(|byte| **byte != b'r').count() > 1 { + // Two of `b`/`f`/`t` together, such as `bf"..."`. + continue; + } + return Some(width); + } + None +} + +/// Whether the prefix bytes at `pos` select f-string (interpolating) mode. +fn hy_prefix_is_fstring(prefix: &[u8]) -> bool { + prefix.iter().any(|byte| matches!(byte, b'f' | b't')) +} + +/// How far the complete Hy string literal starting at `pos` reaches. +/// +/// ### Why the whole literal is one opaque atom +/// +/// A Hy f-string genuinely contains code: `read_fcomponent` calls +/// `parse_one_form`, so `f"{(get d \"k\")}"` holds a real function call, and +/// this scanner has to understand that sub-language just to find the closing +/// quote. It would therefore be possible to expose the interpolated forms as +/// children. That is deliberately not done, for three reasons. +/// +/// * Hy models it as a literal. The reader returns `FString`, a value- +/// producing model, not program structure. +/// * There is no node kind for it. `ExpressionKind` is `Root`, `List` or +/// `Atom`; interleaved literal text and forms fits none of them, and adding +/// a kind changes a crate shared by the formatter, the edit engine and 320 +/// lint rules. +/// * Children are *editable*, and that is the danger. The formatter reindents +/// children; reindenting inside an f-string rewrites the literal segments +/// between the interpolations and silently changes what the program prints. +/// That is the same failure this fix removes for `#[[...]]` — code visible +/// where there is none — and re-introducing it deliberately would trade one +/// silent-corruption class for another. +/// +/// Blind beats wrong, and the alternative today is worse than blind: the file +/// does not parse at all, so no rule sees any of it. +fn hy_string_extent_at(bytes: &[u8], pos: usize, budget: usize) -> Option { + let scan = hy_string_scan_at(bytes, pos, budget)?; + Some(match scan { + HyScan::End(end) => HyStringExtent::Closed { width: end - pos }, + HyScan::Unterminated => HyStringExtent::Unterminated, + HyScan::Refused { position, byte } => HyStringExtent::Refused { position, byte }, + }) +} + +/// Scans a complete Hy string literal at `pos`, in either spelling. +fn hy_string_scan_at(bytes: &[u8], pos: usize, budget: usize) -> Option { + if budget == 0 { + return Some(HyScan::Unterminated); + } + if bytes.get(pos) == Some(&b'#') && bytes.get(pos + 1) == Some(&b'[') { + return Some(hy_bracket_string_scan(bytes, pos, budget)); + } + let prefix_width = hy_string_prefix_width_at(bytes, pos).unwrap_or(0); + if bytes.get(pos + prefix_width) != Some(&b'"') { + return None; + } + let prefix = &bytes[pos..pos + prefix_width]; + Some(hy_string_body_scan( + bytes, + pos + prefix_width + 1, + hy_prefix_is_fstring(prefix), + HyCloser::Quote, + budget, + )) +} + +/// Scans a `#[delim[...]delim]` bracket string whose `#[` is at `pos`. +/// +/// The delimiter is every byte up to the second `[`; a `]` there is the error +/// Hy raises as "Ran into a ']' where it wasn't expected". A delimiter of `f` +/// or one starting `f-` makes the body an f-string. `t` does not, because +/// `bracketed_templates` is off in the default reader. +fn hy_bracket_string_scan(bytes: &[u8], pos: usize, budget: usize) -> HyScan { + let delim_start = pos + 2; + let mut cursor = delim_start; + loop { + let Some(&byte) = bytes.get(cursor) else { + return HyScan::Unterminated; + }; + if byte == b'[' { + break; + } + if byte == b']' { + return HyScan::Refused { + position: cursor, + byte, + }; + } + cursor += 1; + } + let delim = &bytes[delim_start..cursor]; + let fstring = delim == b"f" || delim.starts_with(b"f-"); + // A single newline straight after the opening `[` is dropped from the + // value. It is still inside the literal's span, so the extent is unchanged + // and the scanner simply steps over it. + let mut body = cursor + 1; + if bytes.get(body) == Some(&b'\r') { + body += 1; + } + if bytes.get(body) == Some(&b'\n') { + body += 1; + } + hy_string_body_scan(bytes, body, fstring, HyCloser::Bracket(delim), budget) +} + +/// Whether the string's closing delimiter sits at `pos`, and how wide it is. +fn hy_closer_width(bytes: &[u8], pos: usize, closer: HyCloser<'_>) -> Option { + match closer { + HyCloser::Quote => (bytes.get(pos) == Some(&b'"')).then_some(1), + HyCloser::Bracket(delim) => { + if bytes.get(pos) != Some(&b']') { + return None; + } + let after = pos + 1; + let end = after + delim.len(); + if bytes.get(after..end) != Some(delim) { + return None; + } + (bytes.get(end) == Some(&b']')).then_some(delim.len() + 2) + } + } +} + +/// Scans a Hy string body from its first content byte to just past its close. +/// +/// Two regions alternate. In the literal region the closing delimiter ends the +/// string, a backslash escapes the next byte (for quoted strings — a bracket +/// string's `delim_closing` has no escape case at all, which is what makes it +/// raw), and in f-string mode `{{`/`}}` are doubled literals while a lone `{` +/// opens an interpolation. +/// +/// Inside an interpolation the bytes are Hy code, so the scanner has to skip +/// what the reader would skip: nested string literals of either spelling, +/// `;` line comments, and nested braces from dict literals. Getting this wrong +/// would move the closing quote, which is why it is a real sub-reader rather +/// than a search for the next `"`. +fn hy_string_body_scan( + bytes: &[u8], + pos: usize, + fstring: bool, + closer: HyCloser<'_>, + budget: usize, +) -> HyScan { + let mut cursor = pos; + let mut escaped = false; + let mut depth = 0usize; + + while cursor < bytes.len() { + let byte = bytes[cursor]; + + if depth == 0 { + // Hy checks `closing(c)` before it looks at braces, so the closing + // delimiter wins over everything except a pending escape. + if !escaped { + if let Some(width) = hy_closer_width(bytes, cursor, closer) { + return HyScan::End(cursor + width); + } + } + if escaped { + escaped = false; + cursor += 1; + continue; + } + if byte == b'\\' && matches!(closer, HyCloser::Quote) { + escaped = true; + cursor += 1; + continue; + } + if !fstring { + cursor += 1; + continue; + } + match byte { + b'{' if bytes.get(cursor + 1) == Some(&b'{') => cursor += 2, + b'{' => { + depth = 1; + cursor += 1; + } + b'}' if bytes.get(cursor + 1) == Some(&b'}') => cursor += 2, + // `read_chars_until` raises "single '}' is not allowed" here. + b'}' => { + return HyScan::Refused { + position: cursor, + byte, + }; + } + _ => cursor += 1, + } + continue; + } + + // Interpolation: this is Hy code. + match byte { + b';' => { + cursor += 1; + while cursor < bytes.len() && bytes[cursor] != b'\n' { + cursor += 1; + } + } + b'{' => { + depth += 1; + cursor += 1; + } + b'}' => { + depth -= 1; + cursor += 1; + } + _ => { + // A nested literal keeps its own rules, including its own + // escapes and its own braces, so it is scanned rather than + // skipped byte by byte. `f"{(str \"}\")}"` depends on this: + // the `}` inside the nested string must not close the field. + let nested = if is_nested_hy_string_start(bytes, cursor) { + hy_string_scan_at(bytes, cursor, budget - 1) + } else { + None + }; + match nested { + Some(HyScan::End(end)) => cursor = end, + Some(other) => return other, + None => cursor += 1, + } + } + } + } + + HyScan::Unterminated +} + +/// Whether a nested string literal starts at `pos` inside an interpolation. +/// +/// A prefix only counts at the start of an identifier, so the `f` of `xf"..."` +/// is not one. +fn is_nested_hy_string_start(bytes: &[u8], pos: usize) -> bool { + if bytes.get(pos) == Some(&b'#') { + return bytes.get(pos + 1) == Some(&b'['); + } + if bytes.get(pos) == Some(&b'"') { + return true; + } + if hy_string_prefix_width_at(bytes, pos).is_none() { + return false; + } + pos == 0 || bytes.get(pos - 1).copied().is_some_and(is_hy_non_ident) +} + /// How many consecutive backticks start at `pos`. fn backtick_run_length(bytes: &[u8], pos: usize) -> usize { bytes[pos..] diff --git a/packages/core/syntax/src/sexpr/tests/parser.rs b/packages/core/syntax/src/sexpr/tests/parser.rs index 851093d6..cadc0e73 100644 --- a/packages/core/syntax/src/sexpr/tests/parser.rs +++ b/packages/core/syntax/src/sexpr/tests/parser.rs @@ -1,7 +1,7 @@ use super::*; use crate::dialect::Dialect; use crate::sexpr::parser::MAX_DISCARDED_FORM_STACK_FRAMES; -use crate::sexpr::reader_policy::{DialectReaderPolicy, LongStringExtent}; +use crate::sexpr::reader_policy::{DialectReaderPolicy, HyStringExtent, LongStringExtent}; #[test] fn parses_balanced_document() { @@ -2098,3 +2098,383 @@ fn lfe_lexical_switches_are_scoped_to_lfe() { ); } } + +/// The four Hy reader defects, each pinned by the shape it produced before. +/// +/// Every expectation here was checked against Hy 1.3.1's own reader +/// (`hy.reader.read_many`) rather than derived from the documentation. +#[test] +fn hy_reads_its_own_string_and_unquote_syntax() { + struct Case { + input: &'static str, + children: &'static [&'static str], + } + + let cases = [ + // f-strings. Before, `f"hi {name}"` scanned as the token `f"hi`, a + // brace list, then an unterminated string, and the file did not parse. + Case { + input: r#"(print f"hi {name}")"#, + children: &["print", r#"f"hi {name}""#], + }, + // Arbitrary Hy code lives in the braces -- `read_fcomponent` calls + // `parse_one_form` -- but the literal stays one opaque atom. + Case { + input: r#"(print f"{(+ 1 2)}")"#, + children: &["print", r#"f"{(+ 1 2)}""#], + }, + // A nested string inside a field holds the outer closing quote + // hostage. This is why the scanner is a sub-reader rather than a + // search for the next `"`. + Case { + input: r#"(print f"{(str "}")}")"#, + children: &["print", r#"f"{(str "}")}""#], + }, + // A nested f-string, and a `;` comment inside a field. + Case { + input: "(print f\"{f\"{x}\"}\")", + children: &["print", "f\"{f\"{x}\"}\""], + }, + Case { + input: "(print f\"{(+ 1 ; c\n 2)}\")", + children: &["print", "f\"{(+ 1 ; c\n 2)}\""], + }, + // Doubled braces are literal text, not fields. + Case { + input: r#"(print f"{{literal}}")"#, + children: &["print", r#"f"{{literal}}""#], + }, + // Every other valid prefix. `r"a)b"` was the widest-reaching case: the + // atom scanner ran past the quote and stopped at the `)` *inside* the + // literal, reporting a stray closing delimiter. + Case { + input: r#"(re.match r"a)b" s)"#, + children: &["re.match", r#"r"a)b""#, "s"], + }, + Case { + input: r#"(f b"x" rb"y" br"z" t"{q}" rf"w" fr"v" rt"u")"#, + children: &[ + "f", + r#"b"x""#, + r#"rb"y""#, + r#"br"z""#, + r#"t"{q}""#, + r#"rf"w""#, + r#"fr"v""#, + r#"rt"u""#, + ], + }, + // Bracket strings. The dangerous one: this used to yield a real `defn` + // node inside what is actually a raw string. + Case { + input: "(setv x #[[(defn evil [] 1)]])", + children: &["setv", "x", "#[[(defn evil [] 1)]]"], + }, + Case { + input: "(setv x #[delim[hello]delim])", + children: &["setv", "x", "#[delim[hello]delim]"], + }, + // The close is `]` + delim + `]`, so a bare `]]` inside is content. + Case { + input: "(setv x #[d[a]]b]d])", + children: &["setv", "x", "#[d[a]]b]d]"], + }, + // Unbalanced delimiters inside are just bytes. + Case { + input: "(setv x #[[ ) ) ) ]])", + children: &["setv", "x", "#[[ ) ) ) ]]"], + }, + // A bracket string whose delimiter is `f` or starts `f-` interpolates. + Case { + input: "(setv x #[f-q[{(+ 1 2)}]f-q])", + children: &["setv", "x", "#[f-q[{(+ 1 2)}]f-q]"], + }, + // `#(` and `#{` keep their existing reading: Hy's tuple and set + // literals really do contain forms, unlike `#[`. + Case { + input: "(f #(1 2) #{3 4})", + children: &["f", "#(1 2)", "#{3 4}"], + }, + ]; + + for case in cases { + let tree = SyntaxTree::parse_with_dialect(case.input, Dialect::Hy) + .unwrap_or_else(|error| panic!("{}: {error}", case.input)); + let form = &tree.root_view().children[0]; + let children = form + .children + .iter() + .map(|child| child.span.slice(case.input)) + .collect::>(); + assert_eq!(children, case.children, "{}", case.input); + } +} + +/// A Hy shebang is trivia, and only at offset 0. +/// +/// `HyReader.parse` peeks the first two characters of the stream, so +/// `\n#!/usr/bin/env hy` stays the "reader macro is not defined" error it has +/// always been. Reading it as a line comment rather than stripping the line +/// keeps every later byte offset unchanged. +#[test] +fn hy_shebang_is_trivia_only_at_offset_zero() { + let input = "#!/usr/bin/env hy\n(print 1)\n"; + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Hy).expect("valid"); + let roots = tree + .root_view() + .children + .iter() + .map(|child| child.span.slice(input)) + .collect::>(); + assert_eq!(roots, vec!["(print 1)"]); + + // Offset 0 only. A `#!` on a later line stays an ordinary dispatch, and + // the two junk atoms it produces are the pre-existing reading. + let later = "(print 1)\n#!/usr/bin/env hy\n"; + let tree = SyntaxTree::parse_with_dialect(later, Dialect::Hy).expect("parses"); + assert_eq!(tree.root_view().children.len(), 3, "{later:?}"); +} + +/// Hy's `~` is deliberately still a bare atom, not a reader prefix. +/// +/// This pins the *known-wrong* reading on purpose, so that whoever makes `~` a +/// prefix has to come here and read why it was held back. `classify_hy` +/// carries the reasoning: several formatter paths open a child list without +/// writing its reader prefixes, so promoting `~` deletes it from the output. +/// Measured over 2825 real files, that newly changed the meaning of 14 files +/// `edit format` had previously handled correctly. +/// +/// The fix belongs with the formatter fix, not before it. +#[test] +fn hy_unquote_is_not_yet_a_reader_prefix() { + let input = "`(foo ~bar ~@baz)"; + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Hy).expect("valid"); + let form = &tree.root_view().children[0]; + assert_eq!(form.reader_prefixes, vec![ReaderPrefix::Quasiquote]); + let children = form + .children + .iter() + .map(|child| (child.span.slice(input), child.reader_prefixes.clone())) + .collect::>(); + assert_eq!( + children, + vec![("foo", vec![]), ("~bar", vec![]), ("~@baz", vec![])] + ); + + // Clojure's `~`, which really is a prefix, must be unaffected either way. + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Clojure).expect("valid"); + let prefixes = tree.root_view().children[0] + .children + .iter() + .map(|child| child.reader_prefixes.clone()) + .collect::>(); + assert_eq!( + prefixes, + vec![ + vec![], + vec![ReaderPrefix::Unquote], + vec![ReaderPrefix::UnquoteSplicing], + ] + ); +} + +/// An unterminated Hy literal fails loudly rather than swallowing the file. +/// +/// Reading to EOF as one atom is the silent corruption this fix removes: every +/// later command would be handed a tree in which the rest of the document is +/// one giant symbol. Hy refuses all of these too. +#[test] +fn hy_unterminated_literals_are_refused() { + for input in [ + r#"(print f"abc"#, + r#"(print f"{(+ 1 2"#, + "(setv x #[[abc)", + "(setv x #[delim[abc]nope])", + r#"(print r"abc"#, + ] { + let error = SyntaxTree::parse_with_dialect(input, Dialect::Hy) + .expect_err("an unterminated literal must be refused"); + assert!( + matches!( + error, + ParseError::UnterminatedString(_) | ParseError::UnexpectedClose { .. } + ), + "{input:?}: {error}" + ); + } + + // The two bytes Hy names explicitly become delimiter errors, since that is + // what they are: a `]` in a bracket string's delimiter, and an undoubled + // `}` in f-string literal text. + assert!(matches!( + SyntaxTree::parse_with_dialect("(setv x #[a]b[y]a]b])", Dialect::Hy), + Err(ParseError::UnexpectedClose { delimiter: ']', .. }) + )); + assert!(matches!( + SyntaxTree::parse_with_dialect(r#"(print f"}")"#, Dialect::Hy), + Err(ParseError::UnexpectedClose { delimiter: '}', .. }) + )); +} + +/// The prefix table, at the boundaries `prefixed_string` actually enforces. +#[test] +fn hy_string_prefix_width_matches_hys_validation() { + let policy = DialectReaderPolicy::new(Dialect::Hy); + // Distinct characters, a proper subset of `bfrt`, at most one of `b`/`f`/`t`. + for good in ["r", "b", "f", "t", "rb", "br", "rf", "fr", "rt", "tr"] { + let source = format!("{good}\"x\""); + assert_eq!( + policy.hy_string_prefix_width(source.as_bytes(), 0), + Some(good.len()), + "{good}" + ); + } + // Two of `b`/`f`/`t`, a repeated character, or not a prefix at all. + for bad in ["bf", "ff", "bt", "q", "xf", "foo"] { + let source = format!("{bad}\"x\""); + assert_eq!( + policy.hy_string_prefix_width(source.as_bytes(), 0), + None, + "{bad}" + ); + } + // A prefix is only a prefix immediately before a quote. + assert_eq!(policy.hy_string_prefix_width(b"r x", 0), None); + // And never outside Hy. + assert_eq!( + DialectReaderPolicy::new(Dialect::CommonLisp).hy_string_prefix_width(b"r\"x\"", 0), + None + ); +} + +/// The extent scanner, including the cases that decide where a literal ends. +#[test] +fn hy_string_extent_matches_hys_reader() { + let policy = DialectReaderPolicy::new(Dialect::Hy); + let closed = |width| Some(HyStringExtent::Closed { width }); + + assert_eq!(policy.hy_string_extent(br#"f"hi {name}""#, 0), closed(12)); + assert_eq!(policy.hy_string_extent(br#"r"a)b""#, 0), closed(6)); + // The nested string holds the outer closing quote hostage. + assert_eq!(policy.hy_string_extent(br#"f"{(str "}")}""#, 0), closed(14)); + // Doubled braces are literal, so this closes at its own final quote. + assert_eq!(policy.hy_string_extent(br#"f"{{a}}""#, 0), closed(8)); + // An escaped quote does not close a literal. + assert_eq!(policy.hy_string_extent(br#"f"a\"b {x}""#, 0), closed(11)); + // Bracket strings: the close is `]` + delim + `]`, so `]]` here is content. + assert_eq!(policy.hy_string_extent(b"#[d[a]]b]d]", 0), closed(11)); + assert_eq!(policy.hy_string_extent(b"#[[a]]", 0), closed(6)); + // Scanning starts at `pos`, not at 0. + assert_eq!(policy.hy_string_extent(br#"(f r"a)b")"#, 3), closed(6)); + // Unterminated, and not a string at all. + assert_eq!( + policy.hy_string_extent(br#"f"abc"#, 0), + Some(HyStringExtent::Unterminated) + ); + assert_eq!( + policy.hy_string_extent(b"#[[abc", 0), + Some(HyStringExtent::Unterminated) + ); + assert_eq!(policy.hy_string_extent(b"abc", 0), None); + assert_eq!(policy.hy_string_extent(b"", 0), None); + // And never one outside Hy. + assert_eq!( + DialectReaderPolicy::new(Dialect::Clojure).hy_string_extent(b"#[[a]]", 0), + None + ); +} + +/// Nothing above may leak into another dialect. +/// +/// `#[`, `~` and an `r"..."` prefix all mean something else, or nothing, in the +/// other ten readers. This pins them so a later edit to `has_prefixed_strings` +/// or `has_bracket_strings` cannot quietly widen, the way the Janet long +/// string test pins backtick. +#[test] +fn hy_string_and_unquote_rules_do_not_leak_to_other_dialects() { + for dialect in [ + Dialect::CommonLisp, + Dialect::EmacsLisp, + Dialect::Scheme, + Dialect::Racket, + Dialect::Clojure, + Dialect::Fennel, + Dialect::Lfe, + Dialect::Carp, + Dialect::Janet, + Dialect::Unknown, + ] { + let policy = DialectReaderPolicy::new(dialect); + assert!(!policy.has_prefixed_strings(), "{}", dialect.label()); + assert!(!policy.has_bracket_strings(), "{}", dialect.label()); + assert_eq!( + policy.hy_string_prefix_width(b"r\"x\"", 0), + None, + "{}", + dialect.label() + ); + assert_eq!( + policy.hy_string_extent(b"#[[a]]", 0), + None, + "{}", + dialect.label() + ); + + // `#[` stays whatever it already was: a hash-literal prefix on a + // bracket *list* in the dialects that have one, and a refusal in the + // rest. Comparing the sliced text would not discriminate -- Fennel's + // `#` prefix spans exactly the same bytes -- so this compares the + // node kind, which is the thing that actually changed for Hy. + let input = "(setv x #[[a]])"; + if let Ok(tree) = SyntaxTree::parse_with_dialect(input, dialect) { + let third = &tree.root_view().children[0].children[2]; + assert_eq!( + third.kind, + ExpressionKind::List, + "{}: `#[[a]]` must stay a list, not become one raw-string atom", + dialect.label() + ); + } + } + + // `~` is unquote in Clojure too; only Hy gained an arm, and Clojure's + // existing one must be untouched. + let input = "`(foo ~bar ~@baz)"; + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Clojure).expect("valid"); + let prefixes = tree.root_view().children[0] + .children + .iter() + .map(|child| child.reader_prefixes.clone()) + .collect::>(); + assert_eq!( + prefixes, + vec![ + vec![], + vec![ReaderPrefix::Unquote], + vec![ReaderPrefix::UnquoteSplicing], + ] + ); +} + +/// The recording reader and the discarded-form scanner must agree. +/// +/// `#_` is Hy's datum comment, so unlike Janet's unreachable arm this path runs +/// on real input. If the two disagreed about where an f-string or bracket +/// string ends, a `#_` would discard the wrong number of bytes. +#[test] +fn hy_discarded_forms_use_the_same_string_extent() { + for (input, expected) in [ + (r#"(f #_ f"{(str "}")}" tail)"#, vec!["f", "tail"]), + ("(f #_ #[d[a]]b]d] tail)", vec!["f", "tail"]), + (r#"(f #_ r"a)b" tail)"#, vec!["f", "tail"]), + ] { + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Hy) + .unwrap_or_else(|error| panic!("{input}: {error}")); + let children = tree.root_view().children[0] + .children + .iter() + .map(|child| child.span.slice(input)) + .collect::>(); + assert_eq!(children, expected, "{input}"); + } +} From ddf135510c0e4cf695b38c7e857d74bc73b113e5 Mon Sep 17 00:00:00 2001 From: takeokunn Date: Mon, 3 Aug 2026 22:47:13 +0900 Subject: [PATCH 2/2] perf(syntax): keep the Hy string check off the other dialects' parse path CI's bench gate measured `parse-scaling/reader-conditional` +11.7% at 1MiB and +10.6% at 8MiB. Both sizes moving together by the same amount is the real-regression signature, not the `edit-all-loop` measurement artifact -- and unlike that one, this branch genuinely does touch the parser's inner loop. `at_hy_string` is called once per *form* from `parse_form`'s match and again from the discarded-form scanner. Its dialect test sat inside `hy_string_prefix_width`, one call deep, with a second `has_bracket_strings()` test after it -- so for the other nine dialects it was two guarded calls and two byte reads per form rather than one comparison against a loop-invariant field. Hoisted the dialect gate to the top and marked the function `#[inline]`. On an 8MiB Common Lisp document that is millions of calls that can only ever answer `false`. This is the ordering the Carp reader change adopted deliberately for the same reason: `self.dialect` is loop-invariant across the per-byte and per-form calls, so testing it first lets the rest fold away. --- packages/core/syntax/src/sexpr/parser.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/core/syntax/src/sexpr/parser.rs b/packages/core/syntax/src/sexpr/parser.rs index 775f1827..1ba8ec95 100644 --- a/packages/core/syntax/src/sexpr/parser.rs +++ b/packages/core/syntax/src/sexpr/parser.rs @@ -480,7 +480,20 @@ impl<'a> Parser<'a> { /// no prefix and cannot interpolate, and that path already applies the /// same escape rule. Only a prefixed string (which may be an f-string) and /// a `#[` bracket string need the sub-reader. + /// Whether a Hy string literal starts at the cursor. + /// + /// The dialect test is hoisted to the top and the whole function is + /// `#[inline]`, so for the other nine dialects this collapses to a single + /// comparison against a loop-invariant field rather than two guarded calls + /// and two byte reads. That matters because this is called once per *form* + /// from `parse_form`'s match and again from the discarded-form scanner: on + /// an 8 MiB Common Lisp document the difference is millions of calls that + /// can only ever answer `false`. + #[inline] fn at_hy_string(&self) -> bool { + if !self.policy.has_prefixed_strings() && !self.policy.has_bracket_strings() { + return false; + } let pos = self.pos.get(); if self .policy