From 55655fd06a6a882d205318df57015c7ded7ecb1b Mon Sep 17 00:00:00 2001 From: takeokunn Date: Mon, 3 Aug 2026 23:15:40 +0900 Subject: [PATCH] fix(syntax): implement Carp's reader macros and literals `docs/LanguageGuide.md:170-173` defines `&` and `@` under a literal "Reader Macros" heading -- `&x ;; same as (ref x)`, `@x ;; same as (copy x)` -- but Carp routed through `classify_legacy`, which implements neither. So `@(f x)` split into a bare `@` atom **plus a sibling**, inflating the enclosing call's arity in **116 of 248 files (47%)**, 1493 atoms in all. Byte spans survived, so round-trips were lossless; what broke was structure, which makes any argument-counting analysis unsound for Carp. Also fixed: `@"..."` silently split string literals, because `@` glued to the next token and swallowed the opening quote; character literals were unrecognized, so `\{ \} \[ \] \( \) \"` were read as real delimiters; and `#"..."` Pattern literals were unrecognized. Three more the original brief missed, all real: **`~` (deref) is also a reader macro** (76 glued atoms in 15 files); **`$[...]` static arrays** (55 bare `$` atoms, the same arity-inflating defect); and **`,` is whitespace in Carp, not unquote** (39 sites, where the legacy reader gave `max` and `val` phantom `Unquote` prefixes in `[min, max, val]`). `src/Parsing.hs` settles the shape: `readerMacro` is `string macroStr` then `expr <- sexpr`, recursing into `sexpr` rather than `atom`, so prefixes bind any form, `@@x` stacks, and `@"..."` is a copy of a genuine string literal. `validCharacters` excludes `& ~ @ % $ #`, so a sigil can never be inside a symbol. `aChar` is `\` then one character with no terminator, so `\{` and `\ ` are valid. Corpus differential over all 248 files: parse failures **6 -> 0**, 171 trees changed, **1623 child slots removed** and reconciled token-by-token, **0 files gained atoms**. `%`/`%@` (Carp's real unquote) is deliberately deferred: it needs two new `ReaderPrefix` variants threaded through 40+ sites and *adds* findings inside macro templates, so it wants its own FP audit. Leaving it keeps templates inert, which is the suppressing direction. `r"..."` has zero corpus occurrences. `Ref`/`Copy`/`Deref`/`StaticArray` are new `ReaderPrefix` variants rather than reuses of `Function`, whose `as_source()` is `#'` and which feeds three real re-emission paths -- reusing it would have been latent corruption in place of the old kind. No `carp` binary was obtainable (removed from nixpkgs 2026-02-05 as broken; no ghc/stack/cabal), so the semantics are **grammar-derived from `src/Parsing.hs`, not oracle-checked** -- 20 hand-verified cases. Said plainly rather than implied. Rebased across #96, #98, #100 and #102. The `classify_reader_macro` dispatch table was resolved arm by arm: ours had `Unknown | Hy | Carp` with `Lfe` pulled out by #98, theirs had `Unknown | Lfe | Hy` with `Carp` pulled out -- each side's group line still named the other's dialect, so taking either would have silently deleted a dialect's reader support. A pristine `origin/main` control binary was built and diffed over 36 repo files x 10 dialects plus a 28-case battery: **444 comparisons, 0 differences**. Nothing outside Carp changed. Three things in `lint-carp-idiom` (#95) needed updating, because the reader change was authored before that package existed and it documents these defects as *current behaviour*. One was a real test failure: `the_correct_corpus_exercises_the_readers_arity_inflation` pinned the bug with `assert!(bare > 0)`. Inverted rather than deleted -- it now asserts `bare == 0` *and* `prefixed_lists == 8`, keeping the corpus-coverage pin it existed for, with the 8 derived from the corpus text rather than read off the parser. `edit format` on `@(unsafe-nth world i)` is confirmed fixed by #100 -- that was the original agent's top-ranked follow-up, and #100's `carries_reader_prefix` fix is dialect-agnostic, so Carp's new variants were covered automatically. --- packages/core/syntax/src/sexpr/reader.rs | 13 +- .../core/syntax/src/sexpr/reader_policy.rs | 116 +++++++++++++- .../core/syntax/src/sexpr/tests/parser.rs | 147 ++++++++++++++++++ packages/core/syntax/src/sexpr/tree.rs | 20 +++ .../domain/macro_expansion/mod.rs | 10 +- packages/feature/lint-carp-idiom/README.md | 84 +++++----- .../lint-carp-idiom/src/corpus_tests.rs | 64 ++++++-- .../feature/lint-carp-idiom/src/support.rs | 39 +++-- .../src/macro_hygiene_report/domain.rs | 18 ++- tests/parser_robustness.rs | 8 +- 10 files changed, 441 insertions(+), 78 deletions(-) diff --git a/packages/core/syntax/src/sexpr/reader.rs b/packages/core/syntax/src/sexpr/reader.rs index dd6046b4..08f62cac 100644 --- a/packages/core/syntax/src/sexpr/reader.rs +++ b/packages/core/syntax/src/sexpr/reader.rs @@ -51,13 +51,24 @@ pub fn apply_reader_prefix_context( // binary segment is `(value (size N) (unit 8))` and a map literal's // values are arbitrary forms, so rename and reference tracking must // keep descending into them. + // + // Carp's `&x`/`@x`/`~x`/`$[...]` join them for the same reason and + // more directly: each wraps an ordinary subexpression -- `(ref x)`, + // `(copy x)`, a deref, a static array literal -- so `x` is a live + // reference that rename and reference tracking must still see. + // Treating them as opaque would hide roughly a fifth of every Carp + // file from those commands. ReaderPrefix::HashLiteral | ReaderPrefix::LfeBinary | ReaderPrefix::LfeMap | ReaderPrefix::LfeStruct | ReaderPrefix::Metadata | ReaderPrefix::ReaderConditional - | ReaderPrefix::ReaderConditionalSplicing => {} + | ReaderPrefix::ReaderConditionalSplicing + | ReaderPrefix::Ref + | ReaderPrefix::Copy + | ReaderPrefix::Deref + | ReaderPrefix::StaticArray => {} } } diff --git a/packages/core/syntax/src/sexpr/reader_policy.rs b/packages/core/syntax/src/sexpr/reader_policy.rs index 428939c1..0ead9715 100644 --- a/packages/core/syntax/src/sexpr/reader_policy.rs +++ b/packages/core/syntax/src/sexpr/reader_policy.rs @@ -111,8 +111,16 @@ impl DialectReaderPolicy { } } + /// Whether `byte` separates tokens rather than belonging to one. + /// + /// Carp joins Clojure in treating a comma as whitespace: upstream's + /// `emptyCharacters` is `[space, tab, comma, linebreak, eof, comment]` + /// (`src/Parsing.hs`), so a comma is a separator there exactly as it is in + /// Clojure. Without this it fell through to the shared quote-prefix table + /// and read as an unquote, which is a reader macro Carp does not have. pub(super) const fn is_whitespace(self, byte: u8) -> bool { - byte.is_ascii_whitespace() || matches!(self.dialect, Dialect::Clojure) && byte == b',' + byte.is_ascii_whitespace() + || matches!(self.dialect, Dialect::Clojure | Dialect::Carp) && byte == b',' } pub(super) fn line_comment_width(self, bytes: &[u8], pos: usize) -> Option { @@ -392,6 +400,19 @@ impl DialectReaderPolicy { 2 } Dialect::Clojure if byte == b'\\' => 1, + // Carp spells a character literal `\a`, like Clojure. Upstream's + // `aChar` is `Parsec.char '\\'` followed by one of the named + // characters (`space`, `newline`, `tab`, `backspace`, `return`, + // `formfeed`), `\"`, `\uXXXX`, or `Parsec.anyChar` -- so the + // payload is always at least one character and never a delimiter + // boundary. Without this arm `\{`, `\}`, `\(`, `\)`, `\[`, `\]` + // and `\"` had their payload read as a *real* delimiter, which is + // what made `core/Format.carp` and `examples/json_parser.carp` + // fail to parse outright. The named spellings need no arm of their + // own: consuming `\` plus one character leaves `pace` of `\space` + // to the atom scanner, which stops at the same boundary and yields + // the one atom `\space`. + Dialect::Carp if byte == b'\\' => 1, Dialect::EmacsLisp if byte == b'?' && next == Some(b'\\') => 2, Dialect::EmacsLisp if byte == b'?' => 1, _ => return None, @@ -429,9 +450,10 @@ impl DialectReaderPolicy { let third = bytes.get(pos + 2).copied(); match self.dialect { - Dialect::Unknown | Dialect::Carp => self.classify_legacy(byte, next, third), + Dialect::Unknown => self.classify_legacy(byte, next, third), Dialect::Lfe => self.classify_lfe(bytes, pos), Dialect::Hy => self.classify_hy(byte, next, third), + Dialect::Carp => Self::classify_carp(byte, next), 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), @@ -617,6 +639,96 @@ impl DialectReaderPolicy { } } + /// Carp's reader macros, from the `sexpr` dispatch in `src/Parsing.hs`. + /// + /// Carp used to share [`Self::classify_legacy`] with LFE, Hy and the + /// permissive reader, and the two grammars have almost nothing in common. + /// Upstream dispatches on a single lookahead character: + /// + /// ```text + /// sexpr = do + /// c <- Parsec.lookAhead Parsec.anyChar + /// x <- case c of + /// '&' -> ref + /// '~' -> deref + /// '@' -> copy + /// '\'' -> quote + /// '`' -> quasiquote + /// '%' -> Parsec.try unquoteSplicing <|> unquote + /// '(' -> list + /// '[' -> array + /// '$' -> staticArray + /// '{' -> dictionary + /// _ -> atom + /// ``` + /// + /// where `readerMacro` consumes the sigil and then recurses into `sexpr`, + /// so a sigil prefixes *any* following form -- symbol, list, string + /// literal, or another prefixed form (`@@x`, `&@x`). None of `& ~ @ % $ #` + /// is in upstream's `validCharacters`, so none can occur inside a symbol + /// and each is unambiguously a prefix wherever it appears. + /// + /// Missing these cost more than tidiness. `&` and `@` alone accounted for + /// 1493 bare sigil atoms across 116 of the 248 files in `carp-lang/Carp`, + /// each one an extra sibling that inflated its enclosing call's arity, so + /// no argument-counting analysis was sound for Carp. + /// + /// Two upstream forms are deliberately still not implemented here: + /// + /// * `%` / `%@` (unquote, unquote-splicing). Recognizing them would make + /// the interior of every `` ` `` template read as code rather than data, + /// which *adds* lint findings on macro bodies. That direction needs a + /// per-dialect false-positive audit of its own, and leaving it reads + /// templates as inert data -- the suppressing, safe direction. + /// * `{...}` dictionaries desugar to `(Map.from-array [(Pair.init k v)…])` + /// in upstream's reader. Structurally they are already a brace list here, + /// which is the right shape for a structural tool; the desugaring is a + /// semantic concern. + const fn classify_carp(byte: u8, next: Option) -> Option { + match byte { + // A trailing `\` is a truncated character literal, not a symbol. + // Same contract `classify_clojure`, `classify_scheme` and + // `classify_emacs_lisp` already state for their own spellings: the + // formatter appends a trailing newline, a `\` left as an atom + // claims that newline as its character on the next parse, and + // `format(format(x))` stops converging. The recorded fuzz input + // `fuzz/corpus/format_idempotence/truncated-clojure-char` is + // exactly this byte and found it here too. + b'\\' if next.is_none() => Some(ReaderMacro::UnsupportedDispatch { width: 1 }), + b'&' => prefix(ReaderPrefix::Ref, 1), + b'@' => prefix(ReaderPrefix::Copy, 1), + b'~' => prefix(ReaderPrefix::Deref, 1), + b'\'' => prefix(ReaderPrefix::Quote, 1), + b'`' => prefix(ReaderPrefix::Quasiquote, 1), + // `staticArray` matches the two-byte string `$[`; a `$` anywhere + // else is a parse error upstream, and reading it as an atom here + // is the more permissive of the two options. + b'$' if matches!(next, Some(b'[')) => prefix(ReaderPrefix::StaticArray, 1), + // `#"…"` is a `Pattern` literal and the only `#` form Carp has. + // One dispatch byte plus exactly one datum, which is how + // `classify_clojure` reads its `#"…"` regex literal too, so both + // go through the same string scanner. Upstream's + // `parseInternalPattern` accepts a `"` only as `\"` and every + // other escape as backslash-plus-one, so a backslash-aware scan + // ends the literal on exactly the byte upstream ends it on. + b'#' if matches!(next, Some(b'"')) => Some(ReaderMacro::MultiDatum { + width: 1, + payload_forms: 1, + }), + // `#"` is the *only* `#` form Carp has. `#` is absent from + // upstream's `validCharacters`, so it cannot occur inside a symbol + // either, and `atom` has no branch that accepts it: `#` anywhere + // else is a parse error upstream. Refusing it here keeps that, + // and keeps the robustness suite's out-of-band oracle -- "no + // complete document contains a bare `#+`/`#-` standing alone as + // its own datum" -- true for Carp rather than exempting it. + // Carp inherited `#;`, `#_`, `#+`, `#-`, `#.`, `#'`, `#?`, `#(`, + // `#[` and `#{` from the legacy reader; it has none of them. + b'#' => Some(ReaderMacro::UnsupportedDispatch { width: 1 }), + _ => None, + } + } + fn classify_common_lisp(self, bytes: &[u8], pos: usize) -> Option { let byte = *bytes.get(pos)?; let next = bytes.get(pos + 1).copied(); diff --git a/packages/core/syntax/src/sexpr/tests/parser.rs b/packages/core/syntax/src/sexpr/tests/parser.rs index cadc0e73..1e920642 100644 --- a/packages/core/syntax/src/sexpr/tests/parser.rs +++ b/packages/core/syntax/src/sexpr/tests/parser.rs @@ -2478,3 +2478,150 @@ fn hy_discarded_forms_use_the_same_string_extent() { assert_eq!(children, expected, "{input}"); } } + +// --------------------------------------------------------------------------- +// Carp. Every expectation below is read off the `sexpr` dispatch and the +// `aChar` / `pat` / `emptyCharacters` productions in Carp's own +// `src/Parsing.hs`, not off this reader's previous behaviour: Carp shared the +// permissive legacy reader until now, and that reader implemented none of it. +// --------------------------------------------------------------------------- + +/// `&`, `@` and `~` prefix the *following form*, not just a symbol. +/// +/// This is the defect these tests exist for. `readerMacro` consumes the sigil +/// and recurses into `sexpr`, so `@(g y)` is one form. Reading it as a bare +/// `@` atom plus a sibling inflated the enclosing call's arity by one, which +/// happened 1493 times across 116 of the 248 files in `carp-lang/Carp`. +#[test] +fn carp_sigils_prefix_the_following_form() { + for (input, prefix, expected) in [ + ("(f @(g y))", ReaderPrefix::Copy, "@(g y)"), + ("(f &(g y))", ReaderPrefix::Ref, "&(g y)"), + ("(f ~(g y))", ReaderPrefix::Deref, "~(g y)"), + ("(f @x)", ReaderPrefix::Copy, "@x"), + ("(f &x)", ReaderPrefix::Ref, "&x"), + ("(f $[1 2])", ReaderPrefix::StaticArray, "$[1 2]"), + ] { + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Carp) + .unwrap_or_else(|error| panic!("{input}: {error}")); + let form = &tree.root_view().children[0]; + assert_eq!(form.children.len(), 2, "{input}"); + assert_eq!(form.children[1].span.slice(input), expected, "{input}"); + assert_eq!(form.children[1].reader_prefixes, vec![prefix], "{input}"); + } +} + +/// Sigils stack, because `readerMacro` recurses into `sexpr` rather than into +/// `atom`: `@@x` is `(copy (copy x))` and `&@x` is `(ref (copy x))`. +#[test] +fn carp_sigils_stack_in_source_order() { + let input = "(f &@x)"; + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Carp).expect("valid"); + let form = &tree.root_view().children[0]; + assert_eq!( + form.children[1].reader_prefixes, + vec![ReaderPrefix::Ref, ReaderPrefix::Copy] + ); +} + +/// `@"a b"` is a copy of a *string literal*, so the string stays whole. +/// +/// Because `@` used to glue onto the following token, the opening quote was +/// swallowed and `(f @"a b")` silently became the two atoms `@"a` and `b"`. +#[test] +fn carp_copy_prefix_keeps_a_string_literal_whole() { + let input = r#"(f @"a b")"#; + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Carp).expect("valid"); + let form = &tree.root_view().children[0]; + assert_eq!(form.children.len(), 2); + assert_eq!(form.children[1].span.slice(input), r#"@"a b""#); +} + +/// `aChar` is `\` plus one character, and the character may be a delimiter. +/// +/// `\{` is what made `core/Format.carp` fail to parse: the brace was read as a +/// real delimiter and unbalanced the file. `\ ` is a space character literal +/// and occurs in `core/String.carp`; `\space` needs no separate arm because +/// the atom scanner carries the remaining letters into the same atom. +#[test] +fn carp_character_literals_cover_delimiters_and_named_characters() { + for (input, expected) in [ + ("(f \\a)", "\\a"), + ("(f \\{)", "\\{"), + ("(f \\})", "\\}"), + ("(f \\()", "\\("), + ("(f \\))", "\\)"), + ("(f \\[)", "\\["), + ("(f \\])", "\\]"), + ("(f \\\")", "\\\""), + ("(f \\space)", "\\space"), + ("(f \\ )", "\\ "), + ] { + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Carp) + .unwrap_or_else(|error| panic!("{input}: {error}")); + let form = &tree.root_view().children[0]; + assert_eq!(form.children.len(), 2, "{input}"); + assert_eq!(form.children[1].span.slice(input), expected, "{input}"); + } +} + +/// `#"…"` is a `Pattern` literal: one dispatch byte and one datum. +/// +/// `parseInternalPattern` admits a `"` only as `\"`, so a backslash-aware scan +/// ends the literal where Carp ends it. +#[test] +fn carp_pattern_literal_is_one_span() { + for (input, expected) in [ + (r#"(f #"[a-z]+")"#, r#"#"[a-z]+""#), + (r#"(f #"a\"b")"#, r#"#"a\"b""#), + ] { + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Carp) + .unwrap_or_else(|error| panic!("{input}: {error}")); + let form = &tree.root_view().children[0]; + assert_eq!(form.children.len(), 2, "{input}"); + assert_eq!(form.children[1].span.slice(input), expected, "{input}"); + } +} + +/// A comma is whitespace, not an unquote: `emptyCharacters` lists it beside +/// space and tab. It separates `deftype` fields and `defn` parameters +/// throughout `core/`, e.g. `(deftype Point [x Int, y Int])`. +#[test] +fn carp_comma_is_whitespace() { + let input = "(deftype Point [x Int, y Int])"; + let tree = SyntaxTree::parse_with_dialect(input, Dialect::Carp).expect("valid"); + let fields = &tree.root_view().children[0].children[2]; + assert_eq!(fields.children.len(), 4); + assert!( + fields + .children + .iter() + .all(|child| child.reader_prefixes.is_empty()) + ); +} + +/// Forms Carp's reader has no dispatch for are refused rather than read as +/// something else. +/// +/// `#` is absent from `validCharacters` and `atom` has no branch that accepts +/// it, so every `#` form except `#"…"` is a read error upstream -- including +/// the `#;`, `#_`, `#+`, `#-`, `#.`, `#'`, `#?`, `#(`, `#[` and `#{` Carp +/// inherited from the legacy reader. An unterminated `#"` or `@"` must fail +/// loudly rather than swallow the rest of the file. +#[test] +fn carp_refuses_what_its_reader_has_no_dispatch_for() { + for input in [ + "(f #(g))", + "(f #{1})", + "#+sbcl (f)", + "#;(f)", + "(f #\"abc)", + "(f @\"abc)", + "\\", + ] { + assert!( + SyntaxTree::parse_with_dialect(input, Dialect::Carp).is_err(), + "{input} should be refused" + ); + } +} diff --git a/packages/core/syntax/src/sexpr/tree.rs b/packages/core/syntax/src/sexpr/tree.rs index 4a5b4436..20bc8637 100644 --- a/packages/core/syntax/src/sexpr/tree.rs +++ b/packages/core/syntax/src/sexpr/tree.rs @@ -234,6 +234,22 @@ pub enum ReaderPrefix { ReaderConditional, /// Clojure splicing reader conditional (`#?@(:clj [a] :cljs [b])`). ReaderConditionalSplicing, + /// Carp's `&x`, which its reader expands to `(ref x)`. + /// + /// Carp gets its own variants rather than borrowing [`Self::Function`] -- + /// the "catch-all single-prefix slot" `quote_edit` describes, which already + /// carries Clojure's `@x` and Janet's `|x`. That slot spells itself `#'` + /// through [`Self::as_source`], and `as_source` is not decorative: three + /// re-emission paths write it straight back into source text. Carp is the + /// dialect where three distinct sigils would have collided in that one + /// slot, so each keeps its own spelling. + Ref, + /// Carp's `@x`, which its reader expands to `(copy x)`. + Copy, + /// Carp's `~x`, which its reader expands to a `Deref` node. + Deref, + /// Carp's `$[1 2 3]` static array literal, whose `$` glues to the `[`. + StaticArray, } impl ReaderPrefix { @@ -261,6 +277,10 @@ impl ReaderPrefix { Self::Metadata => "^", Self::ReaderConditional => "#?", Self::ReaderConditionalSplicing => "#?@", + Self::Ref => "&", + Self::Copy => "@", + Self::Deref => "~", + Self::StaticArray => "$", } } diff --git a/packages/feature/inline/src/inline_function/domain/macro_expansion/mod.rs b/packages/feature/inline/src/inline_function/domain/macro_expansion/mod.rs index 9e71d3f5..9596663a 100644 --- a/packages/feature/inline/src/inline_function/domain/macro_expansion/mod.rs +++ b/packages/feature/inline/src/inline_function/domain/macro_expansion/mod.rs @@ -261,13 +261,21 @@ fn render_prefixed_expression( // writes upper case, so this normalises the spelling of a literal // rather than changing it. Every span-based path -- the formatter and // all structural edits -- keeps the original bytes. + // + // Carp's `&`/`@`/`~`/`$` join these: each re-emits its own spelling and + // keeps expanding the form underneath, which is why they carry a real + // `as_source` rather than borrowing another variant's. ReaderPrefix::HashLiteral | ReaderPrefix::LfeBinary | ReaderPrefix::LfeMap | ReaderPrefix::LfeStruct | ReaderPrefix::Metadata | ReaderPrefix::ReaderConditional - | ReaderPrefix::ReaderConditionalSplicing => Ok(format!( + | ReaderPrefix::ReaderConditionalSplicing + | ReaderPrefix::Ref + | ReaderPrefix::Copy + | ReaderPrefix::Deref + | ReaderPrefix::StaticArray => Ok(format!( "{}{}", prefix.as_source(), render_prefixed_expression( diff --git a/packages/feature/lint-carp-idiom/README.md b/packages/feature/lint-carp-idiom/README.md index 36e69177..86b3468a 100644 --- a/packages/feature/lint-carp-idiom/README.md +++ b/packages/feature/lint-carp-idiom/README.md @@ -35,55 +35,67 @@ bodies. It is withheld when the file defines its own `->` or `-->`. ## What this workspace's reader does with Carp Investigating the rules turned up four reader defects, all of them larger than -the rules. They are recorded here because they bound what any Carp rule can -do; fixing them belongs in `core/syntax`, not this package. +the rules. **All four are now fixed in `core/syntax`**, which is where the fix +belonged; they are kept on record here because they are why the rule in this +package is shaped the way it is. -Measured over `carp-lang/Carp` at 248 `.carp` files: +Measured over `carp-lang/Carp` at 248 `.carp` files, as they stood: -**1. `@` and `&` are not reader prefixes.** Carp's guide +**1. `@` and `&` were not reader prefixes.** Carp's guide (`docs/LanguageGuide.md`, "Reader Macros") defines `&x` as `(ref x)` and `@x` -as `(copy x)`. `reader_policy.rs` routes Carp through `classify_legacy`, which -implements neither. So `@x` lexes as a single atom `"@x"`, and `@(f x)` lexes -as a **bare `@` atom followed by a sibling list** — which inflates the +as `(copy x)`. `reader_policy.rs` routed Carp through `classify_legacy`, which +implements neither. So `@x` lexed as a single atom `"@x"`, and `@(f x)` lexed +as a **bare `@` atom followed by a sibling list** — which inflated the enclosing call's arity by one: ```text -(f @(g y)) Carp => 3 children: ["f", "@", "(g y)"] wrong -(f @(g y)) Clojure => 2 children: ["f", "@(g y)"] right +(f @(g y)) was => 3 children: ["f", "@", "(g y)"] wrong +(f @(g y)) now => 2 children: ["f", "@(g y)"] right ``` -1493 such bare sigil atoms occur in 116 of the 248 files. Byte spans stay -intact, so a round trip is lossless — but **no arity or argument-position -analysis is trustworthy for Carp**, which is why the rule here keys on the head -symbol alone. - -**2. A string literal directly after `@` is not lexed as a string.** Because -`@` glues to the following token, `@"…"` is read as an atom that swallows the -opening quote. `(f @"a b")` silently becomes *two* atoms, `@"a` and `b"`, with -no error; `(f @"{")` fails to parse outright. 46 silently split string atoms -occur across 10 files that otherwise parse cleanly, and this is the cause of -three of the six outright parse failures (`core/Map.carp`, `core/Pattern.carp`, -`core/Test.carp`). - -**3. Character literals are not recognized.** Carp spells them `\a`, and -`character_literal_prefix_width` has arms for Scheme, Racket, Clojure and Emacs -Lisp but none for Carp. `\a` and `\space` survive by luck; `\{`, `\}`, `\[`, -`\]`, `\(`, `\)` and `\"` do not, because the delimiter is read as a real -delimiter. This is the cause of `core/Format.carp` (`\{`) and, with defect 2, +1493 such bare sigil atoms occurred in 116 of the 248 files. Byte spans stayed +intact, so a round trip was lossless — but **no arity or argument-position +analysis was trustworthy for Carp**, which is why the rule here keys on the +head symbol alone. It still does: that was never the weaker choice. + +Carp now has its own `classify_carp`, covering `&` (`Ref`), `@` (`Copy`), `~` +(`Deref`) and `$[…]` (`StaticArray`) as well as `'` and `` ` ``. + +**2. A string literal directly after `@` was not lexed as a string.** Because +`@` glued to the following token, `@"…"` was read as an atom that swallowed the +opening quote. `(f @"a b")` silently became *two* atoms, `@"a` and `b"`, with +no error; `(f @"{")` failed to parse outright. 46 silently split string atoms +occurred across 10 files that otherwise parsed cleanly, and this was the cause +of three of the six outright parse failures (`core/Map.carp`, +`core/Pattern.carp`, `core/Test.carp`). Fixed as a consequence of defect 1: `@` +now prefixes the following *form*, so the string stays whole. + +**3. Character literals were not recognized.** Carp spells them `\a`, and +`character_literal_prefix_width` had arms for Scheme, Racket, Clojure and Emacs +Lisp but none for Carp. `\a` and `\space` survived by luck; `\{`, `\}`, `\[`, +`\]`, `\(`, `\)` and `\"` did not, because the delimiter was read as a real +delimiter. This was the cause of `core/Format.carp` (`\{`) and, with defect 2, `examples/json_parser.carp` (`\]`, `\"`). -**4. `#"…"` pattern literals are not recognized.** Carp's `Pattern` type has a -literal syntax, used 36 times in 2 files. It is the cause of +**4. `#"…"` pattern literals were not recognized.** Carp's `Pattern` type has a +literal syntax, used 36 times in 2 files. It was the cause of `test/pattern.carp`. -Together these make **6 of 248 files fail to parse**, four of them in `core/` +Together these made **6 of 248 files fail to parse**, four of them in `core/` — and a file that does not parse is one no command in this tool can say -anything about. - -A fifth, benign observation: Carp's unquote is `%` and its unquote-splicing is -`%@` (`docs/Quasiquotation.md`), neither of which the reader recognizes as a -prefix. Everything textually inside a `` ` `` template therefore reads as data, -which suppresses findings rather than inventing them. +anything about. The corpus now parses at **0 failures**. + +Two further defects were found while fixing the four above: `~` (deref) was +also missing, at 76 glued atoms in 15 files, and `,` is *whitespace* in Carp +(`emptyCharacters` in `src/Parsing.hs`) rather than an unquote, which the +legacy reader had been reading as a phantom `Unquote` prefix at 39 sites. + +A benign observation that still stands: Carp's unquote is `%` and its +unquote-splicing is `%@` (`docs/Quasiquotation.md`), neither of which the +reader recognizes as a prefix. Recognizing them would make the interior of +every `` ` `` template read as code and *add* findings on macro bodies, so it +is deliberately deferred. Everything textually inside a `` ` `` template +therefore reads as data, which suppresses findings rather than inventing them. ## Cost diff --git a/packages/feature/lint-carp-idiom/src/corpus_tests.rs b/packages/feature/lint-carp-idiom/src/corpus_tests.rs index b2a947a5..c9013133 100644 --- a/packages/feature/lint-carp-idiom/src/corpus_tests.rs +++ b/packages/feature/lint-carp-idiom/src/corpus_tests.rs @@ -14,13 +14,14 @@ //! take an owned copy, `Array.reduce` with a `&(fn …)`, `let-do` for //! sequencing, and `-> `/`-->` for threading. //! -//! The correct corpus deliberately includes `@(…)` and `&(…)` forms, which -//! this workspace's reader mis-lexes into an extra sibling atom (see -//! `crate::support`). They belong here precisely because the rule must be -//! immune to that: it keys on the head symbol and never on arity. +//! The correct corpus deliberately includes `@(…)` and `&(…)` forms. The +//! reader used to mis-lex them into an extra sibling atom; it now reads them as +//! reader prefixes (see `crate::support`). They stay here either way, because +//! the rule must be immune to how they lex: it keys on the head symbol and +//! never on arity. use paredit_core_syntax::dialect::Dialect; -use paredit_core_syntax::sexpr::SyntaxTree; +use paredit_core_syntax::sexpr::{ExpressionKind, ExpressionView, ReaderPrefix, SyntaxTree}; use crate::deprecated_thread_macro; use crate::engine_pass_tests::fired; @@ -143,24 +144,55 @@ fn the_dangerous_twin_fires_each_rule_exactly_once() { ); } -/// The correct corpus exercises the reader's `@(…)` / `&(…)` arity inflation. -/// Pinned so a later edit that "simplifies" the corpus does not quietly remove -/// the only coverage of the shape the rule had to be built around. +/// The correct corpus exercises Carp's `@(…)` / `&(…)` reader prefixes. +/// +/// This pin used to run the other way. It asserted that the reader split +/// `@(…)` into a bare `@` atom plus a sibling list, inflating the enclosing +/// call's arity by one, because that is what the reader then did: Carp shared +/// the permissive legacy reader, which implements neither sigil. `core/syntax` +/// now implements Carp's own `sexpr` dispatch, so each sigil is a +/// [`ReaderPrefix`] on the form it prefixes and no bare sigil atom survives. +/// +/// Both halves stay pinned, and they fail in opposite directions: +/// +/// * The zero-bare-sigil half fails if the reader regresses to the old split, +/// which is the shape every rule in this package must stay immune to. +/// * The prefixed-form count fails if a later edit "simplifies" the corpus and +/// quietly removes the only coverage of that shape. The expected 8 is read +/// off `CARP_CORPUS` itself, not off the parser: seven `&(`/`@(` paren forms +/// (the `sig` argument type, three `&(fn …)` arguments, `@(Point.x p)`, and +/// the two `println*` arguments) plus the one `&[1.0 2.0 3.0]` array. #[test] -fn the_correct_corpus_exercises_the_readers_arity_inflation() { +fn the_correct_corpus_exercises_carps_sigil_reader_prefixes() { let tree = SyntaxTree::parse_with_dialect(CARP_CORPUS, Dialect::Carp).expect("parse"); - fn count_bare(view: &paredit_core_syntax::sexpr::ExpressionView, n: &mut usize) { + + fn walk(view: &ExpressionView, bare: &mut usize, prefixed_lists: &mut usize) { for child in &view.children { if matches!(child.text.as_deref(), Some("@") | Some("&")) { - *n += 1; + *bare += 1; + } + if child.kind == ExpressionKind::List + && child + .reader_prefixes + .iter() + .any(|prefix| matches!(prefix, ReaderPrefix::Copy | ReaderPrefix::Ref)) + { + *prefixed_lists += 1; } - count_bare(child, n); + walk(child, bare, prefixed_lists); } } + let mut bare = 0; - count_bare(&tree.root_view(), &mut bare); - assert!( - bare > 0, - "the corpus should contain `@(…)`/`&(…)`, which the reader splits into a bare sigil atom" + let mut prefixed_lists = 0; + walk(&tree.root_view(), &mut bare, &mut prefixed_lists); + + assert_eq!( + bare, 0, + "no bare `@`/`&` sigil atom may survive the Carp reader" + ); + assert_eq!( + prefixed_lists, 8, + "the corpus must keep its `@(…)`/`&(…)` coverage" ); } diff --git a/packages/feature/lint-carp-idiom/src/support.rs b/packages/feature/lint-carp-idiom/src/support.rs index 49afbfbd..9f3f4226 100644 --- a/packages/feature/lint-carp-idiom/src/support.rs +++ b/packages/feature/lint-carp-idiom/src/support.rs @@ -17,26 +17,33 @@ //! @x ;; same as (copy x) //! ``` //! -//! This workspace's reader implements **neither**. `reader_policy.rs` routes -//! Carp through `classify_legacy`, which knows `#;`, `#_`, `#+`, `#-` and the -//! shared quote prefixes, and nothing about `@` or `&`. The consequences are -//! measured and recorded in this package's README; the two that constrain the -//! rules here are: +//! This workspace's reader now implements both, along with `~x` (deref) and +//! `$[…]` (static array). `reader_policy.rs` gives Carp its own +//! `classify_carp` rather than routing it through the permissive +//! `classify_legacy`, which knew `#;`, `#_`, `#+`, `#-` and the shared quote +//! prefixes and nothing about `@` or `&`. //! -//! - `@x` and `&x` lex as *one atom* whose text includes the sigil (`"@x"`), -//! so a head or symbol comparison must not assume the sigil was stripped. -//! - `@(f x)` and `&(f x)` lex as a **bare `@` atom followed by a sibling -//! list**, which inflates the enclosing call's arity by one. Over the -//! upstream corpus that is 1493 sites in 116 of 248 files, so **no rule in -//! this crate may index or count a call's arguments** — the count is not -//! trustworthy for Carp. Every rule here keys on the head symbol alone. +//! That history still constrains the rules here, because the rules were built +//! against the old reading and stay correct under both: +//! +//! - `@x` and `&x` used to lex as *one atom* whose text included the sigil +//! (`"@x"`); they are now the atom `x` carrying a [`ReaderPrefix::Copy`] or +//! [`ReaderPrefix::Ref`]. A head or symbol comparison must therefore not +//! assume either shape — `atom_text` compares the text the reader produced. +//! - `@(f x)` and `&(f x)` used to lex as a **bare `@` atom followed by a +//! sibling list**, inflating the enclosing call's arity by one at 1493 sites +//! in 116 of the upstream corpus's 248 files. They are now a single prefixed +//! form, so arity is trustworthy again — but every rule here still keys on +//! the head symbol alone, which was never weaker than counting arguments. //! //! Carp's unquote is `%` and its unquote-splicing is `%@` //! (`docs/Quasiquotation.md`), neither of which the reader recognizes as a -//! prefix either. [`QuoteState::quasi`] therefore never counts back down for -//! Carp, so everything textually inside a `` ` `` template reads as data. That -//! suppresses findings rather than inventing them, which is the side to be -//! wrong on. +//! prefix; that is a deliberate omission rather than an oversight, since +//! recognizing them would make the interior of every `` ` `` template read as +//! code and *add* findings on macro bodies. [`QuoteState::quasi`] therefore +//! never counts back down for Carp, so everything textually inside a `` ` `` +//! template reads as data. That suppresses findings rather than inventing +//! them, which is the side to be wrong on. use paredit_core_syntax::sexpr::{ ByteSpan, Delimiter, ExpressionKind, ExpressionView, ReaderPrefix, SyntaxTree, diff --git a/packages/feature/lisp-analysis/src/macro_hygiene_report/domain.rs b/packages/feature/lisp-analysis/src/macro_hygiene_report/domain.rs index 450e0588..392e60ef 100644 --- a/packages/feature/lisp-analysis/src/macro_hygiene_report/domain.rs +++ b/packages/feature/lisp-analysis/src/macro_hygiene_report/domain.rs @@ -1551,10 +1551,20 @@ mod tests { Dialect::Hy, "(defmacro m [var form] `(let [,var ,form] ,var))", ), - ( - Dialect::Carp, - "(defmacro m [var form] `(let [,var ,form] ,var))", - ), + // Carp has no row here. It used to, spelling its unquote `,`, and + // it passed only because Carp then shared the permissive legacy + // reader, which read `,` as unquote in every dialect it served. + // Carp's unquote is `%` and its unquote-splicing is `%@` + // (`docs/Quasiquotation.md`; `src/Parsing.hs` dispatches `'%' -> + // try unquoteSplicing <|> unquote`), and `,` is plain whitespace + // there -- `emptyCharacters` lists it beside space and tab. So the + // row asserted a property of a dialect that does not exist. + // + // `%` is not yet a reader prefix here, so there is currently no + // spelling of "an unquoted binding name" in Carp for this test to + // use: a `%var` written today reads as the single atom `%var`, + // which would satisfy the assertion without exercising it. The row + // returns when `%`/`%@` are modelled. ( Dialect::Fennel, "(macro m [var form] `(let [,var ,form] ,var))", diff --git a/tests/parser_robustness.rs b/tests/parser_robustness.rs index 68f75783..4edacc1c 100644 --- a/tests/parser_robustness.rs +++ b/tests/parser_robustness.rs @@ -488,12 +488,16 @@ fn a_feature_conditional_is_never_broken_apart_by_formatting() { // datum, so the `(declare ...)` after it genuinely is a separate top-level // form and separating them is correct. Asserting otherwise would pin a // Common Lisp reading onto a dialect that does not share it. - const FEATURE_CONDITIONAL_DIALECTS: [Dialect; 5] = [ + // Carp is *not* here, and was only ever here because it shared the + // permissive legacy reader. `src/Parsing.hs` gives `#` exactly one meaning + // -- the `#"…"` `Pattern` literal -- and leaves `#` out of + // `validCharacters`, so `#+sbcl` is neither a feature conditional nor a + // symbol in Carp: it is a read error, which the Carp reader now reports. + const FEATURE_CONDITIONAL_DIALECTS: [Dialect; 4] = [ Dialect::Unknown, Dialect::CommonLisp, Dialect::Lfe, Dialect::Hy, - Dialect::Carp, ]; for dialect in FEATURE_CONDITIONAL_DIALECTS {