From 8e9a083f533da43447d7b9a3edb0286a30fefb42 Mon Sep 17 00:00:00 2001 From: Oscar Villavicencio <9220505+odvcencio@users.noreply.github.com> Date: Mon, 24 Aug 2026 08:46:49 -0700 Subject: [PATCH 1/2] fix(parser): Fix PHP issue #454 recovery leaf error clearing - Track direct lexer acceptance using a new token flag. - Require a source-bearing parsed prefix to clear ordinary leaf errors. - Reject pending, missing, error, dirty, and invalid payloads for clearance. - Block clearance for generated, external, no-lookahead, and error-mode tokens. - Update PHP issue #454 parity tests to validate raw, production, and locked-C digests. - Document that PHP compact admission remains open due to persistent fallback routing. Refs #454 Buckley-Change-Hash: sha256:12248e721d4385f28f84b58df6a9d4e67437e9475901714469f96bacf43c9938 Buckley-Change-Stats: files=7 insertions=400 deletions=62 binaries=0 --- CHANGELOG.md | 22 +- cgo_harness/php_issue454_parity_test.go | 58 ++++- docs/issue-454-compact-correctness-blocker.md | 58 ++--- lexer.go | 4 + parser_dfa_token_source_test.go | 6 + parser_recover_c.go | 72 +++++- parser_recover_c_leaf_policy_test.go | 242 ++++++++++++++++++ 7 files changed, 400 insertions(+), 62 deletions(-) create mode 100644 parser_recover_c_leaf_policy_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f6badef..f04e658e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1044,17 +1044,19 @@ for tags and release notes while still in `0.x`. `docs/root-normalization-retirement.md` for current artifact hashes and reopening conditions. -- Hardened the PHP issue #454 compact fallback guard at base commit - `c25686c882affd7408e5ef4a7d65e92cc8391fab`. The 140,287-byte edited - witness produced the pinned production and compact Go deep digest - `4456730ce6919a623dd6db2e6ae7f11933aeb454c7e337b7da5c08a8d9ba267c`. - The locked-C `gts-deep-tree-v1` digest is +- Corrected the PHP issue #454 recovery-leaf flags at base commit + `55681868d3a23971d042f9f79083fd6d39c7e33b`. A recovery region now needs a + source-bearing parsed prefix. Each cleared leaf also needs positive internal + deterministic finite automaton (DFA) provenance. + The prefix proof rejects pending, missing, error, dirty, and invalid payloads. + It also rejects payloads that end after the current token starts. + Recovery rejects end-of-input, zero-width, generated, external, missing, + no-lookahead, and error-mode tokens. The raw, production, compact fallback, + and locked-C `gts-deep-tree-v1` digests now equal `1516308c38163089778464ad171875308c559af11af7c8c03ee17ae4eacd23c6`. - The digest covers byte and point spans, fields, extra and missing flags, - error flags, and root `HasError`. Both Go routes differ from locked C. - Compact recorded `routed=0` and `fallback=1`. Two one-CPU, 4 GiB Docker - runs passed with one test worker, a 20-minute timeout, `GOMAXPROCS=1`, and - `GOFLAGS=-p=1`. Keep issue #454 open. Ship no parser change. See + Compact still records `routed=0` and `fallback=1` on recovery. This change + does not graduate PHP compact admission. The incremental memory budget, + resident set size, and remaining issue #454 performance work stay open. See `docs/issue-454-compact-correctness-blocker.md`. - Reject the issue #454 generic recovery candidate from PR #793. The candidate diff --git a/cgo_harness/php_issue454_parity_test.go b/cgo_harness/php_issue454_parity_test.go index 07a179bc..6d74dcfd 100644 --- a/cgo_harness/php_issue454_parity_test.go +++ b/cgo_harness/php_issue454_parity_test.go @@ -23,8 +23,9 @@ const issue454PHPGrammarBlobSHA256 = "15724627db479c27304b43fa3b5ef7d8d81f85e3b9 const issue454PHPCArtifactSHA256 = "1daea60ac1ee31227b8e1ed3cbd76b841435fe693e95af65cc61dad447d27891" const ( - issue454PHPProductionDeepDigest = "4456730ce6919a623dd6db2e6ae7f11933aeb454c7e337b7da5c08a8d9ba267c" - issue454PHPCompactDeepDigest = "4456730ce6919a623dd6db2e6ae7f11933aeb454c7e337b7da5c08a8d9ba267c" + issue454PHPRawDeepDigest = "1516308c38163089778464ad171875308c559af11af7c8c03ee17ae4eacd23c6" + issue454PHPProductionDeepDigest = "1516308c38163089778464ad171875308c559af11af7c8c03ee17ae4eacd23c6" + issue454PHPCompactDeepDigest = "1516308c38163089778464ad171875308c559af11af7c8c03ee17ae4eacd23c6" issue454PHPLockedCDeepDigest = "1516308c38163089778464ad171875308c559af11af7c8c03ee17ae4eacd23c6" issue454PHPRootHasError = true ) @@ -44,6 +45,17 @@ func TestParityIssue454PHPWholeTreeFallback(t *testing.T) { } goLang := grammars.PhpLanguage() + rawParser := gotreesitter.NewParser(goLang) + rawParser.SetAdmissionCandidateRoute(false) + rawTree, err := rawParser.ParseNoResultCompatibilityBenchmarkOnly(edited) + if err != nil { + t.Fatal(err) + } + defer releaseGoTree(rawTree) + if rawTree.ParseStoppedEarly() { + t.Fatalf("raw Go parse stopped early: %s", rawTree.ParseRuntime().Summary()) + } + goParser := gotreesitter.NewParser(goLang) goParser.SetAdmissionCandidateRoute(false) goTree, err := goParser.Parse(edited) @@ -87,6 +99,10 @@ func TestParityIssue454PHPWholeTreeFallback(t *testing.T) { t.Fatal("C reference parser returned nil tree") } defer cTree.Close() + rawInspection, err := benchfixtures.InspectGoTree(rawTree.RootNode(), goLang) + if err != nil { + t.Fatal(err) + } productionInspection, err := benchfixtures.InspectGoTree(goTree.RootNode(), goLang) if err != nil { t.Fatal(err) @@ -95,22 +111,39 @@ func TestParityIssue454PHPWholeTreeFallback(t *testing.T) { if err != nil { t.Fatal(err) } + if rawInspection.SHA256 != issue454PHPRawDeepDigest { + t.Fatalf("PHP issue #454 raw gts-deep-tree-v1 digest=%s, want %s", rawInspection.SHA256, issue454PHPRawDeepDigest) + } if productionInspection.SHA256 != issue454PHPProductionDeepDigest { t.Fatalf("PHP issue #454 production gts-deep-tree-v1 digest=%s, want %s", productionInspection.SHA256, issue454PHPProductionDeepDigest) } if cDeepDigest != issue454PHPLockedCDeepDigest { t.Fatalf("PHP issue #454 locked-C gts-deep-tree-v1 digest=%s, want %s", cDeepDigest, issue454PHPLockedCDeepDigest) } + if rawInspection.SHA256 != productionInspection.SHA256 || productionInspection.SHA256 != cDeepDigest { + t.Fatalf("PHP issue #454 digests differ: raw=%s production=%s locked-C=%s", rawInspection.SHA256, productionInspection.SHA256, cDeepDigest) + } + if got := rawTree.RootNode().HasError(); got != issue454PHPRootHasError { + t.Fatalf("PHP issue #454 raw root HasError=%t, want %t", got, issue454PHPRootHasError) + } if got := goTree.RootNode().HasError(); got != issue454PHPRootHasError { t.Fatalf("PHP issue #454 production root HasError=%t, want %t", got, issue454PHPRootHasError) } if got := cTree.RootNode().HasError(); got != issue454PHPRootHasError { t.Fatalf("PHP issue #454 locked-C root HasError=%t, want %t", got, issue454PHPRootHasError) } - if productionInspection.SHA256 == cDeepDigest { - t.Fatal("PHP issue #454 production deep digest unexpectedly matches locked C; review the NO-GO receipt before changing route policy") + if diff := FirstDivergenceDumpV1(rawTree.RootNode(), goLang, cTree.RootNode()); diff != nil { + t.Fatalf("PHP issue #454 raw tree differs from the pinned C oracle: %+v", *diff) + } + if diff := FirstDivergenceDumpV1(goTree.RootNode(), goLang, cTree.RootNode()); diff != nil { + t.Fatalf("PHP issue #454 production tree differs from the pinned C oracle: %+v", *diff) } + var rawErrs []string + compareNodes(rawTree.RootNode(), goLang, cTree.RootNode(), "root", &rawErrs) + if len(rawErrs) > 0 { + t.Fatalf("PHP issue #454 raw tree differs from the pinned C oracle: %s", rawErrs[0]) + } var errs []string compareNodes(goTree.RootNode(), goLang, cTree.RootNode(), "root", &errs) if len(errs) > 0 { @@ -153,10 +186,10 @@ func TestParityIssue454PHPWholeTreeFallback(t *testing.T) { if got := compactTree.RootNode().HasError(); got != issue454PHPRootHasError { t.Fatalf("PHP issue #454 compact root HasError=%t, want %t", got, issue454PHPRootHasError) } - if compactInspection.SHA256 == cDeepDigest { - t.Fatal("PHP issue #454 compact deep digest unexpectedly matches locked C; review the NO-GO receipt before changing route policy") + if compactInspection.SHA256 != cDeepDigest { + t.Fatalf("PHP issue #454 compact and locked-C deep digests differ: compact=%s locked-C=%s", compactInspection.SHA256, cDeepDigest) } - t.Logf("deep_digest format=%s production=%s compact=%s locked_c=%s production_root_has_error=%t compact_root_has_error=%t locked_c_root_has_error=%t exact_locked_c=false fields=type+named,field,byte+point,extra+missing+error+has_error,child_order", benchfixtures.DeepTreeDigestVersion, productionInspection.SHA256, compactInspection.SHA256, cDeepDigest, goTree.RootNode().HasError(), compactTree.RootNode().HasError(), cTree.RootNode().HasError()) + t.Logf("deep_digest format=%s raw=%s production=%s compact=%s locked_c=%s raw_root_has_error=%t production_root_has_error=%t compact_root_has_error=%t locked_c_root_has_error=%t exact_locked_c=true fields=type+named,field,byte+point,extra+missing+error+has_error,child_order", benchfixtures.DeepTreeDigestVersion, rawInspection.SHA256, productionInspection.SHA256, compactInspection.SHA256, cDeepDigest, rawTree.RootNode().HasError(), goTree.RootNode().HasError(), compactTree.RootNode().HasError(), cTree.RootNode().HasError()) t.Logf("PHP issue #454 compact route source_sha256=%x bytes=%d routed=%d fallback=%d reason=%q", sha256.Sum256(edited), len(edited), routedAfter-routedBefore, fallbackAfter-fallbackBefore, gotreesitter.AdmissionCandidateLastFallbackReason()) document, err := os.ReadFile("../docs/issue-454-compact-correctness-blocker.md") if err != nil { @@ -164,17 +197,18 @@ func TestParityIssue454PHPWholeTreeFallback(t *testing.T) { } documentText := strings.Join(strings.Fields(string(document)), " ") for _, marker := range []string{ - "## 2026-08-24 PHP compact fallback guard", - "Publication base: `c25686c882affd7408e5ef4a7d65e92cc8391fab`.", + "## 2026-08-24 PHP recovery-leaf correction", + "Candidate base: `55681868d3a23971d042f9f79083fd6d39c7e33b`.", "The locked-C artifact SHA-256 is `1daea60ac1ee31227b8e1ed3cbd76b841435fe693e95af65cc61dad447d27891`.", + issue454PHPRawDeepDigest, issue454PHPProductionDeepDigest, issue454PHPCompactDeepDigest, issue454PHPLockedCDeepDigest, "The `gts-deep-tree-v1` stream covers type and named identity, incoming fields, byte and point spans, and child order. It also covers extra and missing flags, error flags, and the `HasError` flag.", - "The production and compact deep digests differ from the locked-C digest.", - "All three roots report `HasError=true`.", + "The raw, production, compact fallback, and locked-C deep digests are equal.", + "All four roots report `HasError=true`.", "The compact route recorded `routed=0` and `fallback=1`.", - "This guard does not graduate PHP compact admission.", + "This correction does not graduate PHP compact admission.", } { marker = strings.Join(strings.Fields(marker), " ") if !strings.Contains(documentText, marker) { diff --git a/docs/issue-454-compact-correctness-blocker.md b/docs/issue-454-compact-correctness-blocker.md index 81da7693..d6e2d166 100644 --- a/docs/issue-454-compact-correctness-blocker.md +++ b/docs/issue-454-compact-correctness-blocker.md @@ -1,6 +1,7 @@ # Issue #454 compact-parser correctness blocker -Status: **NO-GO**. Ship no parser change from this investigation. Keep issue +Status: **KEEP LIVE**. The PHP recovery-leaf candidate restores locked-C +correctness. Compact admission and performance work remain open. Keep issue [#454](https://github.com/odvcencio/gotreesitter/issues/454) open. ## Scope @@ -205,14 +206,23 @@ is not safe without wider recovery validation. The 1 KiB known-divergence ratchet passes in `/tmp/gts-issue454-artifacts-rebase/20260822T230037Z-issue454-c-1k-ratchet-20260822`. -## 2026-08-24 PHP compact fallback guard +## 2026-08-24 PHP recovery-leaf correction -Publication base: `c25686c882affd7408e5ef4a7d65e92cc8391fab`. +Candidate base: `55681868d3a23971d042f9f79083fd6d39c7e33b`. -Status: **KEEP LIVE / NO-GO**. Keep issue #454 open. Ship no parser change. +Status: **CORRECTNESS FIX / KEEP LIVE**. Keep issue #454 open. -The focused guard extends the existing PHP issue #454 parity test. It checks -the compact candidate route against the production and locked-C trees. +The correction adds positive internal deterministic finite automaton (DFA) +provenance to tokens. Recovery requires a source-bearing parsed stack prefix. +It then records a region proof only for a direct, visible, named DFA token. +The prefix proof rejects pending, missing, error, dirty, and invalid payloads. +It rejects state mismatches and payloads that end after the current token starts. +It rejects the end-of-input symbol and zero-width tokens. It also rejects +generated, external, missing, no-lookahead, and error-mode tokens. + +Each later absorbed token must carry its own positive DFA proof. A region +proof cannot clear a later token by itself. A skipped-prefix token can qualify +only when the internal DFA produced that token directly. The edited PHP source has 140,287 bytes. Its SHA-256 is `cbf52f81ea212353a3bf04d7c9b37668b5cdfb6cd428c2d0cb3799a8e13ae82f`. @@ -222,13 +232,6 @@ The embedded PHP grammar blob SHA-256 is The locked-C artifact SHA-256 is `1daea60ac1ee31227b8e1ed3cbd76b841435fe693e95af65cc61dad447d27891`. -The compact route recorded `routed=0` and `fallback=1`. -Its fallback reason was: - -```text -compact route declined at recovery [mechanism=recovery-entered]: did not accept EOF: generic scheduler has no table action for the elected token -``` - The `gts-deep-tree-v1` stream covers type and named identity, incoming fields, byte and point spans, and child order. It also covers extra and missing flags, error flags, and the `HasError` flag. @@ -237,25 +240,18 @@ The pinned deep digests are: | Route | Deep digest | Root `HasError` | | --- | --- | --- | -| Production Go | `4456730ce6919a623dd6db2e6ae7f11933aeb454c7e337b7da5c08a8d9ba267c` | `true` | -| Compact fallback Go | `4456730ce6919a623dd6db2e6ae7f11933aeb454c7e337b7da5c08a8d9ba267c` | `true` | +| Raw Go | `1516308c38163089778464ad171875308c559af11af7c8c03ee17ae4eacd23c6` | `true` | +| Production Go | `1516308c38163089778464ad171875308c559af11af7c8c03ee17ae4eacd23c6` | `true` | +| Compact fallback Go | `1516308c38163089778464ad171875308c559af11af7c8c03ee17ae4eacd23c6` | `true` | | Locked C | `1516308c38163089778464ad171875308c559af11af7c8c03ee17ae4eacd23c6` | `true` | -All three roots report `HasError=true`. - -The compact tree equals the production tree. The production and compact deep -digests differ from the locked-C digest. This full comparison keeps the PHP -route at **NO-GO**. - -The guard passed twice with one CPU, 4 GiB, one test worker, a 20-minute -timeout, `GOMAXPROCS=1`, and `GOFLAGS=-p=1`. Both runs had no out-of-memory -kill and no wall timeout. - -The final artifacts are: +The raw, production, compact fallback, and locked-C deep digests are equal. +All four roots report `HasError=true`. -- `/tmp/gotreesitter-php454-deep-guard-refresh-artifacts/20260824T124547Z-php454-refresh-1` -- `/tmp/gotreesitter-php454-deep-guard-refresh-artifacts/20260824T124620Z-php454-refresh-2` +The compact route recorded `routed=0` and `fallback=1`. Its fallback reason +still reports `mechanism=recovery-entered`. The compact parser does not accept +this recovery path yet. -This guard does not graduate PHP compact admission. -Reopen the route only after a generic recovery proof removes the fallback, -matches both Go route digests to locked C, and preserves all deep-digest fields. +This correction does not graduate PHP compact admission. It does not close +the remaining issue #454 performance work. The incremental memory-budget +fallback and the large-file resident-set-size target also remain open. diff --git a/lexer.go b/lexer.go index 3491c6c5..a74d0fee 100644 --- a/lexer.go +++ b/lexer.go @@ -38,6 +38,9 @@ type Token struct { // lexerErrorModeLexed proves that the active DFA source produced this // recovery token while parser state zero selected the error lex mode. lexerErrorModeLexed bool + // lexerInternalDFALexed proves that Lexer.scan accepted this token from + // the internal DFA. External, generated, missing, and EOF tokens omit it. + lexerInternalDFALexed bool } func bytesToStringNoCopy(b []byte) string { @@ -418,6 +421,7 @@ func (l *Lexer) scan(startState uint32, startPos int, startRow, startCol uint32) EndPoint: Point{Row: acceptRow, Column: acceptCol}, lexerSkippedPrefix: skippedPrefix, lexerSkippedPrefixStart: uint32(startPos), + lexerInternalDFALexed: true, }, true } diff --git a/parser_dfa_token_source_test.go b/parser_dfa_token_source_test.go index 91e4980e..cf29f7b3 100644 --- a/parser_dfa_token_source_test.go +++ b/parser_dfa_token_source_test.go @@ -1628,6 +1628,9 @@ func TestNextDFATokenPrefersParserValidZeroWidthStartAccept(t *testing.T) { if got, want := tok.EndByte, uint32(0); got != want { t.Fatalf("token end = %d, want %d", got, want) } + if tok.lexerInternalDFALexed { + t.Fatal("synthetic zero-width start token has internal-DFA provenance") + } } func TestNextDFATokenSynthesizesGeneratedNULSentinelLookahead(t *testing.T) { @@ -1675,6 +1678,9 @@ func TestNextDFATokenSynthesizesGeneratedNULSentinelLookahead(t *testing.T) { if tok.StartByte != 0 || tok.EndByte != 0 { t.Fatalf("token span = %d..%d, want zero-width at 0", tok.StartByte, tok.EndByte) } + if tok.lexerInternalDFALexed { + t.Fatal("generated NUL token has internal-DFA provenance") + } } func TestNextDFATokenDoesNotSynthesizeGeneratedNULSentinelOverValidToken(t *testing.T) { diff --git a/parser_recover_c.go b/parser_recover_c.go index 9c7848bb..6cc2d0e4 100644 --- a/parser_recover_c.go +++ b/parser_recover_c.go @@ -905,6 +905,7 @@ func (p *Parser) cRecoverResumeLookahead(ts TokenSource, source []byte, s *glrSt hasErrorRunLexState: true, } relexed := lx.NextWithErrorRuns(uint32(errLS)) + relexed.lexerErrorModeLexed = true if relexed.Symbol == tok.Symbol && relexed.StartByte == tok.StartByte && relexed.EndByte == tok.EndByte { return tok, false } @@ -1026,6 +1027,7 @@ func (p *Parser) cRecoverInternalErrorModeToken(ts TokenSource, stacks []glrStac hasErrorRunLexState: true, } tok := lx.NextWithErrorRuns(uint32(ls)) + tok.lexerErrorModeLexed = true // The shared token now carries the C error-mode identity; the election // can trust it directly. p.cRecoverSharedTokenErrorModeLexed = true @@ -1287,6 +1289,9 @@ type cRecoverState struct { // per-segment recoveries are tracked here; forks drop cRec and with it // these charges, exactly like C. extraRecoveries uint32 + // clearOrdinaryLeafErrors records a source-bearing stack prefix and direct + // internal-lexer provenance for the first token in this recovery region. + clearOrdinaryLeafErrors bool } var cRecoverStateCloneObserver func() @@ -1299,11 +1304,12 @@ func (r *cRecoverState) clone() *cRecoverState { observer() } cp := &cRecoverState{ - openErr: r.openErr, - group: r.group, - groupOrder: r.groupOrder, - extraRecoveries: r.extraRecoveries, - summary: r.summary, + openErr: r.openErr, + group: r.group, + groupOrder: r.groupOrder, + extraRecoveries: r.extraRecoveries, + clearOrdinaryLeafErrors: r.clearOrdinaryLeafErrors, + summary: r.summary, } return cp } @@ -3542,6 +3548,7 @@ func (p *Parser) cHandleError(stacks *[]glrStack, si int, source []byte, tok Tok } v := &versions[vi] entries := cStackEntriesTopFirst(v, gssScratch) + hasParsedPrefix := cRecoveryEntriesHaveParsedPrefix(entries, tok.StartByte) if debugRecoveryCycleChecks { for ei := range entries { if entries[ei].node != nil { @@ -3553,7 +3560,10 @@ func (p *Parser) cHandleError(stacks *[]glrStack, si int, source []byte, tok Tok if reason != ParseStopNone { return cRecHalted, false, reason } - v.cRec = &cRecoverState{summary: summary, group: group, groupOrder: uint32(vi)} + v.cRec = &cRecoverState{ + summary: summary, group: group, groupOrder: uint32(vi), + clearOrdinaryLeafErrors: cRecoveryRegionClearsOrdinaryLeafErrors(p, tok, hasParsedPrefix), + } v.cRecoverMissingGroup = nil } @@ -3617,6 +3627,45 @@ func (p *Parser) cHandleError(stacks *[]glrStack, si int, source []byte, tok Tok return outcome, needsRedispatch, ParseStopNone } +// cRecoveryRegionClearsOrdinaryLeafErrors reports whether C would keep +// ordinary visible leaves clean in this recovery region. The predicate uses +// token provenance, symbol metadata, and a source-bearing stack prefix. +func cRecoveryRegionClearsOrdinaryLeafErrors(p *Parser, tok Token, hasParsedPrefix bool) bool { + return p != nil && hasParsedPrefix && p.cSymbolVisible(tok.Symbol) && + p.isNamedSymbol(tok.Symbol) && !tok.lexerSkippedPrefix && + cRecoveryTokenCanClearOrdinaryLeafError(tok) +} + +// cRecoveryEntriesHaveParsedPrefix proves that recovery follows a clean, +// source-bearing stack node at or before the current token. The error +// discontinuity and the base state do not provide this proof. +func cRecoveryEntriesHaveParsedPrefix(entries []stackEntry, tokenStartByte uint32) bool { + for _, entry := range entries { + if entry.node == nil || entry.state == cErrorState || entry.kind == stackEntryKindPendingParent || + (entry.kind != stackEntryKindNode && entry.kind != stackEntryKindNoTreeNode && entry.kind != stackEntryKindCompactFullLeaf) || + stackEntryNodeSymbol(entry) == 0 || stackEntryNodeSymbol(entry) == errorSymbol || + stackEntryNodeParseState(entry) != entry.state || stackEntryNodeIsMissing(entry) || + stackEntryNodeHasError(entry) || stackEntryNodeDirty(entry) { + continue + } + startByte := stackEntryNodeStartByte(entry) + endByte := stackEntryNodeEndByte(entry) + if endByte > startByte && endByte <= tokenStartByte { + return true + } + } + return false +} + +// cRecoveryTokenCanClearOrdinaryLeafError requires positive internal-DFA +// provenance for each absorbed token. A region proof cannot authorize a +// later external, generated, error-mode, zero-width, or EOF token. +func cRecoveryTokenCanClearOrdinaryLeafError(tok Token) bool { + return tok.Symbol != 0 && tok.Symbol != errorSymbol && tok.EndByte > tok.StartByte && + tok.lexerInternalDFALexed && !tok.ExternalScannerToken && + !tok.lexerErrorModeLexed && !tok.Missing && !tok.NoLookahead +} + // --------------------------------------------------------------------------- // recover port // --------------------------------------------------------------------------- @@ -4329,9 +4378,14 @@ func (p *Parser) cAbsorbTokenIntoError(v *glrStack, tok Token, nodeCount *int, a if leafVisible { leaf = newLeafNodeInArena(arena, tok.Symbol, tok.Symbol == errorSymbol || p.isNamedSymbol(tok.Symbol), tok.StartByte, tok.EndByte, tok.StartPoint, tok.EndPoint) - // C marks the enclosing ERROR node as erroneous. Only a proven - // lexer-produced ERROR leaf omits the second has-error flag. - if tok.Symbol != errorSymbol || !tok.lexerErrorModeLexed { + // C marks the enclosing ERROR node as erroneous. Keep ordinary leaves + // clean when the region has direct internal-lexer provenance. + clearLeafError := v.cRec != nil && v.cRec.clearOrdinaryLeafErrors && + cRecoveryTokenCanClearOrdinaryLeafError(tok) + if !clearLeafError && tok.Symbol != errorSymbol { + leaf.setHasError(true) + } + if tok.Symbol == errorSymbol && !tok.lexerErrorModeLexed { leaf.setHasError(true) } // C: if the token shifts as extra in state 1, mark it extra so it is diff --git a/parser_recover_c_leaf_policy_test.go b/parser_recover_c_leaf_policy_test.go new file mode 100644 index 00000000..d605df83 --- /dev/null +++ b/parser_recover_c_leaf_policy_test.go @@ -0,0 +1,242 @@ +package gotreesitter + +import "testing" + +func recoveryLeafPolicyFixture(t *testing.T) (*Parser, Token) { + t.Helper() + language := &Language{ + SymbolMetadata: []SymbolMetadata{ + {}, + {Name: "named_visible", Visible: true, Named: true}, + {Name: "anonymous_visible", Visible: true}, + {Name: "named_hidden", Named: true}, + }, + } + lexer := NewLexer([]LexState{ + {Default: -1, EOF: -1, Transitions: []LexTransition{{Lo: 'x', Hi: 'x', NextState: 1}}}, + {AcceptToken: 1, Default: -1, EOF: -1}, + }, []byte("x")) + tok := lexer.Next(0) + if !tok.lexerInternalDFALexed { + t.Fatal("fixture token lacks positive internal-DFA provenance") + } + return &Parser{language: language}, tok +} + +func TestCRecoveryRegionClearsOrdinaryLeafErrors(t *testing.T) { + parser, direct := recoveryLeafPolicyFixture(t) + tests := []struct { + name string + tok Token + want bool + }{ + {name: "direct named internal-DFA token", tok: direct, want: true}, + {name: "unproven token", tok: Token{Symbol: 1, StartByte: 0, EndByte: 1}}, + {name: "end-of-input symbol", tok: Token{Symbol: 0, EndByte: 1, lexerInternalDFALexed: true}}, + {name: "synthetic zero-width token", tok: Token{Symbol: 1, lexerInternalDFALexed: true}}, + {name: "generated token", tok: Token{Symbol: 1, StartByte: 0, EndByte: 1}}, + {name: "anonymous internal-DFA token", tok: func() Token { tok := direct; tok.Symbol = 2; return tok }()}, + {name: "invisible named internal-DFA token", tok: func() Token { tok := direct; tok.Symbol = 3; return tok }()}, + {name: "skipped-prefix first token", tok: func() Token { tok := direct; tok.lexerSkippedPrefix = true; return tok }()}, + {name: "external scanner token", tok: func() Token { tok := direct; tok.ExternalScannerToken = true; return tok }()}, + {name: "lexer error-mode token", tok: func() Token { tok := direct; tok.lexerErrorModeLexed = true; return tok }()}, + {name: "missing token", tok: func() Token { tok := direct; tok.Missing = true; return tok }()}, + {name: "no-lookahead token", tok: func() Token { tok := direct; tok.NoLookahead = true; return tok }()}, + {name: "error token", tok: Token{Symbol: errorSymbol, StartByte: 0, EndByte: 1, lexerInternalDFALexed: true}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := cRecoveryRegionClearsOrdinaryLeafErrors(parser, test.tok, true); got != test.want { + t.Fatalf("predicate = %t, want %t", got, test.want) + } + }) + } +} + +func TestCRecoveryRegionRequiresParsedPrefix(t *testing.T) { + parser, direct := recoveryLeafPolicyFixture(t) + if cRecoveryRegionClearsOrdinaryLeafErrors(parser, direct, false) { + t.Fatal("predicate accepted a recovery region without a parsed prefix") + } +} + +func TestCRecoveryEntriesHaveParsedPrefix(t *testing.T) { + arena := acquireNodeArena(arenaClassFull) + defer arena.Release() + zeroWidth := newLeafNodeInArena(arena, 1, true, 2, 2, Point{}, Point{}) + sourceBearing := newLeafNodeInArena(arena, 1, true, 2, 3, Point{}, Point{}) + future := newLeafNodeInArena(arena, 1, true, 3, 5, Point{}, Point{}) + outOfOrder := newLeafNodeInArena(arena, 1, true, 4, 3, Point{}, Point{}) + missing := newLeafNodeInArena(arena, 1, true, 1, 2, Point{}, Point{}) + missing.setMissing(true) + hasError := newLeafNodeInArena(arena, 1, true, 1, 2, Point{}, Point{}) + hasError.setHasError(true) + dirty := newLeafNodeInArena(arena, 1, true, 1, 2, Point{}, Point{}) + dirty.setDirty(true) + errorNode := newLeafNodeInArena(arena, errorSymbol, true, 1, 2, Point{}, Point{}) + pending := newPendingParentInArena(arena, 1, true, 0, nil, 1, 2, Point{}, Point{}, false) + invalid := newStackEntryNode(2, sourceBearing) + invalid.kind = 99 + for _, node := range []*Node{zeroWidth, sourceBearing, future, outOfOrder, missing, hasError, dirty, errorNode} { + node.parseState = 2 + } + stateMismatch := newStackEntryNode(3, sourceBearing) + + if cRecoveryEntriesHaveParsedPrefix([]stackEntry{{state: cErrorState}, {state: 1}}, 3) { + t.Fatal("error discontinuity and base state supplied a parsed prefix") + } + if cRecoveryEntriesHaveParsedPrefix([]stackEntry{newStackEntryNode(2, zeroWidth)}, 3) { + t.Fatal("zero-width generated node supplied a parsed prefix") + } + for name, entry := range map[string]stackEntry{ + "future": newStackEntryNode(2, future), + "out-of-order": newStackEntryNode(2, outOfOrder), + "missing": newStackEntryNode(2, missing), + "has-error": newStackEntryNode(2, hasError), + "dirty": newStackEntryNode(2, dirty), + "error-symbol": newStackEntryNode(2, errorNode), + "pending": newStackEntryPendingParent(2, pending), + "invalid": invalid, + "state-mismatch": stateMismatch, + } { + t.Run(name, func(t *testing.T) { + if cRecoveryEntriesHaveParsedPrefix([]stackEntry{entry}, 3) { + t.Fatal("invalid payload supplied a parsed prefix") + } + }) + } + if !cRecoveryEntriesHaveParsedPrefix([]stackEntry{newStackEntryNode(2, sourceBearing)}, 3) { + t.Fatal("source-bearing stack node did not supply a parsed prefix") + } +} + +type recoveryLeafPolicyTokenSource struct{} + +func (*recoveryLeafPolicyTokenSource) Next() Token { return Token{} } +func (*recoveryLeafPolicyTokenSource) SkipToByte(uint32) Token { return Token{} } + +func recoveryLeafErrorModeFixture() (*Parser, []byte) { + language := &Language{ + SymbolMetadata: []SymbolMetadata{{}, {Name: "visible", Visible: true, Named: true}, {Name: "shared", Visible: true, Named: true}}, + LexModes: []LexMode{{LexState: 0}, {LexState: 2}}, + LexStates: []LexState{ + {Default: -1, EOF: -1, Transitions: []LexTransition{{Lo: 'x', Hi: 'x', NextState: 1}}}, + {AcceptToken: 1, Default: -1, EOF: -1}, + {Default: -1, EOF: -1}, + }, + } + return &Parser{language: language}, []byte("x") +} + +func TestCRecoveryManualResumeMarksVisibleErrorModeToken(t *testing.T) { + parser, source := recoveryLeafErrorModeFixture() + parser.cRecoverCustomSourceEligible = true + stack := newGLRStack(1) + tok, replaced := parser.cRecoverResumeLookahead(&recoveryLeafPolicyTokenSource{}, source, &stack, Token{Symbol: 2, EndByte: 1}, nil) + if !replaced || !parser.cSymbolVisible(tok.Symbol) || !tok.lexerErrorModeLexed { + t.Fatalf("manual resume token = %+v, replaced = %t", tok, replaced) + } + if cRecoveryTokenCanClearOrdinaryLeafError(tok) { + t.Fatal("manual error-mode token can clear an ordinary leaf error") + } +} + +func TestCRecoveryInternalErrorModeMarksVisibleToken(t *testing.T) { + parser, source := recoveryLeafErrorModeFixture() + stack := newGLRStack(1) + stack.pushEntry(stackEntry{state: cErrorState}, nil, nil) + stack.cRec = &cRecoverState{group: &cRecGroup{}} + tok, ok := parser.cRecoverInternalErrorModeToken(&recoveryLeafPolicyTokenSource{}, []glrStack{stack}, source) + if !ok || !parser.cSymbolVisible(tok.Symbol) || !tok.lexerErrorModeLexed { + t.Fatalf("internal error-mode token = %+v, ok = %t", tok, ok) + } + if cRecoveryTokenCanClearOrdinaryLeafError(tok) { + t.Fatal("internal error-mode token can clear an ordinary leaf error") + } +} + +func TestCRecoveryTokenCanClearOrdinaryLeafError(t *testing.T) { + _, direct := recoveryLeafPolicyFixture(t) + tests := []struct { + name string + tok Token + want bool + }{ + {name: "later direct internal-DFA token", tok: direct, want: true}, + {name: "later skipped-prefix internal-DFA token", tok: func() Token { tok := direct; tok.lexerSkippedPrefix = true; return tok }(), want: true}, + {name: "later external scanner token", tok: func() Token { tok := direct; tok.ExternalScannerToken = true; return tok }()}, + {name: "later missing token", tok: func() Token { tok := direct; tok.Missing = true; return tok }()}, + {name: "later no-lookahead token", tok: func() Token { tok := direct; tok.NoLookahead = true; return tok }()}, + {name: "later lexer error-mode token", tok: func() Token { tok := direct; tok.lexerErrorModeLexed = true; return tok }()}, + {name: "later error token", tok: Token{Symbol: errorSymbol, StartByte: 0, EndByte: 1, lexerInternalDFALexed: true}}, + {name: "later end-of-input token", tok: Token{Symbol: 0, StartByte: 1, EndByte: 1, lexerInternalDFALexed: true}}, + {name: "later synthetic zero-width token", tok: Token{Symbol: 1, lexerInternalDFALexed: true}}, + {name: "later generated token", tok: Token{Symbol: 1, StartByte: 0, EndByte: 1}}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := cRecoveryTokenCanClearOrdinaryLeafError(test.tok); got != test.want { + t.Fatalf("predicate = %t, want %t", got, test.want) + } + }) + } +} + +func TestCAbsorbOrdinaryLeafChecksEveryTokenProvenance(t *testing.T) { + parser, direct := recoveryLeafPolicyFixture(t) + tests := []struct { + name string + tok Token + regionProof bool + wantChildError bool + }{ + {name: "no region proof", tok: direct, wantChildError: true}, + {name: "direct internal-DFA token", tok: direct, regionProof: true}, + {name: "skipped-prefix internal-DFA token", tok: func() Token { tok := direct; tok.lexerSkippedPrefix = true; return tok }(), regionProof: true}, + {name: "external scanner token", tok: func() Token { tok := direct; tok.ExternalScannerToken = true; return tok }(), regionProof: true, wantChildError: true}, + {name: "missing token", tok: func() Token { tok := direct; tok.Missing = true; return tok }(), regionProof: true, wantChildError: true}, + {name: "no-lookahead token", tok: func() Token { tok := direct; tok.NoLookahead = true; return tok }(), regionProof: true, wantChildError: true}, + {name: "lexer error-mode token", tok: func() Token { tok := direct; tok.lexerErrorModeLexed = true; return tok }(), regionProof: true, wantChildError: true}, + {name: "error-symbol token", tok: Token{Symbol: errorSymbol, EndByte: 1, lexerInternalDFALexed: true}, regionProof: true, wantChildError: true}, + {name: "synthetic zero-width token", tok: Token{Symbol: 1, lexerInternalDFALexed: true}, regionProof: true, wantChildError: true}, + {name: "generated token", tok: Token{Symbol: 1, EndByte: 1}, regionProof: true, wantChildError: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + arena := acquireNodeArena(arenaClassFull) + defer arena.Release() + openErr := newParentNodeInArena(arena, errorSymbol, true, nil, nil, 0) + openErr.setHasError(true) + stack := newGLRStack(1) + stack.pushEntry(newStackEntryNode(cErrorState, openErr), nil, nil) + stack.cRec = &cRecoverState{ + group: &cRecGroup{}, + openErr: openErr, + clearOrdinaryLeafErrors: test.regionProof, + } + parser.cAbsorbTokenIntoError(&stack, test.tok, nil, arena, nil, nil, nil) + if got, want := openErr.ChildCount(), 1; got != want { + t.Fatalf("child count = %d, want %d", got, want) + } + if got := openErr.Child(0).HasError(); got != test.wantChildError { + t.Fatalf("child HasError = %t, want %t", got, test.wantChildError) + } + }) + } +} + +func TestCRecoveryRegionClearsOrdinaryLeafErrorsNilParser(t *testing.T) { + if cRecoveryRegionClearsOrdinaryLeafErrors(nil, Token{Symbol: 1, StartByte: 0, EndByte: 1, lexerInternalDFALexed: true}, true) { + t.Fatal("predicate accepted a nil parser") + } +} + +func TestCRecoverStateClonePreservesLeafPolicy(t *testing.T) { + clone := (&cRecoverState{clearOrdinaryLeafErrors: true}).clone() + if clone == nil || !clone.clearOrdinaryLeafErrors { + t.Fatal("recovery-state clone lost the leaf policy") + } +} From bea477e63b59986b2a65e83d26ec06bcd2d01ce3 Mon Sep 17 00:00:00 2001 From: Oscar Villavicencio <9220505+odvcencio@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:08:42 -0700 Subject: [PATCH 2/2] refactor(parser/recovery): Merge leaf policy into groupOrder field - Pack the recovery-leaf policy into the high bit of `groupOrder`. Drop the standalone boolean field to reduce memory layout complexity. - Add packing and accessor helpers to enforce masking, saturation, and nil safety. - Update the state clone method to copy the packed value directly. - Switch strategy-1 member sorting to use the unpacked value. The policy bit must not alter path ordering. - Saturate out-of-range orders to the mask limit and clear the policy flag. Invalid input fails safely. - Add unit tests for clone retention, sort stability, collision handling, and struct size budget. Buckley-Change-Hash: sha256:a7757c3d1f6835c584bfbf4c51a2a098b98b8a0db1dd5ea97ade4d315fd7ee2d Buckley-Change-Stats: files=2 insertions=92 deletions=23 binaries=0 --- parser_recover_c.go | 64 ++++++++++++++++++++-------- parser_recover_c_leaf_policy_test.go | 51 +++++++++++++++++++--- 2 files changed, 92 insertions(+), 23 deletions(-) diff --git a/parser_recover_c.go b/parser_recover_c.go index 6cc2d0e4..45a258d0 100644 --- a/parser_recover_c.go +++ b/parser_recover_c.go @@ -1272,9 +1272,8 @@ type cRecoverState struct { // state — the C "ERROR_STATE head with NULL subtree" shape, which costs an // extra ERROR_COST_PER_RECOVERY in ts_stack_error_cost. openErr *Node - // groupOrder preserves the path order inside the C merged error-state - // version. Later Go stack ordering can move members around, but C's - // strategy-1 summary scan still walks the merged paths in record order. + // groupOrder preserves the path order in its low bits. The reserved high + // bit stores the recovery-leaf policy. Read the order through groupOrderValue. groupOrder uint32 // extraRecoveries counts the additional error segments C opens while this // version keeps absorbing: an unlexable-run (ERROR-token) lookahead has no @@ -1289,9 +1288,36 @@ type cRecoverState struct { // per-segment recoveries are tracked here; forks drop cRec and with it // these charges, exactly like C. extraRecoveries uint32 - // clearOrdinaryLeafErrors records a source-bearing stack prefix and direct - // internal-lexer provenance for the first token in this recovery region. - clearOrdinaryLeafErrors bool +} + +const ( + cRecoverGroupOrderLeafClearBit uint32 = 1 << 31 + cRecoverGroupOrderValueMask = cRecoverGroupOrderLeafClearBit - 1 +) + +// cPackRecoverGroupOrder packs the path order and recovery-leaf policy. Current +// recovery ceilings keep vi below the reserved bit. The uint64 input checks a +// future larger value before narrowing. Invalid input saturates and clears policy. +func cPackRecoverGroupOrder(order uint64, clearOrdinaryLeafErrors bool) uint32 { + if order > uint64(cRecoverGroupOrderValueMask) { + return cRecoverGroupOrderValueMask + } + packed := uint32(order) + if clearOrdinaryLeafErrors { + return packed | cRecoverGroupOrderLeafClearBit + } + return packed +} + +func (r *cRecoverState) groupOrderValue() uint32 { + if r == nil { + return 0 + } + return r.groupOrder & cRecoverGroupOrderValueMask +} + +func (r *cRecoverState) clearsOrdinaryLeafErrors() bool { + return r != nil && r.groupOrder&cRecoverGroupOrderLeafClearBit != 0 } var cRecoverStateCloneObserver func() @@ -1304,12 +1330,11 @@ func (r *cRecoverState) clone() *cRecoverState { observer() } cp := &cRecoverState{ - openErr: r.openErr, - group: r.group, - groupOrder: r.groupOrder, - extraRecoveries: r.extraRecoveries, - clearOrdinaryLeafErrors: r.clearOrdinaryLeafErrors, - summary: r.summary, + openErr: r.openErr, + group: r.group, + groupOrder: r.groupOrder, + extraRecoveries: r.extraRecoveries, + summary: r.summary, } return cp } @@ -3560,9 +3585,14 @@ func (p *Parser) cHandleError(stacks *[]glrStack, si int, source []byte, tok Tok if reason != ParseStopNone { return cRecHalted, false, reason } + // Ordinary recovery caps versions near cRecoverMaxVersionCount. Pass vi + // before narrowing so the packer fails closed if a future path exceeds it. v.cRec = &cRecoverState{ - summary: summary, group: group, groupOrder: uint32(vi), - clearOrdinaryLeafErrors: cRecoveryRegionClearsOrdinaryLeafErrors(p, tok, hasParsedPrefix), + summary: summary, group: group, + groupOrder: cPackRecoverGroupOrder( + uint64(vi), + cRecoveryRegionClearsOrdinaryLeafErrors(p, tok, hasParsedPrefix), + ), } v.cRecoverMissingGroup = nil } @@ -4064,11 +4094,11 @@ func (p *Parser) cRecoverStrategy1Election(stacks *[]glrStack, group *cRecGroup, func cSortRecoverMembersByGroupOrder(stacks []glrStack, members []int) { for i := 1; i < len(members); i++ { cur := members[i] - curOrder := stacks[cur].cRec.groupOrder + curOrder := stacks[cur].cRec.groupOrderValue() j := i - 1 for ; j >= 0; j-- { prev := members[j] - if stacks[prev].cRec.groupOrder <= curOrder { + if stacks[prev].cRec.groupOrderValue() <= curOrder { break } members[j+1] = prev @@ -4380,7 +4410,7 @@ func (p *Parser) cAbsorbTokenIntoError(v *glrStack, tok Token, nodeCount *int, a tok.StartByte, tok.EndByte, tok.StartPoint, tok.EndPoint) // C marks the enclosing ERROR node as erroneous. Keep ordinary leaves // clean when the region has direct internal-lexer provenance. - clearLeafError := v.cRec != nil && v.cRec.clearOrdinaryLeafErrors && + clearLeafError := v.cRec.clearsOrdinaryLeafErrors() && cRecoveryTokenCanClearOrdinaryLeafError(tok) if !clearLeafError && tok.Symbol != errorSymbol { leaf.setHasError(true) diff --git a/parser_recover_c_leaf_policy_test.go b/parser_recover_c_leaf_policy_test.go index d605df83..2a26904c 100644 --- a/parser_recover_c_leaf_policy_test.go +++ b/parser_recover_c_leaf_policy_test.go @@ -1,6 +1,9 @@ package gotreesitter -import "testing" +import ( + "testing" + "unsafe" +) func recoveryLeafPolicyFixture(t *testing.T) (*Parser, Token) { t.Helper() @@ -213,9 +216,9 @@ func TestCAbsorbOrdinaryLeafChecksEveryTokenProvenance(t *testing.T) { stack := newGLRStack(1) stack.pushEntry(newStackEntryNode(cErrorState, openErr), nil, nil) stack.cRec = &cRecoverState{ - group: &cRecGroup{}, - openErr: openErr, - clearOrdinaryLeafErrors: test.regionProof, + group: &cRecGroup{}, + openErr: openErr, + groupOrder: cPackRecoverGroupOrder(0, test.regionProof), } parser.cAbsorbTokenIntoError(&stack, test.tok, nil, arena, nil, nil, nil) if got, want := openErr.ChildCount(), 1; got != want { @@ -235,8 +238,44 @@ func TestCRecoveryRegionClearsOrdinaryLeafErrorsNilParser(t *testing.T) { } func TestCRecoverStateClonePreservesLeafPolicy(t *testing.T) { - clone := (&cRecoverState{clearOrdinaryLeafErrors: true}).clone() - if clone == nil || !clone.clearOrdinaryLeafErrors { + original := &cRecoverState{groupOrder: cPackRecoverGroupOrder(2, true)} + clone := original.clone() + if clone == nil || !clone.clearsOrdinaryLeafErrors() { t.Fatal("recovery-state clone lost the leaf policy") } + if got, want := clone.groupOrderValue(), uint32(2); got != want { + t.Fatalf("clone group order = %d, want %d", got, want) + } +} + +func TestCRecoverGroupOrderPolicyDoesNotChangeSorting(t *testing.T) { + group := &cRecGroup{} + stacks := []glrStack{ + {cRec: &cRecoverState{group: group, groupOrder: cPackRecoverGroupOrder(2, false)}}, + {cRec: &cRecoverState{group: group, groupOrder: cPackRecoverGroupOrder(0, true)}}, + {cRec: &cRecoverState{group: group, groupOrder: cPackRecoverGroupOrder(1, false)}}, + } + members := []int{0, 1, 2} + cSortRecoverMembersByGroupOrder(stacks, members) + for i, want := range []int{1, 2, 0} { + if members[i] != want { + t.Fatalf("members[%d] = %d, want %d", i, members[i], want) + } + } +} + +func TestCRecoverGroupOrderPolicyFailsClosedOnCollision(t *testing.T) { + state := &cRecoverState{groupOrder: cPackRecoverGroupOrder(uint64(cRecoverGroupOrderLeafClearBit), true)} + if state.clearsOrdinaryLeafErrors() { + t.Fatal("colliding group order retained the leaf-clear policy") + } + if got := state.groupOrderValue(); got != cRecoverGroupOrderValueMask { + t.Fatalf("colliding group order = %d, want saturated %d", got, cRecoverGroupOrderValueMask) + } +} + +func TestCRecoverStateLeafPolicyKeepsSizeBudget(t *testing.T) { + if got := unsafe.Sizeof(cRecoverState{}); got != 48 { + t.Fatalf("cRecoverState size = %d, want 48", got) + } }