fix(parser): remove duplicate [T] array-sugar production (kills never-reduced + 26 R/R conflicts)#214
Merged
Merged
Conversation
`type_expr_primary` declared the array-shorthand production
`LBRACKET type_expr RBRACKET -> TyApp("Array", ...)` twice (lines
443/469) — added independently by issues-drafts/02 and #40 with
byte-identical semantic actions. The second copy was a verbatim
duplicate of the first.
Effects of the duplication, both confirmed via `menhir --explain`:
- Menhir reported "1 production is never reduced" (the second copy
is dead — the first always wins in the LALR automaton).
- It generated a real reduce/reduce conflict family: state 80
(reached after `[ type_expr ]`) could not choose which identical
production to reduce, and that ambiguity propagated through
inherited lookaheads.
Removing the dead copy and folding its substantive rationale (the
`Array`-not-`List` desugar reason + typecheck.ml canonicalisation
note + #40 ref) into the surviving comment so no documented knowledge
is lost.
Measured impact (full `dune test --force` gate green at 257/257 before
and after — zero parse-behaviour change, this is a pure dedup):
- "production never reduced" warning: eliminated.
- reduce/reduce conflicts: 36 -> 10 (R/R conflict states 5 -> 4).
- shift/reduce conflicts: unchanged at 75 (the duplicate was a
reduce/reduce source only).
The residual 75 S/R + 10 R/R conflicts are inherent LALR(1)
ambiguities that Menhir resolves deterministically and correctly (the
257-test suite, incl. STAGE-A AOT + multi-module + STAGE-B effect-row
coverage, proves the chosen resolution is the intended language).
This Menhir (20260209) has no expected-conflict declaration mechanism,
so eliminating the rest requires deliberate per-conflict grammar
refactoring with estate-wide parse blast radius — tracked separately,
not bundled here.
Refs #40
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hyperpolymath
added a commit
that referenced
this pull request
May 18, 2026
… precedence (#216) First PR of the dedicated grammar-conflict-elimination workstream (epic #215), family A (effect-row MINUS overload), owner-prioritised first. In `type_expr_arrow`, after a `type_expr_primary`, lookahead `ARROW` (continue an arrow type) or `MINUS` (open the legacy `-{E}->` effect row, parser.mly:410) raced against reducing the base case `type_expr_arrow -> type_expr_primary` (state 41, tokens MINUS ARROW). Shifting is *always* the correct choice (a bare type can never both be followed by `->`/`-{` and want to stop there), and was already Menhir's arbitrary resolution — so making it explicit changes no parse. Fix: a lowest-precedence pseudo-token `LOWEST_TYPE_ARROW` (never lexed) tags the base reduction; `ARROW` is given a precedence just above it. `MINUS` already outranks both, so on either lookahead the parser shifts *by declared precedence* instead of by an arbitrary tie-break, and the conflict disappears. `ARROW` is a conflict lookahead ONLY in state 41 (verified via `menhir --explain`), so giving it precedence is side-effect-free. Rigorous verification (full `menhir --explain` diff, baseline = #214): - S/R arbitrarily resolved: 75 -> 72 - S/R conflict states: 25 -> 23 (state 41 eliminated) - R/R: 10 (untouched); no new conflict states introduced - The 16 pre-existing "precedence/`%prec` never useful" warnings are byte-for-byte identical before and after — NOT introduced here (they are dead precedence declarations; filed as family F on #215). - `dune test --force` green at 257/257 (parse-behaviour-preserving). Refs #215 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
🔍 Hypatia Security ScanFindings: 44 issues detected
View findings[
{
"reason": "Stray AI.a2ml in root -- use 0-AI-MANIFEST.a2ml only",
"type": "banned",
"file": "AI.a2ml",
"action": "delete",
"rule_module": "root_hygiene",
"severity": "high"
},
{
"reason": "Superseded by 0-AI-MANIFEST.a2ml",
"type": "banned",
"file": "AI.djot",
"action": "delete",
"rule_module": "root_hygiene",
"severity": "high"
},
{
"reason": "Issue in quality.yml",
"type": "missing_workflow",
"file": "quality.yml",
"action": "create",
"rule_module": "workflow_audit",
"severity": "high"
},
{
"reason": "Issue in security-policy.yml",
"type": "missing_workflow",
"file": "security-policy.yml",
"action": "create",
"rule_module": "workflow_audit",
"severity": "medium"
},
{
"reason": "Action hyperpolymath/standards/.github/workflows/governance-reusable.yml@main needs attention",
"type": "unpinned_action",
"file": "governance.yml",
"action": "pin_sha",
"rule_module": "workflow_audit",
"severity": "high"
},
{
"reason": "TypeScript file detected -- banned language",
"type": "banned_language_file",
"file": "/home/runner/work/affinescript/affinescript/affinescript-deno-test/example/smoke_driver.ts",
"action": "flag",
"rule_module": "cicd_rules",
"severity": "critical"
},
{
"reason": "TypeScript file detected -- banned language",
"type": "banned_language_file",
"file": "/home/runner/work/affinescript/affinescript/affinescript-deno-test/cli.ts",
"action": "flag",
"rule_module": "cicd_rules",
"severity": "critical"
},
{
"reason": "TypeScript file detected -- banned language",
"type": "banned_language_file",
"file": "/home/runner/work/affinescript/affinescript/affinescript-deno-test/mod.ts",
"action": "flag",
"rule_module": "cicd_rules",
"severity": "critical"
},
{
"reason": "TypeScript file detected -- banned language",
"type": "banned_language_file",
"file": "/home/runner/work/affinescript/affinescript/affinescript-deno-test/lib/compile.ts",
"action": "flag",
"rule_module": "cicd_rules",
"severity": "critical"
},
{
"reason": "TypeScript file detected -- banned language",
"type": "banned_language_file",
"file": "/home/runner/work/affinescript/affinescript/affinescript-deno-test/lib/runner.ts",
"action": "flag",
"rule_module": "cicd_rules",
"severity": "critical"
}
]Powered by Hypatia Neurosymbolic CI/CD Intelligence |
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.
fix(parser): remove duplicate [T] array-sugar production
type_expr_primarydeclared the array-shorthand productionLBRACKET type_expr RBRACKET -> TyApp("Array", ...)twice (lines443/469) — added independently by issues-drafts/02 and #40 with
byte-identical semantic actions. The second copy was a verbatim
duplicate of the first.
Effects of the duplication, both confirmed via
menhir --explain:is dead — the first always wins in the LALR automaton).
(reached after
[ type_expr ]) could not choose which identicalproduction to reduce, and that ambiguity propagated through
inherited lookaheads.
Removing the dead copy and folding its substantive rationale (the
Array-not-Listdesugar reason + typecheck.ml canonicalisationnote + #40 ref) into the surviving comment so no documented knowledge
is lost.
Measured impact (full
dune test --forcegate green at 257/257 beforeand after — zero parse-behaviour change, this is a pure dedup):
reduce/reduce source only).
The residual 75 S/R + 10 R/R conflicts are inherent LALR(1)
ambiguities that Menhir resolves deterministically and correctly (the
257-test suite, incl. STAGE-A AOT + multi-module + STAGE-B effect-row
coverage, proves the chosen resolution is the intended language).
This Menhir (20260209) has no expected-conflict declaration mechanism,
so eliminating the rest requires deliberate per-conflict grammar
refactoring with estate-wide parse blast radius — tracked separately,
not bundled here.
Refs #40
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com