Skip to content

Implement SplitNTermLevels transform with tests - #14

Open
AlanPietrasz wants to merge 20 commits into
fram-lang:masterfrom
AlanPietrasz:feature/8-split-nterm-levels
Open

Implement SplitNTermLevels transform with tests#14
AlanPietrasz wants to merge 20 commits into
fram-lang:masterfrom
AlanPietrasz:feature/8-split-nterm-levels

Conversation

@AlanPietrasz

Copy link
Copy Markdown

#8
Implemented SplitNTermLevels transform.
Added Level comparison/toString methods.
Added tests for splitting productions by level and forwarding tags/actions.

@ppolesiuk ppolesiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! The algorithm looks good. I've found one bug related to handling tag conditions and I have some other minor comments, shown below.

Comment thread src/Repr/Common/Level.fram
Comment thread src/Repr/Common/Level.fram
Comment thread src/Transform/SplitNTermLevels.fram Outdated
Comment thread src/Transform/SplitNTermLevels.fram Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Implements the SplitNTermLevels transform (issue #8), which splits each non-terminal into one non-terminal per production-level and inserts forwarding productions that carry tags from the level above. Also adds Level comparison/toString methods, a unit-test suite for the transform, and a minimal bash test runner / harness.

Changes:

  • Add equal, lt, le, and toString methods on Level.
  • Implement Transform/SplitNTermLevels.transform: collect distinct production levels per non-terminal, allocate fresh NTermIds per (NTerm, level), rewrite PS_NTerm symbols to point at the correct split, and emit forwarding productions between adjacent levels carrying the upper level's tags.
  • Add test/TransformTests/SplitNTermLevels.fram with four test cases, plus test/TestAll.fram, test/test_suite, and a test.sh bash harness.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/Repr/Common/Level.fram Adds equal/lt/le/toString methods on Level.
src/Transform/SplitNTermLevels.fram Full implementation of the level-splitting transform replacing the prior stub.
test/TransformTests/SplitNTermLevels.fram New unit tests covering NTerm count per level, retention of original productions, forwarding productions, and tag forwarding.
test/TestAll.fram Aggregates test modules; currently imports the new SplitNTermLevels tests.
test/test_suite Declares a framyard_tests suite that runs TestAll.fram with -L src -L test.
test.sh New bash test driver that locates dbl/DBL_LIB, sources a test_suite file, and reports pass/fail counts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Transform/SplitNTermLevels.fram
Comment thread src/Transform/SplitNTermLevels.fram Outdated
Comment thread src/Transform/SplitNTermLevels.fram Outdated
Comment thread src/Transform/SplitNTermLevels.fram Outdated
Comment thread src/Transform/SplitNTermLevels.fram Outdated
Comment thread src/Repr/Common/Level.fram
Comment thread src/Repr/Common/Level.fram
Comment thread test.sh
Comment thread test.sh Outdated
@AlanPietrasz
AlanPietrasz marked this pull request as draft June 15, 2026 16:12
@AlanPietrasz
AlanPietrasz marked this pull request as ready for review June 15, 2026 16:59
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.

3 participants