Conversation
A C or C++ "//" comment continued with a trailing "\" could lose the code
line below it. The splice happens in translation phase 2, before the
comment is lexed, so the grammar reports one node spanning both lines and
the tool is free to rearrange bytes across the seam. Two routes reach the
same damage. Reflow can park the "\" last on the last emitted line, where
the splice swallows the following line of code. And deleting an interior
line that is nothing but whitespace, which is correct treatment of
comment-interior whitespace, re-points an existing splice onto the code
line with no reflow involved at all, so no column limit is safe.
Compiler-verified: a three-line file went from nm reporting "D _x" to no
symbol, the declaration having become comment text. The second pass then
reads the swallowed code as prose and packs it, so --check converges on
the damage instead of reporting it.
parse::spans_line_splice forces passthrough, beside spans_bare_cr and for
the reason that one already states: it destroys code, no amount of
downstream care can undo it, so refuse the comment outright. Gated to C
and C++ line comments, because Rust has no line splicing, shell comments
end at the newline, assembly claims only "/* */", and "*/" terminates a
block whatever precedes the newline.
The rule for "this line is continued" had three encodings in two files
and they disagreed: two spelled it trim_end().ends_with('\\'), which
strips Unicode whitespace, so a "\" followed by a no-break space was a
splice to two callers and not to the third. No compiler splices that
either, so the strict horizontal-whitespace form is the one that
survived, as parse::ends_with_line_splice, and all three callers use it.
Transform 5 no longer inserts a blank line under a "#!" shebang. It
exempted only file offset 0, so a shebang on line 1 made the line-2
header flush against code and detached it, on essentially every shell
script in existence, and split a "#! nix-shell -i bash" run off the
shebang it belongs to. "#!" and "#if" are now one "#"-directive parser
rather than two six lines apart, which is what left the BOM strip on only
one of the pair: U+FEFF is not Unicode White_Space, so a BOM'd file whose
first line is "#ifndef GUARD" failed a test its BOM-free twin passes.
The corpus harness asserted every file's output contained the literal
"int f(int a, int b);", which no Rust, shell, or assembly fixture could
satisfy. Each input is now compared byte for byte against a recorded
X.EXT.expected sibling, read with expect so a deleted sibling fails
instead of silently skipping, and any file detect_language rejects other
than a sibling is a hard error rather than a quiet skip. Recorded output
rather than the simpler "input is its own fixed point": that form can
only hold shapes the tool already leaves alone, so every input the tool
is supposed to fix, which is the whole premature-wrap failure mode, would
have been inexpressible. tests/corpus grows from one C header to one file
per supported language, carrying the shapes that have historically broken
this tool: CRLF throughout, a BOM before a shebang, nested Rust blocks,
a macro continuation chain, unterminated blocks at EOF, fenced code,
tables, and art. Two fixtures hold the splice shapes above.
The two transforms that read a function_definition rather than a comment
move to src/signature.rs. They are the only sanctioned uses of syntactic
context beyond locating comments, and giving them a name and an address
makes the ceiling TODO.md sets something a reviewer can point at. It is
documentation, not a wall: both are still called from parse.
ASan and UBSan jobs, since src/ has no unsafe and the only memory either
can find a bug in is the linked C. CFLAGS is the load-bearing half, so
each job ends by grepping the built grammar objects for instrumentation;
a job reporting zero findings because it checked nothing is worse than no
job. UBSan uses -fsanitize-trap rather than -fno-sanitize-recover: the
runtime form does not link, because rustc drives the final link and never
adds libclang_rt.ubsan. Both carry continue-on-error until they have run
green twice, which puts that promise in the file it constrains instead of
in a branch-protection setting no reviewer of the workflow can see.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A C or C++ "//" comment continued with a trailing "" could lose the code line below it. The splice happens in translation phase 2, before the comment is lexed, so the grammar reports one node spanning both lines and the tool is free to rearrange bytes across the seam. Two routes reach the same damage. Reflow can park the "" last on the last emitted line, where the splice swallows the following line of code. And deleting an interior line that is nothing but whitespace, which is correct treatment of comment-interior whitespace, re-points an existing splice onto the code line with no reflow involved at all, so no column limit is safe. Compiler-verified: a three-line file went from nm reporting "D _x" to no symbol, the declaration having become comment text. The second pass then reads the swallowed code as prose and packs it, so --check converges on the damage instead of reporting it.
parse::spans_line_splice forces passthrough, beside spans_bare_cr and for the reason that one already states: it destroys code, no amount of downstream care can undo it, so refuse the comment outright. Gated to C and C++ line comments, because Rust has no line splicing, shell comments end at the newline, assembly claims only "/* /", and "/" terminates a block whatever precedes the newline.
The rule for "this line is continued" had three encodings in two files and they disagreed: two spelled it trim_end().ends_with('\'), which strips Unicode whitespace, so a "" followed by a no-break space was a splice to two callers and not to the third. No compiler splices that either, so the strict horizontal-whitespace form is the one that survived, as parse::ends_with_line_splice, and all three callers use it.
Transform 5 no longer inserts a blank line under a "#!" shebang. It exempted only file offset 0, so a shebang on line 1 made the line-2 header flush against code and detached it, on essentially every shell script in existence, and split a "#! nix-shell -i bash" run off the shebang it belongs to. "#!" and "#if" are now one "#"-directive parser rather than two six lines apart, which is what left the BOM strip on only one of the pair: U+FEFF is not Unicode White_Space, so a BOM'd file whose first line is "#ifndef GUARD" failed a test its BOM-free twin passes.
The corpus harness asserted every file's output contained the literal "int f(int a, int b);", which no Rust, shell, or assembly fixture could satisfy. Each input is now compared byte for byte against a recorded X.EXT.expected sibling, read with expect so a deleted sibling fails instead of silently skipping, and any file detect_language rejects other than a sibling is a hard error rather than a quiet skip. Recorded output rather than the simpler "input is its own fixed point": that form can only hold shapes the tool already leaves alone, so every input the tool is supposed to fix, which is the whole premature-wrap failure mode, would have been inexpressible. tests/corpus grows from one C header to one file per supported language, carrying the shapes that have historically broken this tool: CRLF throughout, a BOM before a shebang, nested Rust blocks, a macro continuation chain, unterminated blocks at EOF, fenced code, tables, and art. Two fixtures hold the splice shapes above.
The two transforms that read a function_definition rather than a comment move to src/signature.rs. They are the only sanctioned uses of syntactic context beyond locating comments, and giving them a name and an address makes the ceiling TODO.md sets something a reviewer can point at. It is documentation, not a wall: both are still called from parse.
ASan and UBSan jobs, since src/ has no unsafe and the only memory either can find a bug in is the linked C. CFLAGS is the load-bearing half, so each job ends by grepping the built grammar objects for instrumentation; a job reporting zero findings because it checked nothing is worse than no job. UBSan uses -fsanitize-trap rather than -fno-sanitize-recover: the runtime form does not link, because rustc drives the final link and never adds libclang_rt.ubsan. Both carry continue-on-error until they have run green twice, which puts that promise in the file it constrains instead of in a branch-protection setting no reviewer of the workflow can see.
Summary by cubic
Prevents code loss from backslash‑continued C/C++
//comments by refusing to transform them and by using one strict line‑splice rule. Unifies#!and#parsing, grows the corpus with per‑language fixtures, and adds sanitizer jobs; signature‑aware code moves behind a crate‑private module and the parameter‑drift rule tightens.// … \comments are now force_passthrough; splice detection is standardized asends_with_line_splice(ASCII horizontal whitespace only) and reused where macro/blank‑line logic needs it.#parsing: one parser for both; Shell‑only shebang gate avoids treating#![no_std]as a shebang; no blank line under a line‑1#!; BOMs are stripped for both shebangs and#guards.src/signature.rs; parameter‑drift now fires only when exactly one block comment trails); the manpage hoist is unchanged.*.expectedsibling byte‑for‑byte; missing siblings and orphaned*.expectedfail the run. New fixtures cover C/C++/Rust/Shell/assembly, including CRLF, BOM before shebang, nested blocks, macro continuations, unterminated blocks, ASCII art/tables, and backslash‑splice cases.findand reports what exists. UBSan links its runtime via-Clinker=clang -Clink-arg=-fsanitize=undefined, greps teed test output withpipefail, and disables only the upstream-fsanitize=functioncheck. Both jobs remain continue‑on‑error until stabilized.Written for commit def11e2. Summary will update on new commits.