From c683af0f311272749a776bf58f14e2ba14acbef6 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sun, 6 Sep 2026 10:45:14 -0700 Subject: [PATCH] One scope table for a whole use-resolution, not one per level 1.4.0 broke `use `: 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) --- external/openscad_cpp_parser | 2 +- pyproject.toml | 2 +- src/eval_use.cpp | 39 +++++++++++++++++++++++++++++------- tests/test_include_cache.cpp | 29 +++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 9 deletions(-) diff --git a/external/openscad_cpp_parser b/external/openscad_cpp_parser index e4a75fc..a62a7ea 160000 --- a/external/openscad_cpp_parser +++ b/external/openscad_cpp_parser @@ -1 +1 @@ -Subproject commit e4a75fc48c9ca7cecd27edad4939aa26f7492480 +Subproject commit a62a7ea8cd197949bb539584119ac8375e85c7a9 diff --git a/pyproject.toml b/pyproject.toml index 115970f..d75fd71 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "1.4.0" +version = "1.4.1" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/eval_use.cpp b/src/eval_use.cpp index 781def3..80e3146 100644 --- a/src/eval_use.cpp +++ b/src/eval_use.cpp @@ -17,8 +17,19 @@ ResolvedUseScopes resolveUseScopes(const std::vector& ownNodes, - const std::string& currentFile, const std::function& logFn) { +namespace { + +// The recursion. `table` is threaded through every level rather than each +// level building its own: `use ` gives each used file its own root +// Scope, but one evaluation reads all of them back, so every node's scope +// has to land in ONE table. A table per level meant the outer evaluation +// could not see anything a nested resolution recorded -- every node of a +// used file read back as having no scope, and `use` silently resolved to +// undef. BelfrySCAD's own suite caught that; this repo's did not. +ResolvedUseScopes resolveUseScopesInto(const std::vector& ownNodes, + const std::string& currentFile, + const std::function& logFn, + oscad::ScopeTable& table) { ResolvedUseScopes result; std::vector injected; @@ -56,7 +67,10 @@ ResolvedUseScopes resolveUseScopes(const std::vector& own result.usedFileAsts.push_back(std::move(lib.ast)); const std::vector>& libAst = result.usedFileAsts.back(); - ResolvedUseScopes nested = resolveUseScopes(libAst, lib.resolvedPath, logFn); + std::vector libBorrowed; + libBorrowed.reserve(libAst.size()); + for (const auto& n : libAst) libBorrowed.push_back(n.get()); + ResolvedUseScopes nested = resolveUseScopesInto(libBorrowed, lib.resolvedPath, logFn, table); std::vector libInjected; for (const oscad::ASTNode* n : nested.ownNodesFiltered) { @@ -92,12 +106,11 @@ ResolvedUseScopes resolveUseScopes(const std::vector& own std::vector mutableProcessed; mutableProcessed.reserve(result.processedNodes.size()); for (const oscad::ASTNode* n : result.processedNodes) mutableProcessed.push_back(const_cast(n)); - result.rootScope = oscad::buildScopes(mutableProcessed); + result.rootScope = oscad::buildScopesInto(mutableProcessed, table); { - // Re-anchoring writes scopes too, into the same table buildScopes() - // just filled -- which the root scope now owns. - oscad::ScopeTableScope recording(*const_cast(result.rootScope->table())); + // Re-anchoring writes scopes too, into the same shared table. + oscad::ScopeTableScope recording(table); for (const auto& [libInjected, libRootScope] : reanchor) { for (const oscad::ASTNode* n : libInjected) const_cast(n)->buildScope(*libRootScope); } @@ -106,4 +119,16 @@ ResolvedUseScopes resolveUseScopes(const std::vector& own return result; } +} // namespace + +ResolvedUseScopes resolveUseScopes(const std::vector& ownNodes, const std::string& currentFile, + const std::function& logFn) { + // One table for the whole resolution, handed to the root scope at the + // end so it lives exactly as long as the scopes it points into. + auto table = std::make_unique(); + ResolvedUseScopes result = resolveUseScopesInto(ownNodes, currentFile, logFn, *table); + if (result.rootScope) result.rootScope->adoptTable(std::move(table)); + return result; +} + } // namespace oscadeval diff --git a/tests/test_include_cache.cpp b/tests/test_include_cache.cpp index ccd5c8c..b8cc16c 100644 --- a/tests/test_include_cache.cpp +++ b/tests/test_include_cache.cpp @@ -130,3 +130,32 @@ TEST(IncludeCache, ADerivedContextKeepsTheScopeTable) { ev.evaluate(used.processedNodes, ctx, {}, /*generate=*/false); for (const std::string& m : logs) EXPECT_EQ(m.find("Recursion"), std::string::npos) << m; } + + +TEST(IncludeCache, NestedUseKeepsItsScopesInTheSharedTable) { + // `use ` gives each used file its own root Scope, and a used file + // may itself `use` another. All of them are read back by ONE + // evaluation, so every node's scope has to land in ONE ScopeTable. + // + // The version of this that built a table per recursion level threw the + // nested ones away: the outer evaluation then saw no scope at all for a + // used file's nodes, and combo() below resolved to undef instead of + // 107. Every C++ test here passed anyway -- BelfrySCAD's suite is what + // caught it -- so this is that case, kept where the code lives. + writeFile("inner.scad", "inner_val = 100;\nfunction get_inner() = inner_val;\n"); + writeFile("lib2.scad", + "use \n" + "lib2_val = 7;\n" + "function combo() = get_inner() + lib2_val;\n"); + const auto main2 = writeFile("main2.scad", + "use \n" + "echo(combo());\n" + "echo(is_undef(inner_val));\n"); + + const std::vector echoes = echoesOf(main2); + ASSERT_EQ(echoes.size(), 2u); + // combo() reaches through lib2's own nested use into inner.scad. + EXPECT_EQ(echoes[0], "ECHO: 107"); + // ...while inner.scad's own declarations stay invisible to main2. + EXPECT_EQ(echoes[1], "ECHO: true"); +}