One scope table for a whole use-resolution, not one per level - #149
Merged
Merged
Conversation
1.4.0 broke `use <file>`: a file used through another file resolved to undef. Moving scopes out of the nodes and into a ScopeTable made a table per buildScopes() call, and resolveUseScopes recurses -- each `use` builds its used file's own root Scope. Every level therefore made its own table and only the outermost was kept, so the evaluation had no scope recorded for any node of a used file. While the scope lived in the node, the nested writes were visible to everyone by construction; in a per-level table they were thrown away. The recursion now threads one table through and the outermost root adopts it (openscad_cpp_parser#8's buildScopesInto). Nothing about the include cache changes. Caught by BelfrySCAD's suite, not this one, which is why a nested-use case now lives here too -- it fails if the table stops being shared. 1154 C++ tests, 648 parser, 31 binding, BelfrySCAD's 1296, BOSL2's 909. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
1.4.0 broke
use <file>: a file used through another file resolved toundef.Cause
Moving scopes out of the nodes and into a
ScopeTable(#148) made a table perbuildScopes()call — andresolveUseScopesrecurses, since eachusebuilds its used file's own rootScope. Every level therefore made its own table and only the outermost was kept, so the evaluation had no scope recorded for any node of a used file.While the scope lived in the node, the nested writes were visible to everyone by construction. In a per-level table they were thrown away.
Fix
The recursion threads one table through, and the outermost root adopts it (openscad_cpp_parser#8's
buildScopesInto). Nothing about the include cache changes — the 9× speedup is unaffected.How it was caught
BelfrySCAD's suite, not this one. All 1153 tests here passed. A nested-
usecase now lives here too, and it fails if the table stops being shared (verified by re-breaking it).Verified
🤖 Generated with Claude Code