feat(core-engine): yacc's %union is a real class_start, with the allowlist entry that makes it safe (#2644) - #2737
Merged
Conversation
…wlist entry that makes it safe (#2644) `yacc.py` wired `class_start: None` on the reasoning that a grammar file has no object/type concept. True for OOP-style classes, but it misses that Bison's `%union` directive declares a real compound type -- the C union spanning every grammar rule's semantic value (`$$`/`$1`, which `args` already counts). That is the same "non-OOP language's struct/class equivalent" mapping the engine already makes for Fortran's `TYPE ... END TYPE`, COBOL's `PROGRAM-ID` and assembly's `struc` macros, and it is core grammar syntax rather than incidentally-embedded C: `internal_discriminator` already lists `union` among the `%`-directives used to identify a file as yacc in the first place. The rule alone would have made yacc worse. detector.py's named-class extractor consults a language's own `class_start` only for languages in `_CLASS_START_NAMED_EXTRACTION_LANGS`; everyone else falls through to a legacy generic regex (`class|struct|interface|trait|enum`), which reads every `struct foo` declaration in a grammar's embedded C action code as a class -- 17 on config.y, 9 on jailparse.y and 109 across all four real grammar files in the crucible corpus, where the honest answers are 1, 1 and 2. So yacc joins the allowlist in the same change; the two halves only make sense together. Verified by direct source cross-check, the same way abap/cobol/jcl/sqlite were: yacc is tree-sitter-blind, and ctags cannot corroborate classes either (`ctags --list-kinds-full=YACC` exposes exactly one kind, `l`, which is why `CTAGS_CLASS_KINDS["yacc"]` is empty on purpose). `%union` fires exactly once in each grammar that declares one (config.y:1, jailparse.y:45) and zero times in the two that do not (gnucobol's parser.y/scanner.l use `%define api.value.type`) -- 100% precision, and an honest zero where there is no union. Golden masters re-blessed. The diff is exactly two lines in each mode: `Class/Entity Declarations` 0 -> 1 on config.y and jailparse.y. Nothing else moves -- no mass, no risk exposure, no topological re-solve. Tests: the yacc extraction gauntlet's `CLASS_CASES` were empty (the rule was None) and are now populated, including the invalid cases that matter here -- a bare C `union`/`struct` in action code, a mid-line or commented-out occurrence, and `%unionize` against the `\b` guard. Two detector-level tests pin the pair together (one `%union` plus surrounding C structs -> exactly one class; a grammar with no `%union` -> none), plus ReDoS coverage for the new rule. Closes #2644. Part of #2669. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Conflict: both #2736 and this branch re-blessed the golden masters. Resolved to main's fixtures (#2736's bless of the jcl/yaml comment-stream rules); this branch's own two lines are regenerated on top in the next commit rather than text-merged, since a golden master is generated output, not source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Regenerated on top of #2736's fixtures rather than text-merging the conflict. The residual diff against main is exactly this branch's own two lines -- `Class/Entity Declarations` 0 -> 1 on yacc/freebsd/config.y and jailparse.y -- confirming the two changes are orthogonal: measured on the merged tree BEFORE regenerating, the only drift against #2736's fixtures was those same two lines. Both modes PASS after the update. The header churn (absolute corpus path, timestamp, scan duration, remote URL suffix) is machine-specific and sanitized away by tests/golden_diff.py's load_and_sanitize. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
27 tasks
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.
Closes #2644. Part of #2669.
Implements the fix shape from the issue, plus the blocker its own follow-up comment identified —
both halves in one PR, because either alone is a regression.
The gap
yacc.pywiredclass_start: None, documented as "a grammar file has no object/type concept".True for OOP-style classes, but Bison's
%uniondeclares a real compound type: the C unionspanning every rule's semantic value (
$$/$1, whichargsalready counts). Same mapping theengine already makes for Fortran's
TYPE … END TYPE, COBOL'sPROGRAM-ID, assembly'sstruc.And it is core grammar syntax, not incidentally-embedded C —
internal_discriminatoralreadylists
unionamong the%-directives used to identify a file as yacc.Optional group 1 for bison's rarer named-tag form (
%union name {); the common anonymous formresolves to
Anonymous_Class, the path assembly's own no-nameclass_starttakes. No group 2 —a union has no inheritance parent for
detector.py's group-2-is-parent convention to misread.Why the allowlist entry is the load-bearing half
detector.py's named-class extractor consults a language's ownclass_startonly forlanguages in
_CLASS_START_NAMED_EXTRACTION_LANGS. yacc was absent, so wiring the rule alonedrops it onto the legacy generic fallback (
class|struct|interface|trait|enum), which reads everystruct foodeclaration in a grammar's embedded C actions as a class:%unionyacc/freebsd/config.yyacc/freebsd/jailparse.ycobol/gnucobol_internals/parser.ycobol/gnucobol_internals/scanner.l109 phantom classes across the four real grammar files, against a true total of 2. The rule
without the allowlist entry would have been worse than the
Noneit replaced, which is why#2644 was marked blocked on a slicer-layer companion change — this is that change.
Verification
yacc is tree-sitter-blind, so
tree_sitter_accuracy_audit.pycannot check it (it is not inNODE_MAPS), and ctags cannot corroborate classes either:ctags --list-kinds-full=YACCexposes exactly one kind,
l(label), which is whyCTAGS_CLASS_KINDS["yacc"]is empty onpurpose and the §9 tri-comparison covers functions only. Verified instead by direct source
cross-check against all four real grammar files — the same standing abap, cobol, jcl and sqlite
have on this allowlist. Result: 100% precision, no false positives, and an honest zero where a
grammar declares no union.
Class/Entity Declarations0 → 1 onconfig.yandjailparse.y. Nothing else moves — nostructural mass, no risk exposure, not even a topological coordinate.
pytest tests/— 7583 passed. The yacc gauntlet'sCLASS_CASESwere empty (the rule wasNone) and are now populated, with the invalid cases that matter here: a bare Cunion/structin action code, mid-line and commented-out occurrences, and
%unionizeagainst the\bguard.Two detector-level tests pin the two halves together (one
%unionamid C structs → exactly oneclass; a grammar with no
%union→ none), plus ReDoS coverage for the new quantifier.ruff_audit.py --ci,mypy_audit.py --ci,dead_key_audit.py --ci— clean against baseline.tri-comparison-auditdoes not cover yacc (baselines exist for typescript, javascript and zigonly), and
tree-sitter-accuracy-auditcannot see it at all.docs/language_status/yacc.md§1/§3/§4/§6/§7/§8 updated — including the stale key count, whichread 31/47 and is now 33/48 measured.
Corpus follow-up (keyword-rosetta)
The
yacc/class_startn/a cell becomes plantable: the rosetta corpus can now add a%unionprobeto
data/yacc/*.yand updateexpected_signals.json. Screened against yacc's full rules dict, theverbatim
config.y:1-6shape fires nothing butclass_start(no(,,,=,&,<,void,const,static, comment markers, or line-ending:). Not included here — that is a corpus PRagainst merged engine main, per the no-pins flow.
🤖 Generated with Claude Code