Skip to content

Remove store parameter from gen_element#732

Merged
gfx merged 2 commits intomainfrom
claude/remove-store-option-ROr85
Mar 29, 2026
Merged

Remove store parameter from gen_element#732
gfx merged 2 commits intomainfrom
claude/remove-store-option-ROr85

Conversation

@gfx
Copy link
Copy Markdown
Member

@gfx gfx commented Mar 29, 2026

Summary

  • Remove store: bool parameter from gen_element in the Gale parser generator, always generating meaningful variable names (the old store=true behavior)
  • Generated code now uses descriptive names like k_explain, comma, database_name instead of throwaway tok / tok_2 names
  • Remove completed TODO item from package-gale/TODO.md

Background

store=false was a workaround for naming collisions in dedup_name. All store=false call sites either use fresh scoped name_counts (loop bodies) or don't contribute to struct field assignments, so the collision concern doesn't apply. Always using meaningful names is safe and improves generated code readability.

Test plan

  • All 98 Gale tests pass (wado test package-gale/src)
  • Golden fixtures regenerated and match
  • Full on-task-done passes (format, clippy, all Rust and Wado tests)

https://claude.ai/code/session_01DsE7sSd7BNvctRdNnYXLA3

claude added 2 commits March 29, 2026 22:54
The `store: bool` parameter was a workaround for naming collisions in
`dedup_name` — `store=false` generated throwaway `tok` names while
`store=true` generated meaningful names like `k_explain`, `comma`, etc.

Since all `store=false` call sites are for elements that either use fresh
scoped `name_counts` (loop bodies) or don't contribute to struct field
assignments, the collision concern doesn't apply. Always using meaningful
names is safe and improves generated code readability.

https://claude.ai/code/session_01DsE7sSd7BNvctRdNnYXLA3
Elements inside optional/backtrack if-blocks and group if/else branches
are scoped, so their dedup counters must not pollute the parent scope.
Use fresh name_counts for these scoped blocks to keep parent counters
in sync with gen_field_assignments.

https://claude.ai/code/session_01DsE7sSd7BNvctRdNnYXLA3
@gfx gfx enabled auto-merge March 29, 2026 23:19
@gfx gfx merged commit 5b8974c into main Mar 29, 2026
10 checks passed
@gfx gfx deleted the claude/remove-store-option-ROr85 branch March 29, 2026 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants