Repository navigation
Conversation
- Add line-by-line ports of the pinned C scanners, selected by the behavioural mode; the existing scan stays the default. - Pin each divergence from C with parity tests covering lookahead bounds, NUL handling, C-locale classes, signed delimiter compare, serialization limits - Register both scanners with the core so it can serve them, and extend the bindall allowlist accordingly. Buckley-Change-Hash: sha256:f4b65a5dcdc7ee8ac224ec50fd203c288f3e5a323abb80dd42b082978971a1a9 Buckley-Change-Stats: files=9 insertions=1897 deletions=4 binaries=0
This branch has not been deployed
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.
What does this PR do?
Registers the bash and html external-scanner ports with the scanner registry, so that the C-faithful core may bind them. With the scanner mode on, each port follows its pinned C scanner (tree-sitter-bash
src/scanner.c, commit a06c2e4; tree-sitter-htmlsrc/scanner.candsrc/tag.h, commit 73a3947). With the mode off, each port makes the same decisions as the port on main. The core serves no public parse, so no default behaviour changes.Why this approach?
Both ports differed from C in many calls of the differential test (200,000 generated calls, seed 1): bash in 4,329 ordinary and 865 valid-symbol-mask calls, html in 11,828 and 21,860. The core is checked against C, so it needs the C behaviour. The default engine uses the same ports and must not change, so the C behaviour sits behind the scanner mode that
internal/scanmodedefines. A registration means: the core may bind this scanner and switch the mode on for its calls. Nothing else reads the mode.The rule for the mode off: the port's
Scan,SerializeandDeserializeequal main's. Each*_scanner.gokeeps main's code and gains the mode flag in its payload, three dispatch lines and onescanmode.RegisterScannercall. The C-faithful code is new and lives inbash_scanner_c.goandhtml_scanner_c.go. The cost is duplicated scanning logic (about 1,100 lines for bash, 440 for html); the gain is that "equal to main" can be read and tested.The reference of the mode is the one stated in
internal/scanmode: the pinned C scanner compiled with signedcharfor a little-endian target.bash. The Go port of bash was a re-implementation, not a transliteration: in the differential test it differed from C in 4,329 ordinary calls and 865 valid-symbol-mask calls of 200,000 (seed 1, made-up saved states off). The cause of the largest group, 1,908 calls where the port reads one byte more than C before it returns, is a structural one. In C a section that ends with
return falseends the scan (for examplescanner.c:879-884, the extglob section, when a)is followed by white space). In the port each section isif scanX() { return true }, so a section that returns false falls through to the later sections, and the next one (bshScanBraceStart,grammars/runtime/bash_scanner.go:1120on the base) reads the next byte. One fix in one place could not remove it, so the mode uses a line-by-line port of C'sscanin a new file, and the existing port stays as the default path.The bash scanner depends on the reference in one place: the heredoc delimiter is an array of
charthat is compared with the lookahead as anint(scanner.c:233), and its saved state writes a 32-bit length withmemcpy. The mode uses signed comparison and a little-endian length on every target.html. The Go port of html uses the tag table of
html_tags.go, which the html, vue, angular, svelte, blade and astro ports share. That table numbers the tags in another order thantag.h, lacks some of its entries (for example COMMAND, ISINDEX, NEXTID and AUDIO) and has another content model for<p>. In the differential test the port differed from C in 11,828 ordinary calls and 21,860 valid-symbol-mask calls of 200,000 (seed 1). Because the saved state holds the tag numbers, the state bytes differ too.Changing the shared table would change the default path of all six ports. This change does not touch it. The C tag table of html is added beside it, in a new file that only the html port uses, and the html scan, serialize and deserialize pick it when the mode is on.
html_tags.goand the other five ports are byte for byte as on main. The cost is a second tag table; the gain is that the default path of six ports is unchanged and "equal to main" can be tested.The html scanner does not depend on either: tag names are upper-cased ASCII, and the saved state holds two 16-bit counts, which the mode writes little-endian on every target.
What this changes
grammars/runtime/bash_scanner_c.go,html_scanner_c.go(new): the C-faithful scans, saved-state codecs and, for html, the C tag table and tag stack.grammars/runtime/bash_scanner.go,html_scanner.go: the mode flag in the payload, three dispatch lines, one registration call each. Nothing else.html_tags.gois not changed.grammars/runtime/{bash,html}_registration_test.go: the registration is present, the declared payload type is whatCreatereturns, the mode is off in a new payload and can be set on and off, and the port binds through the real registration behind 0, 1 and 2 order adapters.grammars/runtime/{bash,html}_scanner_c_parity_test.go: 8 bash tests and 5 html tests without cgo, each naming the C line it mirrors, run with the mode on, with the mode-off behaviour of main pinned beside it. Each fails without its fix: with the dispatch lines disabled, all 13 fail (negative/dispatch-disabled.login the asset).internal/ccore/bind/bindall/bindall_test.go: the list of registered languages gains bash and html.No engine file changes and no root file is added. The CI matrix and
cgo_harness/docker/ccore_equality.share unchanged.bash: differences from main that the mode removes
scanner.c:879-884and everyreturn falseTestBashExtglobReadsNoByteBeyondClexer->eofwhere C says so (:431),lookahead == 0elsewhereTestBashNulIsNotTheEndLikeCiswspace,iswalpha,iswalnum,iswdigitof the C locale (ASCII)unicode.IsSpaceand friendsTestBashWhiteSpaceIsTheCLocaleOPENING_PAREN,ESAC:11-41), never producedTestBashNeverProducesOpeningParenOrEsaccharagainstint(:233)TestBashDelimiterIsSignedLikeCadvance_word:164-190,:210-224)TestBashAdvanceWordAppendsLikeCTREE_SITTER_SERIALIZATION_BUFFER_SIZE1,024 (:101)TestBashSerializeStopsAtC1024deserializeof a state with fewer heredocs:121-157)TestBashDeserializeKeepsExtraHeredocsLikeChtml: differences from main that the mode removes
tag.h:5-166: 126 names,END_OF_VOID_TAGS23,CUSTOM126; counts then one byte per tag (scanner.c:36-72)TestHTMLTagTableAndStateFollowC<p>tag.h:271-276end a paragraph<ul>ends it)TestHTMLParagraphContentModelFollowsC/>with no open tagSTART_TAG_NAME(scanner.c:293-304)TestHTMLSelfClosingWithNoOpenTagReturnsStartTagNameLikeCiswspaceof the C locale (scanner.c:322)TestHTMLWhiteSpaceIsTheCLocaleEND_tags (scanner.c:36-110)TestHTMLSerializeLimitIs1024LikeCCorrectness
Gate results, both architectures
The logs, summary lines, signature files, reproducers and the copy of the equality script behind this section and the next ones are in the asset pr-scanner-bash-html-33d0a.tar.gz (sha256
d16941644433b9382fe28c13aab20ba73890bfe588171b9d2690daf5b45f2260) of theevidencerelease; its README maps each folder to a check.Seed 1, 200,000 generated calls, mode on, one grammar per process, no known-differences file. "Gate" is
SCANDIFF_MODE=on SCANDIFF_BATCH=20000 SCANDIFF_HOSTILE=<off for bash, all for html> cgo_harness/docker/scandiff.sh <grammar>(exit 0 required), with the pinned C scanners and the repository's C runtime. "Equality" iscgo_harness/docker/ccore_equality.sh <grammar>. Each cell is compared / differing. The gate runs were made on the tree of #1625 plus this change; the files of this change are identical in the tree on main0f3af76e.Equality is
failed=0for both grammars on both architectures (bash: 73 trace, 76 digest, 76 record lines; html: 73, 74, 74); the incremental count is 0 because the harness has edit-session sets only for go, json, python, typescript and tsx. Range probes (a diagnostic, not in the gate): bash 1,360 / 0, html 6,223 / 0 in the gate runs.bash and made-up saved states. The gate runs with made-up saved states off. The C scanner of bash reads past a damaged heredoc stack and crashes on made-up states (687 C crashes in the run with them on); a parse cannot produce such a state. The run with them on is in report mode: it found 0 differing in 166,239 ordinary, 31,630 mask and 49 damaged-state calls among the calls where C did not crash.
For comparison, main's port in the same harness (report mode, arm64): bash ordinary 4,329 differing of 165,704 and mask 865 of 32,937; html ordinary 11,828 of 87,786 and mask 21,860 of 102,676.
Linux amd64 is the CI image (Go 1.25.14, gcc 12.2,
charsigned). On Linux arm64 (Go 1.26.3, gcc 14.2)charis unsigned by default, so the C oracle of the arm64 runs was built with-fsigned-charthrough accand ac++script at the front ofPATHof the run; the repository's harness is not changed.scandiff.sh --self-test,TestScandiffSelf*)grammars/runtime,internal/ccore/bind/bindallWhat a gate PASS covers: gate mode rejects any difference in ordinary calls, in valid-symbol-mask calls and in calls that start inside a character. It does not decide on calls with damaged saved states or on included-range probes. A PASS does not show that a port equals its C scanner on every input.
How the mode-off rule was checked
First, the differential test ran twice over the same 200,000 calls on Linux arm64, in report mode: with the mode forced off, and with main's port in place of the registered one. Equal sets of difference signatures mean the two Go scanners gave the same result on every call where either differs from C.
The 20 html signatures are all in the category of made-up (damaged) saved states. Both html runs report the same counts of differing ordinary (11,828), mask (21,860), midchar (0 of 76) and range-probe (743) calls, and differ only in the damaged-state diagnostic (776 against 790). This is not traced (see the open point below).
Second, a direct comparison with no C involved: each port with the mode off against a renamed copy of main's file of the port. The test generates cases from a seed: random sources built from the literals of the file, white space of several kinds, control bytes, non-ASCII letters, invalid and cut-off UTF-8; every byte value 0 to 255 inserted at every position of 150 short bases; the empty input; offsets including the end; valid-symbol masks; and three kinds of start state (fresh, made-up states of 0 to 4,096 bytes including 1,023, 1,024 and 1,025, and states the scanner wrote itself). Each case compares the return of
Scan, the token, the cursor and the farthest byte examined, the payload after the scan and after eachDeserialize, andSerializeinto buffers of 0 to 4,096 bytes. A panic in one scanner and not in the other counts as a difference. For bash, a made-up state whose heredoc length field is above 1 MiB is not compared (see the defect below).Scancalls comparedSerializecalls comparedDeserializecalls comparedControls: with the dispatch line of
Scanforced to the C-faithful path while the mode is off, the same test (4,000 random cases and 12 bases per port) reports 1,939 differing comparisons for bash (21,409 cases) and 20,147 for html (20,129 cases), both on arm64. The comparison is a sample. It is not in the diff because it needs main's file as a copy; its source, runner and logs are in the asset.A defect in the present engine
Main's bash
Deserializeallocates a 32-bit length read from the saved state without checking it against the buffer (bash_scanner.go,make([]byte, delimLen)). With the mode on, the port clamps the length to the bytes given. With the mode off, the port is main's and still allocates. Only a corrupt saved state reaches it; a parse does not produce one. The direct comparison hit it (a test process ran out of memory), which is why that comparison skips such states.Open point: html damaged-state signatures
In the signature check, 20 html signatures (47,289 with the mode off, 47,309 with main's port) appear only with main's port, all in the damaged-saved-state category. The direct comparison, which includes made-up states of 0 to 4,096 bytes, finds no difference between the port with the mode off and main's file, so I suspect the harness (the history of the previous call, or bytes the C scanner reads beyond the state), but I did not trace it. Separately, the calls with damaged saved states that differ from C in the html gate (diagnostic, outside the gate) count 42 of 3,240 on arm64 and 57 of 3,240 on amd64, and vary between runs of the same seed.
Local checks
On Linux amd64 (this host, Go 1.25.1), on the tree of this PR (a copy of main
0f3af76ewith the diff applied), each check as its own job through the single-job Go slot of the host:go test -count=1 ./grammars/runtime ./internal/ccore/bind/bindall: exit 0.-race(-p 1): exit 0.-race -count=20: exit 0.go test . ./grammars ./grammars/runtime ./roottest/bench ./roottest/parse -count=1 -timeout 25m): exit 0.gofmt -lon the changed Go files: no output;go vet ./grammars/runtime ./internal/scanmode ./internal/ccore/bind/bindall: no output; the pinned staticcheck 0.6.1 with CI's package list: no output;go run ./cmd/v1layout -base main: 650 of 650 root files;go run ./cmd/v1guard -base main: 481 language-name uses match their baselines;go run ./cmd/citestplan: ok;-stale-run-names: no stale references;scripts/check_evidence_paths.sh: ok;scripts/check_generated_ownership.sh: ok.go test ./grammars/runtime ./internal/ccore/bind/bindall: ok;go vet: ok.git apply --checkof the diff on a fresh copy of main0f3af76e: ok.Not validated
0f3af76e; the files of this change are identical, and the diff applies to main (git apply --check).., C's does not; the mode follows C, and no unit test pins it.Performance
scripts/run_randomized_benchmarks.sh(GOMAXPROCS=1,-count=1per process, 20 shuffle seeds,-benchtime=750ms,-benchmem); a fixed-order run is not evidencePerformance is not the goal of this PR. With the mode off nothing changes for the existing engines. The counter ledger (
GOMAXPROCS=1 GOWORK=off go run ./cmd/perfcounterledger) printedcounter ledger passed: 412 rows, 206 languageson this tree andcounter ledger passed: 412 rows, 206 languageson main0f3af76e; the outputs are byte-identical; the run on main left the tree clean (git status), and the run on this tree changed no file beyond the diff.Maintainability
Self-review
Review your own diff before requesting review. Walk through it as if you're seeing it for the first time, and keep any self-review comments concise and focused on the non-obvious parts.
The points to review first are
bash_scanner_c.goagainstscanner.c, section by section (the labelsregex,extglob_pattern,expansion_wordandbrace_startof C become the methods thatbshScanCcalls in order), andhtml_scanner_c.goagainstscanner.candtag.h(the tablehtmlCTagNamesmust equalTAG_TYPES_BY_TAG_NAMEin order; the tag type is the index, plus one from the entry after the last void tag, because the enum hasEND_OF_VOID_TAGSin between). The empty-stack/>case of html returns a token with the symbol that C's lexer starts with.Due diligence