diff --git a/packages/core/syntax/src/sexpr/parser.rs b/packages/core/syntax/src/sexpr/parser.rs index 193bb4d9c..6aac64974 100644 --- a/packages/core/syntax/src/sexpr/parser.rs +++ b/packages/core/syntax/src/sexpr/parser.rs @@ -848,7 +848,7 @@ impl<'a> Parser<'a> { } self.close_list()?; } - byte if DialectReaderPolicy::is_raw_delimiter(byte) => { + byte if self.policy.is_token_delimiter(byte) => { return Err(self.raw_delimiter_error()); } b'"' => self.atom_string_with_prefixes(prefixes)?, @@ -1030,7 +1030,7 @@ impl<'a> Parser<'a> { position: self.pos.get(), }); } - byte if DialectReaderPolicy::is_raw_delimiter(byte) => { + byte if self.policy.is_token_delimiter(byte) => { return Err(self.raw_delimiter_error()); } b'"' => self.skip_string()?, diff --git a/packages/core/syntax/src/sexpr/reader_policy.rs b/packages/core/syntax/src/sexpr/reader_policy.rs index a21a8b774..5d2ac4924 100644 --- a/packages/core/syntax/src/sexpr/reader_policy.rs +++ b/packages/core/syntax/src/sexpr/reader_policy.rs @@ -194,6 +194,27 @@ 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), + // A Common Lisp script starts `#!/usr/bin/env sbcl --script` or + // `#!/usr/bin/cl -sp asdf -E main`, and the implementation running + // it skips that line: `sbcl --script f` runs a file whose first + // line is a shebang and `sbcl --eval '(load "f")'` on the same + // file signals a reader error, so the skip is what `--script` + // *means*. CLISP's `-script` and CCL's `--load` do the same. + // + // Offset 0 only, exactly as the Emacs Lisp and Hy arms are and for + // the same reason: the skip is a property of the first line of the + // stream, and a `#!` anywhere else stays the reader error CLHS + // 2.4.8 makes it, since `!` has no dispatch function. + // + // 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, 8 of 10046 real Common Lisp files from the nix + // store failed to parse on their first byte, including ASDF's own + // `tools/cl-source-registry-cache.lisp`. + Dialect::CommonLisp 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 @@ -312,6 +333,70 @@ impl DialectReaderPolicy { Delimiter::from_open(byte).is_some() || Delimiter::from_close(byte).is_some() } + /// Whether a bracket pair this dialect does not use as a delimiter is an + /// ordinary symbol constituent rather than a stray-delimiter error. + /// + /// Common Lisp, on the standard's own authority. CLHS Figure 2-7 gives + /// `[`, `]`, `{` and `}` the *constituent* syntax type, and 2.4.2 lists + /// them among the characters "reserved to the user" -- they are not + /// delimiters, they are symbol characters, and real code relies on it. + /// SBCL reads `whitespace[2]p`, `[rsp]`, `$*byte[]` and + /// `print-mov[dq]-opcode` as single symbols; so does every other + /// conforming implementation, because there is nothing implementation + /// defined about it. + /// + /// Without this, `is_atom_boundary` split each of those at its bracket + /// and the parser then met a `[` where no form could start, so the file + /// failed to parse outright -- and a file that does not parse gets none + /// of this tool's lint rules. Over a 10046-file deduplicated Common Lisp + /// corpus from the nix store this was 27 files, the single largest + /// failure cluster, and it included SBCL's own `src/code/reader.lisp`, + /// `src/compiler/x86-64/c-call.lisp` and `contrib/sb-cover/cover.lisp`. + /// + /// ### Why this widens rather than terminates + /// + /// The new atom extent is a strict superset of the old one for every + /// input: the boundary test loses cases, never gains them, so a token + /// that used to be one token cannot become two. Nothing re-emitted can + /// acquire a separator it did not have, which is the shape of corruption + /// this reader has shipped four times. + /// + /// ### Why Scheme and Emacs Lisp are deliberately not here + /// + /// Both would gain only `{` and `}`, since [`Self::allows_delimiter`] + /// already gives them `[` and `]`. Emacs Lisp's case is clear on the + /// source -- `read0`'s terminator test is `strchr ("\"';#()[]`,", c)`, + /// which has no brace, so `a{b}` really is one symbol there. Scheme's is + /// not: R7RS 7.1.1 *reserves* `{` and `}` rather than assigning them a + /// type, Guile's reader treats them as constituents and Chez signals on + /// them. Neither has been measured against its own reader the way this + /// was against SBCL, and a brace is rare enough in both that the change + /// would be an unmeasured rider on a measured fix. Each is worth doing + /// on its own evidence. + pub(super) const fn unused_brackets_are_constituents(self) -> bool { + matches!(self.dialect, Dialect::CommonLisp) + } + + /// Whether `byte` ends the token before it because it delimits a list. + /// + /// The dialect-blind [`Self::is_raw_delimiter`] answers for all three + /// bracket pairs at once. That is right for the dialects that use all + /// three and wrong for the ones that do not, so the dialects opted into + /// [`Self::unused_brackets_are_constituents`] narrow it to the pairs + /// [`Self::allows_delimiter`] actually admits. + pub(super) const fn is_token_delimiter(self, byte: u8) -> bool { + let delimiter = match Delimiter::from_open(byte) { + Some(delimiter) => Some(delimiter), + None => Delimiter::from_close(byte), + }; + match delimiter { + None => false, + Some(delimiter) => { + !self.unused_brackets_are_constituents() || self.allows_delimiter(delimiter) + } + } + } + /// Whether a backtick opens a long string in this dialect. /// /// Janet is the only one. Its `root` state sends every backtick to the @@ -379,7 +464,7 @@ impl DialectReaderPolicy { pub(super) fn is_atom_boundary(self, bytes: &[u8], pos: usize) -> bool { bytes.get(pos).is_none_or(|byte| { self.is_whitespace(*byte) - || Self::is_raw_delimiter(*byte) + || self.is_token_delimiter(*byte) // Dialect first: `self.dialect` is loop-invariant across the // per-byte calls this makes for every atom in the document, so // for the nine dialects without long strings the test folds @@ -1145,6 +1230,81 @@ impl DialectReaderPolicy { } } + /// Scheme's `#`-dispatch table. + /// + /// [`Self::classify_racket`] below records that Racket used to share this + /// function and that the sharing was itself the defect. It also names the + /// debt this arm pays off: "`#'`, `` #` ``, `#,`, `#,@` are R6RS lexical + /// syntax that Guile, Chez and Chicken all accept, so Scheme has a real gap + /// here too, but closing it is a change to a *different* dialect's reader + /// that needs its own Scheme corpus audit rather than a rider on this one." + /// + /// That audit was run. Over 6185 content-deduplicated files from the source + /// trees of seven Scheme implementations — Gerbil, Sagittarius, Gauche, + /// Chibi, Guile, Cyclone and Chez — **2253 (36.4%) failed to parse, and + /// 2135 of those failures (94.8%) were an `UnsupportedReaderDispatch` on + /// `#`**. A file that does not parse is a file no command in this workspace + /// can say anything about, so this capped every Scheme lint rule at + /// two-thirds of its corpus. Guile's own `read` was the oracle: it accepts + /// 3979 of those files, and over the subset both readers accept, the spans + /// `inspect outline` reports already agreed with Guile's data — the reader + /// was not reading Scheme *wrongly*, it was refusing too much of it. + /// + /// | source | reading | handled by | corpus files | + /// |---|---|---|---| + /// | `#;` | datum comment | this arm | — | + /// | `#(…)` | vector | [`ReaderPrefix::HashLiteral`] | — | + /// | `#\c` | character | `character_literal_prefix_width` | — | + /// | `#t` `#f` `#e` `#i` `#b` `#o` `#d` `#x` | booleans, exactness, radix | scan as plain atoms | — | + /// | `#!fold-case` `#!r6rs` `#!eof` | reader directives | scan as plain atoms | — | + /// | `#0=` `#0#` | datum label and reference | [`classify_numeric_dispatch`] | — | + /// | `##name` | Gambit namespace-qualified identifier | this arm | 1045 | + /// | `#:name` | keyword | scans as a plain atom | 299 | + /// | `#vu8(…)` `#u8(…)` `#f64(…)` | R6RS / SRFI-4 homogeneous vector | [`scheme_uniform_vector_width`] | 262 | + /// | `#'x` `` #`x `` `#,x` `#,@x` | R6RS `syntax` / `quasisyntax` / `unsyntax` | this arm | 195 | + /// | `#"…"` | bytevector (R6RS, Racket) / interpolated string (Gauche) | [`ReaderPrefix::HashLiteral`] | 90 | + /// | `#%name` `#3%name` | Chez primitive reference | scans as a plain atom | 14 | + /// | `#*"…"` | bytevector from a string literal | this arm | 21 | + /// | `#/re/` `#[a-z]` `#{sym}` `#< Option { let byte = *bytes.get(pos)?; let next = bytes.get(pos + 1).copied(); @@ -1167,6 +1327,12 @@ impl DialectReaderPolicy { payload_forms: 1, }); } + // `#%name` and `#3%name` before the table: the digit run would + // otherwise reach the `_` arm and be refused, and `#%` shares the arm + // so the two spellings cannot drift apart. + if is_scheme_primitive_reference(bytes, pos) { + return None; + } // `#\` at end of input is a truncated character literal, not the // character literal for nothing. Reading it as a complete atom made // the formatter non-idempotent: it appends a trailing newline, the @@ -1174,11 +1340,27 @@ impl DialectReaderPolicy { // appends another. Common Lisp, Emacs Lisp and Clojure already reject // it through their own escape rules; this makes Scheme and Racket // agree rather than being the two that do not. - if next == Some(b'\\') && bytes.get(pos + 2).is_none() { + if next == Some(b'\\') && third.is_none() { return Some(ReaderMacro::UnsupportedDispatch { width: 1 }); } match next { - Some(b'(') => prefix(ReaderPrefix::HashLiteral, 1), + // `#(1 2)` is a vector and `#"…"` a bytevector in R6RS and Racket, + // an interpolated string in Gauche. All three are one dispatch byte + // glued to the literal after it, which is what `HashLiteral` + // spells, and keeping the payload visible rather than opaque + // matters: `#(a b)` really does contain data a rule may want to + // read. Brackets are deliberately absent — `#[a-z]` is Gauche's + // char-set literal, not a bracketed vector. + Some(b'(' | b'"') => prefix(ReaderPrefix::HashLiteral, 1), + // `#*"…"` is a bytevector written from a string literal (Gauche, + // Sagittarius). The `"` is required: `#*` alone has no meaning, and + // claiming it did would consume whatever followed as a payload. + // `MultiDatum` rather than `HashLiteral` because the dispatch is + // two bytes and `HashLiteral` re-emits itself as one. + Some(b'*') if third == Some(b'"') => Some(ReaderMacro::MultiDatum { + width: 2, + payload_forms: 1, + }), Some( b'\\' | b't' | b'T' | b'f' | b'F' | b'b' | b'B' | b'o' | b'O' | b'd' | b'D' | b'x' | b'X' | b'e' | b'E' | b'i' | b'I', @@ -1239,8 +1421,14 @@ impl DialectReaderPolicy { width: 2 + usize::from(third == Some(b'@')), payload_forms: 1, }), - // What is deliberately *not* here: `##name` (Gambit's namespaced - // identifiers, 257 failures), `#/re/` `#"str"` `#[char-set]` + // `##name`: Gambit's namespace-qualified identifier, and with it + // Gerbil's whole runtime and Chicken/Cyclone's `##core#lambda`. + // Guile's reader rejects `##car`, so the oracle cannot adjudicate + // this arm; it rests on Gambit's documented namespace syntax and + // on the fact that the alternative is not "read it correctly" but + // "fail the file". + Some(b'#') => None, + // What is deliberately *not* here: `#/re/` `#"str"` `#[char-set]` // (Gauche, 68 between them) and `#{sym}#` (Guile's extended // symbols, 3). Guile's reader rejects every one of them, so they // are not Scheme — they are per-implementation extensions that @@ -1955,6 +2143,25 @@ fn opens_sequence(bytes: &[u8], pos: usize) -> bool { matches!(bytes.get(pos), Some(b'(' | b'[' | b'{')) } +/// Whether a Chez Scheme primitive reference — `#%name`, `#2%name`, `#3%name` — +/// starts at `pos`. +/// +/// Chez's reader sends `#%` to `read-symbol` and the digit form to the same +/// place after recording the safety level, so all three are *identifiers*: +/// `#3%vector-copy` names the unsafe primitive `vector-copy`. Returning `None` +/// for them hands the token to the atom scanner, which stops in the right place +/// because neither `#` nor `%` is an atom boundary. +/// +/// The digit run must be tested here rather than in the caller's `match next`, +/// because `next` is the first *digit* for `#3%foo` and the `%` for `#%foo`. +fn is_scheme_primitive_reference(bytes: &[u8], pos: usize) -> bool { + let mut cursor = pos + 1; + while matches!(bytes.get(cursor), Some(byte) if byte.is_ascii_digit()) { + cursor += 1; + } + bytes.get(cursor) == Some(&b'%') +} + /// Whether a `#rx`/`#px` payload starts at `pos`: a string, or a byte string. fn opens_regexp_payload(bytes: &[u8], pos: usize) -> bool { match bytes.get(pos) { diff --git a/packages/core/syntax/src/sexpr/tests/parser.rs b/packages/core/syntax/src/sexpr/tests/parser.rs index 0e41f1c8b..84c6db77f 100644 --- a/packages/core/syntax/src/sexpr/tests/parser.rs +++ b/packages/core/syntax/src/sexpr/tests/parser.rs @@ -3636,32 +3636,35 @@ fn racket_string_terminates_the_token_before_it() { } } -/// Which Racket-only dispatch forms Scheme still refuses. -/// -/// This began as `scheme_reader_is_unchanged_by_the_racket_split`, pinning -/// that the Racket split left Scheme byte-identical. Four entries have since -/// moved out of the refusal list below and into -/// [`scheme_reads_r6rs_syntax_quotation`]: `#'`, `` #` ``, `#,` and `#,@` are -/// R6RS 4.3.5 lexical syntax, the old doc comment flagged them as a *known -/// gap* awaiting "its own Scheme corpus audit", and that audit has now been -/// done — Guile 2.2.7's own `read` accepts all four, and refusing them cost -/// 23 files of a 2356-file corpus. They were moved rather than deleted: the -/// list below is still the load-bearing claim that Scheme is not Racket. -/// -/// Everything remaining is genuinely Racket-only, verified by feeding each -/// spelling to Guile 2.2.7, which rejects every one. +/// Which of the forms Racket gained are Racket's alone, and which Scheme +/// shares. +/// +/// This test was written when Racket was split out of `classify_scheme`, and it +/// pinned every one of those forms as still refused for Scheme. Four of them +/// were pinned as a *known gap* rather than an intended reading, and the +/// comment said so: "`#'`, `` #` ``, `#,` and `#,@` are R6RS lexical syntax +/// that Guile, Chez and Chicken all accept, so this pins a known gap; closing +/// it is a change to Scheme's reader needing its own Scheme corpus audit." +/// +/// That audit has been run — 6185 deduplicated files from seven Scheme +/// implementations, with Guile's own `read` as the oracle — so those four move +/// here from the refused list below, along with `#"…"`. See +/// [`DialectReaderPolicy::classify_scheme`] for the measurements. Everything +/// still in the refused list is genuinely Racket-only, and stays refused. #[test] fn scheme_still_refuses_racket_only_dispatch() { for input in [ - "(f #\"bytes\")", "(f #rx\"a\")", "(f #px\"a\")", "(f #hash((a . 1)))", "(f #s(pt 1))", "(f #&x)", + // `#{sym}` is Chez's gensym syntax and `#{sym}#` Guile's symbol escape + // — two different terminators for the same opener, which is why + // Scheme refuses both rather than guessing an extent. "(f #{1})", - // `#[` is a vector in Racket but not in Scheme: `classify_scheme`'s - // dispatch table admits `#(` alone. + // `#[` is a vector in Racket, and Gauche's char-set literal in Scheme. + // `classify_scheme`'s dispatch table admits `#(` alone. "(f #[1 2])", "(f #3(0))", "#<::new()); let common_lisp = "(let ((f (future (risky)))) (other-work))"; assert_eq!(fired(common_lisp, Dialect::CommonLisp), Vec::<&str>::new()); diff --git a/packages/feature/lint-contract-annotation/src/clojure_pre_post_vacuous/domain.rs b/packages/feature/lint-contract-annotation/src/clojure_pre_post_vacuous/domain.rs index 1a6e8358a..4dd30b24b 100644 --- a/packages/feature/lint-contract-annotation/src/clojure_pre_post_vacuous/domain.rs +++ b/packages/feature/lint-contract-annotation/src/clojure_pre_post_vacuous/domain.rs @@ -417,12 +417,14 @@ mod tests { /// as clean. #[test] fn a_non_clojure_dialect_is_reported_as_unmodelled() { - // `[` is not readable as Common Lisp at all, so the Clojure spelling - // cannot even be parsed as one. The Common Lisp shape closest to it - // still reaches no finding. + // `[`, `]`, `{` and `}` are constituent characters in Common Lisp, not + // delimiters, so the Clojure spelling still parses as Common Lisp — + // just as a run of ordinary symbols rather than a `defn` this rule + // recognizes. The Common Lisp shape closest to it still reaches no + // finding. assert!( SyntaxTree::parse_with_dialect("(defn f [x] {:pre [true]} x)", Dialect::CommonLisp) - .is_err() + .is_ok() ); let tree = diff --git a/packages/feature/lint-contract-annotation/src/lib.rs b/packages/feature/lint-contract-annotation/src/lib.rs index c75b47224..8c3c6cb42 100644 --- a/packages/feature/lint-contract-annotation/src/lib.rs +++ b/packages/feature/lint-contract-annotation/src/lib.rs @@ -350,22 +350,22 @@ mod engine_pass_tests { } /// The Clojure rule, the other way round. It has no dual-syntax source: it - /// needs a `[…]` parameter vector and a `{…}` condition map, and neither - /// the Common Lisp nor the Scheme reader accepts those. So its silence - /// elsewhere is pinned two ways — the Clojure spelling is not readable at - /// all as those dialects, and every reader that *does* accept the bytes - /// reaches no finding. + /// needs a `[…]` parameter vector and a `{…}` condition map, which neither + /// the Scheme reader nor Common Lisp's own `let`/`defn` grammar has a + /// binding-vector or map-literal reading for. Common Lisp's reader does + /// still accept the bytes — `[`, `]`, `{` and `}` are CLHS-constituent + /// characters, not delimiters, so the trigger source reads as a run of + /// ordinary symbols rather than failing outright — which is exactly why + /// it joins the "whichever reader accepts these bytes must still find + /// nothing" loop below instead of a dialect it cannot even parse as. #[test] fn the_clojure_only_rule_fires_on_clojure_and_on_nothing_else() { for (rule, source) in CLOJURE_TRIGGERS { assert_eq!(fired(source, Dialect::Clojure), vec![rule]); - assert!( - !parses_as(source, Dialect::CommonLisp), - "{rule}'s trigger must not even read as Common Lisp" - ); // Whichever of the remaining readers accept these bytes must still // find nothing; the ones that reject them cannot report either. for dialect in [ + Dialect::CommonLisp, Dialect::Racket, Dialect::Scheme, Dialect::EmacsLisp, diff --git a/packages/feature/remove-unused/src/definition_report/domain.rs b/packages/feature/remove-unused/src/definition_report/domain.rs index ad135dcd6..0789624c9 100644 --- a/packages/feature/remove-unused/src/definition_report/domain.rs +++ b/packages/feature/remove-unused/src/definition_report/domain.rs @@ -523,7 +523,12 @@ mod tests { fn parses_reader_syntax_with_the_input_dialect() { let text = "(defun el-reader () [?\\)])\n"; assert!(SyntaxTree::parse_with_dialect(text, Dialect::EmacsLisp).is_ok()); - assert!(SyntaxTree::parse_with_dialect(text, Dialect::CommonLisp).is_err()); + // `[`, `]` and `?` are Common Lisp constituent characters and `\)` a + // single-escaped `)`, so this also parses as Common Lisp -- just as a + // symbol-heavy form rather than a character literal in a vector. The + // point of this test is that removal reads `text` with the *input* + // dialect (Emacs Lisp) rather than either reader's opinion of it. + assert!(SyntaxTree::parse_with_dialect(text, Dialect::CommonLisp).is_ok()); let files = vec![parsed_file("reader.el", Dialect::EmacsLisp, text)]; let reports = collect_unused_definition_candidates(&files).expect("report must build"); diff --git a/packages/feature/remove-unused/src/remove_unused_definition/domain/tests/basic.rs b/packages/feature/remove-unused/src/remove_unused_definition/domain/tests/basic.rs index b42101917..2f69084bb 100644 --- a/packages/feature/remove-unused/src/remove_unused_definition/domain/tests/basic.rs +++ b/packages/feature/remove-unused/src/remove_unused_definition/domain/tests/basic.rs @@ -343,7 +343,12 @@ fn uses_the_input_dialect_for_reader_syntax_during_removal() { let text = "(defun el-reader () [?\\)])\n"; let form = "(defun el-reader () [?\\)])"; assert!(SyntaxTree::parse_with_dialect(text, Dialect::EmacsLisp).is_ok()); - assert!(SyntaxTree::parse_with_dialect(text, Dialect::CommonLisp).is_err()); + // `[`, `]` and `?` are Common Lisp constituent characters and `\)` a + // single-escaped `)`, so this also parses as Common Lisp -- just as a + // symbol-heavy form rather than a character literal in a vector. The + // point of this test is that removal reads `text` with the *input* + // dialect (Emacs Lisp) rather than either reader's opinion of it. + assert!(SyntaxTree::parse_with_dialect(text, Dialect::CommonLisp).is_ok()); let mut item = definition(text, form, "el-reader", DefinitionCategory::Function); item.package = None; let request = RemoveUnusedDefinitionsRequest {