-
Notifications
You must be signed in to change notification settings - Fork 0
Fix diamond includes: allow same file in multiple branches #726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
53d5a52
test: diamond includes must reach a shared file twice without error
claude b1e90b6
fix: make diamond includes work; include a file once per visible scope
claude 6545314
test: cover review findings on include-once bookkeeping
claude 675b45d
fix: refine include-once bookkeeping from review findings
claude File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| # 2026-09-04 — Diamond includes: the second branch always broke | ||
|
|
||
| ## Symptom | ||
|
|
||
| A multi-file program where two library files both `include from` the same | ||
| shared file could not run: | ||
|
|
||
| ```text | ||
| util.wfl | ||
| / \ | ||
| auth.wfl render.wfl | ||
| \ / | ||
| main.wfl | ||
| ``` | ||
|
|
||
| ```text | ||
| error[ERROR]: Semantic error in included file 'render.wfl': | ||
| Semantic error at line 3, column 21: 'shout' is not a function | ||
| ``` | ||
|
|
||
| The first branch (`auth.wfl`) worked; the second (`render.wfl`) failed while | ||
| being *analyzed*, before a single line of it ran. Verified on wfl 26.9.2. | ||
|
|
||
| A downstream project had noticed this and concluded that "WFL includes form a | ||
| tree, and diamonds break", working around it by chaining every file into one | ||
| long line (`util <- db <- auth <- render <- site_ext <- main`). The chain works | ||
| only by accident: with every `include from` at the top of its file, nothing | ||
| is defined yet when each file is analyzed, so unknown names are downgraded to | ||
| warnings and resolve at runtime. | ||
|
|
||
| ## Root cause | ||
|
|
||
| Two separate defects, one hiding the other. | ||
|
|
||
| 1. **Parent-scope actions were seeded into an included file's analyzer as | ||
| plain variables.** `extract_parent_variables` turned every runtime binding | ||
| into `SymbolKind::Variable`, actions included. Calling one of those | ||
| (`shout of title`) then hit the analyzer's "'shout' is not a function" | ||
| error, which is fatal for included files. This broke any second file that | ||
| used an action an earlier include had defined — the diamond, but also the | ||
| plain sibling order where `render.wfl` does not include `util.wfl` itself. | ||
|
|
||
| 2. **Re-including a file re-ran it into the same scope.** Once (1) is fixed, | ||
| the second arrival at `util.wfl` executes `store util_loads as 1` and | ||
| `define action called shout` again in a scope that already has both: | ||
| "Variable 'shout' has already been defined at line 0". The cycle check | ||
| only knew about files *currently* loading, not files already finished. | ||
|
|
||
| ## Fix | ||
|
|
||
| - The interpreter now snapshots the enclosing scope as typed variables | ||
| **and** action signatures (`snapshot_parent_scope`), and the analyzer gets | ||
| the actions through `register_parent_actions` as real function symbols | ||
| with their true parameter lists. A same-name definition in the analyzed | ||
| file is treated as an overload under the existing distinctness rules, | ||
| which matches what the runtime already did. | ||
| - `Environment` records the canonical paths `include from` has completed in | ||
| that scope **and that installed at least one definition there**. An | ||
| include whose file is already recorded for the current scope (or an | ||
| ancestor it can see) is a no-op, decided before the import-depth ceiling | ||
| is charged. Recycled loop scopes clear the record along with their | ||
| values. A file that defines nothing is never recorded, so a side-effect- | ||
| only file keeps running on every include — no existing program changes | ||
| behavior (anything with a definition already failed). A failed include is | ||
| not recorded either; what it defined before failing stays in the scope, | ||
| as it always did. | ||
| - `load module` still runs in an isolated child scope. Its analyzer now | ||
| sees outer actions as callable functions (so calls resolve) but rejects a | ||
| same-name definition up front — the runtime would reject it anyway, and | ||
| the rejection must land before the module's earlier statements run. | ||
|
|
||
| ## Evidence | ||
|
|
||
| - Red: `tests/include_diamond_test.rs` (9 tests, four added from review | ||
| findings: loop-scope recycling, side-effect-only re-include, the | ||
| import-depth boundary, and `load module` outer-action rejection) and | ||
| `TestPrograms/modules/include_diamond.wfl` fail on the unmodified | ||
| interpreter with the errors quoted above. | ||
| - Green: same tests pass after the change; `cargo test --workspace`, | ||
| `cargo clippy --all-targets --all-features -- -D warnings`, and the | ||
| gated `TestPrograms/` run are clean. | ||
| - Risk class R3 (backward compatibility). Negative paths covered: a genuine | ||
| include cycle is still rejected; an include inside an action body still | ||
| runs per call when nothing enclosing has included the file. | ||
|
|
||
| ## Residual | ||
|
|
||
| The type checker does not follow includes, so a container defined in a | ||
| shared file still produces non-fatal "Container type 'X' not found" warnings | ||
| in a sibling file that instantiates it. That predates this change and the | ||
| program runs correctly; it is a separate issue. |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // One branch of the diamond: depends on util.wfl directly. | ||
| include from "util.wfl" | ||
| define action called auth_check with parameters who: | ||
| give back shout of who | ||
| end action |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // Top of the diamond: both branches bring in util.wfl; it is included once. | ||
| include from "auth.wfl" | ||
| include from "render.wfl" | ||
| display auth_check of "alice" # alice! | ||
| display render_page of "home" # home! |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // The other branch of the diamond: also depends on util.wfl directly. | ||
| include from "util.wfl" | ||
| define action called render_page with parameters title: | ||
| give back shout of title | ||
| end action |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // Shared leaf of the include diamond documented in | ||
| // Docs/04-advanced-features/modules.md ("Including the same file more than once"). | ||
| define action called shout with parameters msg: | ||
| give back msg with "!" | ||
| end action |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // Diamond include: `auth.wfl` and `render.wfl` both `include from "util.wfl"`. | ||
| // The second arrival at `util.wfl` must be a no-op (its definitions are | ||
| // already in this scope), not an "already defined" failure, and both | ||
| // branches must be able to call the shared action. | ||
| include from "../../tests/fixtures/modules/diamond/auth.wfl" | ||
| include from "../../tests/fixtures/modules/diamond/render.wfl" | ||
|
|
||
| describe "diamond include": | ||
| test "both branches see the shared action": | ||
| expect auth_check of "alice" to equal "alice!" | ||
| expect render_page of "home" to equal "home!" | ||
| end test | ||
| test "the shared file ran exactly once": | ||
| expect util_loads to equal 1 | ||
| end test | ||
| end describe |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.