fix(ll1): walk forward through nullable symbols when computing FOLLOW sets - #316
Merged
Merged
Conversation
… sets FOLLOW-set computation added FIRST of only the immediate next symbol after a nonterminal occurrence, then jumped straight to checking whether the entire remainder was nullable - skipping every symbol in between. A nonterminal followed by one nullable symbol and then a non-nullable one lost the non-nullable symbol's FIRST set entirely, silently truncating FOLLOW and dropping parse-table entries for empty alternatives. Fixes #188
…docs Final review flagged that the plan document claimed all 13 pre-existing build_follow_sets tests would pass unchanged, but test_derive_empty's expected FOLLOW sets were correctly updated as part of the fix (the old values encoded the bug). Record that in the plan and design docs so the historical record is accurate.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an LL(1) FOLLOW-set under-computation bug where the algorithm only considered the immediate next symbol after a nonterminal occurrence, instead of walking forward through a nullable tail to the next non-nullable symbol (or falling back to FOLLOW(lhs) if the entire suffix is nullable). This prevents silently truncated FOLLOW sets that can drop parse-table entries for empty alternatives while still reporting is_ll1: true.
Changes:
- Update FOLLOW-set computation to iterate forward across consecutive nullable symbols, adding
FIRST(Xj) \ {ε}for each until a non-nullable symbol is reached. - Add/adjust unit tests to cover the nullable-tail regression (issue #188) and update expected FOLLOW sets impacted by the fix.
- Add supporting design/plan docs and update the roadmap/issue cross-links to reflect closure of #188.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/plcc/spec/syntax/validations/ll1/build_follow_sets.py | Correct FOLLOW-set forward-walk control flow across nullable suffix symbols. |
| src/plcc/spec/syntax/validations/ll1/build_follow_sets_test.py | Add regression test for nullable-tail walking; update expected FOLLOW sets affected by the fix. |
| dev-docs/specs/2026-08-12-follow-set-nullable-tail-design.md | Document the defect and the one-method control-flow fix strategy. |
| dev-docs/specs/2026-08-12-188-follow-set-nullable-tail-plan.md | Provide an implementation plan and test steps for the fix/issue closure workflow. |
| dev-docs/roadmap.md | Remove #188 from Open Issues. |
| dev-docs/issues/done/188-follow-set-omits-nullable-tail.md | Update relative links after closure (source link + related issue link). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Fixes the FOLLOW-set computation in
build_follow_sets.py: it added FIRST of only theimmediate next symbol after a nonterminal occurrence, then jumped straight to checking
whether the entire remainder was nullable — skipping every symbol in between. A
nonterminal followed by one nullable symbol and then a non-nullable one lost the
non-nullable symbol's FIRST set entirely, silently truncating FOLLOW and dropping
parse-table entries for empty alternatives (while
plcc-ll1still reportedis_ll1: true).Design: dev-docs/specs/2026-08-12-follow-set-nullable-tail-design.md
Plan: dev-docs/specs/2026-08-12-188-follow-set-nullable-tail-plan.md
Closes the issue tracked at dev-docs/issues/done/188-follow-set-omits-nullable-tail.md
Sign off on the DCO.
License their changes under the project's license.