diff --git a/bindings/module.cpp b/bindings/module.cpp index 06bd71e..6839b77 100644 --- a/bindings/module.cpp +++ b/bindings/module.cpp @@ -388,8 +388,8 @@ nb::object evaluate(const std::string& path, nb::dict viewportParams, nb::gil_scoped_release rel; auto logFn = [&echoes](const std::string& m) { echoes.push_back(m); }; try { - std::vector> ast = oscad::getASTFromFile(path); - oscadeval::ResolvedUseScopes used = oscadeval::resolveUseScopes(ast, path, logFn); + oscad::ParsedProgram program = oscad::getProgramFromFile(path); + oscadeval::ResolvedUseScopes used = oscadeval::resolveUseScopes(program.nodes, path, logFn); oscadeval::Evaluator ev(logFn, nullptr, manifoldCache, oscadeval::DebugHooks{}, profile); oscadeval::EvalContext ctx = oscadeval::EvalContext::makeRoot(used.rootScope.get()); bodies = oscadeval::toRenderableBodies(ev.evaluate(used.processedNodes, ctx, vp, generate)); @@ -693,8 +693,8 @@ nb::object debugEvaluate(const std::string& path, nb::dict viewportParams, nb::c }; } try { - std::vector> ast = oscad::getASTFromFile(path); - oscadeval::ResolvedUseScopes used = oscadeval::resolveUseScopes(ast, path, echoCpp); + oscad::ParsedProgram program = oscad::getProgramFromFile(path); + oscadeval::ResolvedUseScopes used = oscadeval::resolveUseScopes(program.nodes, path, echoCpp); oscadeval::Evaluator ev(echoCpp, nullptr, manifoldCache, hooks, false); evPtr = &ev; if (fastContinueSignal) ev.setFastContinueInterruptFlag(fastContinueSignal->flag()); @@ -729,8 +729,8 @@ nb::list parseDecls(const std::string& path) { std::vector decls; { nb::gil_scoped_release rel; - std::vector> ast = oscad::getASTFromFile(path); - oscadeval::ResolvedUseScopes used = oscadeval::resolveUseScopes(ast, path, [](const std::string&) {}); + oscad::ParsedProgram program = oscad::getProgramFromFile(path); + oscadeval::ResolvedUseScopes used = oscadeval::resolveUseScopes(program.nodes, path, [](const std::string&) {}); for (const oscad::ASTNode* n : used.processedNodes) { const oscad::Position& p = n->position(); const char* ns = nullptr; diff --git a/external/openscad_cpp_parser b/external/openscad_cpp_parser index ade1d62..e4a75fc 160000 --- a/external/openscad_cpp_parser +++ b/external/openscad_cpp_parser @@ -1 +1 @@ -Subproject commit ade1d62073d3f47544bc562790a94310b549b7ab +Subproject commit e4a75fc48c9ca7cecd27edad4939aa26f7492480 diff --git a/include/openscad_cpp_evaluator/bytecode_compiler.hpp b/include/openscad_cpp_evaluator/bytecode_compiler.hpp index fd805fa..89ce9e0 100644 --- a/include/openscad_cpp_evaluator/bytecode_compiler.hpp +++ b/include/openscad_cpp_evaluator/bytecode_compiler.hpp @@ -20,7 +20,8 @@ namespace oscadeval { // yet -- the caller falls back to the ordinary AST interpreter for that // whole function, unconditionally correct since nothing about this // function's behavior changes based on whether it happened to compile. -std::optional tryCompileFunction(const oscad::FunctionDeclaration& decl); +std::optional tryCompileFunction(const oscad::FunctionDeclaration& decl, + const oscad::ScopeTable* scopeTable); // Compiles a bare STATEMENT-context expression -- an assignment's RHS, an // if/for condition, a module-call or echo()/assert() argument -- rather @@ -48,7 +49,8 @@ std::optional tryCompileFunction(const oscad::FunctionDeclaration // always set by buildScope() regardless of node kind) -- needed to resolve // any callee inside it statically, the same way a function body's call // sites are. -std::optional tryCompileStatementExpr(const oscad::Expression& expr, const oscad::Scope* scope); +std::optional tryCompileStatementExpr(const oscad::Expression& expr, const oscad::Scope* scope, + const oscad::ScopeTable* scopeTable); // Compiles a run of SIBLING assignment statements sharing one scope -- // exactly Evaluator::evalChildren's own `assignments` sub-list (stmt_eval. @@ -82,6 +84,7 @@ std::optional tryCompileStatementExpr(const oscad::Expression& ex // contained by, so it would leak into the caller's ctx.dyn permanently // instead of just for that one assignment's own RHS. std::optional tryCompileAssignmentBlock(const std::vector& assigns, + const oscad::ScopeTable* scopeTable, const oscad::Scope* scope); // Attempts to compile `decl`'s parameter defaults + STATEMENT-list body @@ -97,7 +100,8 @@ std::optional tryCompileAssignmentBlock(const std::vector tryCompileModuleBody(const oscad::ModuleDeclaration& decl); +std::optional tryCompileModuleBody(const oscad::ModuleDeclaration& decl, + const oscad::ScopeTable* scopeTable); // Same compilation (assignment/if/for/resolved-module-call get real // bytecode, everything else a native passthrough), but for an ARBITRARY @@ -121,6 +125,7 @@ std::optional tryCompileModuleBody(const oscad::ModuleDeclaration // resolving a call site's callee -- callers pass `children.front()-> // scope()`, mirroring tryRunCompiledAssignmentBlock's own convention. std::optional tryCompileChildrenList(const std::vector& children, + const oscad::ScopeTable* scopeTable, const oscad::Scope* scope); } // namespace oscadeval diff --git a/include/openscad_cpp_evaluator/eval_context.hpp b/include/openscad_cpp_evaluator/eval_context.hpp index 6528a8d..49e5063 100644 --- a/include/openscad_cpp_evaluator/eval_context.hpp +++ b/include/openscad_cpp_evaluator/eval_context.hpp @@ -6,6 +6,7 @@ #include "openscad_cpp_parser/ast/ast_node.hpp" #include "openscad_cpp_parser/position.hpp" #include "openscad_cpp_parser/scope.hpp" +#include "openscad_cpp_parser/scope_table.hpp" #include #include @@ -74,11 +75,25 @@ struct EvalContext { // one that sees it. bool viaChildren = false; + // Where every node's lexical Scope lives, since it cannot live in the + // node itself -- one parsed tree is shared by every script that + // includes it, and `include` puts those nodes in the INCLUDER's scope. + // Owned by the Evaluator for the length of a run; aliased, never + // copied, by every derived context. + const oscad::ScopeTable* scopeTable = nullptr; + + // This node's own lexical scope, or null if none was recorded. Reads + // like the old ASTNode::scope() it replaces. + const oscad::Scope* scopeOf(const oscad::ASTNode& node) const { + return scopeTable ? scopeTable->get(node) : nullptr; + } + // The one genuinely fresh construction: seeds `dyn` with OpenSCAD's // built-in $-variable defaults ($fn=0, $fa=12, $fs=2, $t=0, // $parent_modules=0). Every other EvalContext in a run is derived // from this one via the methods below. - static EvalContext makeRoot(const oscad::Scope* rootScope); + static EvalContext makeRoot(const oscad::Scope* rootScope, + const oscad::ScopeTable* scopeTable = nullptr); // Mirrors _eval_children's direct EvalContext(...) construction: swaps // only `scope` (to a sibling statement's own lexical scope from diff --git a/include/openscad_cpp_evaluator/eval_use.hpp b/include/openscad_cpp_evaluator/eval_use.hpp index 7737ddd..9eb46d5 100644 --- a/include/openscad_cpp_evaluator/eval_use.hpp +++ b/include/openscad_cpp_evaluator/eval_use.hpp @@ -29,6 +29,8 @@ struct ResolvedUseScopes { // these (not anything currentFile itself pulled in via `use`; "nested // use has no effect on the base file's environment"). std::vector ownNodesFiltered; + // Owns the ScopeTable holding every node's lexical scope -- see + // oscad::ScopeTable for why that cannot live in the nodes themselves. std::unique_ptr rootScope; }; @@ -48,4 +50,11 @@ struct ResolvedUseScopes { ResolvedUseScopes resolveUseScopes(const std::vector>& ownNodes, const std::string& currentFile, const std::function& logFn); +// Same, for a statement list that is BORROWED rather than owned -- what +// oscad::ParsedProgram hands back when includes come from the shared AST +// cache. The caller must keep that ParsedProgram alive alongside the +// result, exactly as it must keep an owned AST alive. +ResolvedUseScopes resolveUseScopes(const std::vector& ownNodes, const std::string& currentFile, + const std::function& logFn); + } // namespace oscadeval diff --git a/include/openscad_cpp_evaluator/evaluator.hpp b/include/openscad_cpp_evaluator/evaluator.hpp index 6c434b2..21cabf3 100644 --- a/include/openscad_cpp_evaluator/evaluator.hpp +++ b/include/openscad_cpp_evaluator/evaluator.hpp @@ -132,6 +132,16 @@ class Evaluator { // fallback. Value evalExprMaybeCompiled(const oscad::Expression& node, EvalContext& ctx); + // The run's ScopeTable, latched from the EvalContext handed to + // evaluate()/resolveTree(). A handful of chunk-compile helpers below + // need a node's scope but are reached from the VM without an + // EvalContext in hand; they read it here instead of every caller + // growing a parameter. Null outside a run, which reads the same as the + // unset ASTNode::scope() this replaced. + const oscad::Scope* scopeOfNode(const oscad::ASTNode& node) const { + return scopeTable_ ? scopeTable_->get(node) : nullptr; + } + // Evaluates a block's statements in OpenSCAD's assignment-before- // geometry order (all Assignment nodes first, then everything else, // each group preserving source order), each against its own lexical @@ -1191,6 +1201,7 @@ class Evaluator { // this exists. Not a reentrancy guard (resolveTreeImpl is never called // while another one is already active), just an on/off switch for // whether evalExprMaybeCompiled's cache is safe to touch right now. + const oscad::ScopeTable* scopeTable_ = nullptr; bool inResolvePass_ = false; // True only while evalRenderExpr (or the VM's Kind::Measure bracket) is diff --git a/include/openscad_cpp_evaluator/value.hpp b/include/openscad_cpp_evaluator/value.hpp index c5efcae..8668a3f 100644 --- a/include/openscad_cpp_evaluator/value.hpp +++ b/include/openscad_cpp_evaluator/value.hpp @@ -12,6 +12,7 @@ namespace oscad { class FunctionLiteral; +class Scope; } // namespace oscad namespace oscadeval { @@ -75,6 +76,14 @@ using Value = std::variant capturedLet; + // The literal's own lexical scope, captured when the closure is made. + // A node no longer carries its scope (one parsed tree is shared + // between evaluations -- see oscad::ScopeTable), and a closure can be + // called from generate time, where there is no EvalContext left to + // look it up through. Not part of identity: operator== still compares + // `node`, since the same literal always resolves to the same scope + // within one run. + const oscad::Scope* scope = nullptr; friend bool operator==(const Closure& a, const Closure& b) { return a.node == b.node; } }; diff --git a/pyproject.toml b/pyproject.toml index ccad006..115970f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "1.3.1" +version = "1.4.0" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/builtins/topology.cpp b/src/builtins/topology.cpp index 4732d37..ef89768 100644 --- a/src/builtins/topology.cpp +++ b/src/builtins/topology.cpp @@ -873,7 +873,7 @@ std::vector generateLevelSet(Evaluator& ev, const CSGParams& params std::optional fnCtx; std::vector fnParams2; if (fieldFn) { - fnCtx = EvalContext::makeRoot((*fieldFn)->node->scope()); + fnCtx = EvalContext::makeRoot((*fieldFn)->scope); for (const auto& prm : (*fieldFn)->node->parameters) fnParams2.push_back(prm->name->name); } diff --git a/src/bytecode_compiler.cpp b/src/bytecode_compiler.cpp index 6940898..26690be 100644 --- a/src/bytecode_compiler.cpp +++ b/src/bytecode_compiler.cpp @@ -216,7 +216,7 @@ class Compiler; // Returns false (chunk left partially populated, discarded by the caller) // for any NotCompilable thrown while compiling. bool compileFunctionLike(CompiledChunk& chunk, const oscad::Scope* staticScope, const oscad::ASTNode* selfDecl, - std::vector enclosing, + const oscad::ScopeTable* scopeTable, std::vector enclosing, const std::vector>& params, const oscad::Expression& bodyExpr); @@ -238,8 +238,16 @@ class Compiler { // when compiling a FunctionLiteral nested inside another compile -- // see the FunctionLiteral case in compileExpr). Compiler(CompiledChunk& chunk, const oscad::Scope* scope, const oscad::ASTNode* selfDecl, - std::vector enclosing) - : chunk_(chunk), scope_(scope), selfDecl_(selfDecl), enclosing_(std::move(enclosing)) {} + const oscad::ScopeTable* scopeTable, std::vector enclosing) + : chunk_(chunk), scope_(scope), selfDecl_(selfDecl), scopeTable_(scopeTable), + enclosing_(std::move(enclosing)) {} + + // A node's own lexical scope, or null. Replaces ASTNode::scope(), + // which is gone: one parsed tree is shared between evaluations, so a + // node's scope lives in the run's ScopeTable, not the node. + const oscad::Scope* scopeOf(const oscad::ASTNode& node) const { + return scopeTable_ ? scopeTable_->get(node) : nullptr; + } int nextSlot() const { return nextSlot_; } int nextIterList() const { return nextIterList_; } @@ -408,7 +416,7 @@ class Compiler { std::vector childEnclosing = enclosing_; childEnclosing.push_back({selfDecl_, &scope}); CompiledChunk literalChunk; - if (!compileFunctionLike(literalChunk, n.scope(), &n, std::move(childEnclosing), n.parameters, + if (!compileFunctionLike(literalChunk, scopeOf(n), &n, scopeTable_, std::move(childEnclosing), n.parameters, *n.body)) { throw NotCompilable{}; } @@ -1461,11 +1469,12 @@ class Compiler { struct ScopeGuard { const oscad::Scope*& scope; const oscad::Scope* saved; - ScopeGuard(const oscad::Scope*& s, const oscad::ASTNode& node) : scope(s), saved(s) { - if (const oscad::Scope* stmtScope = node.scope()) scope = stmtScope; + ScopeGuard(const oscad::Scope*& s, const oscad::ASTNode& node, const oscad::ScopeTable* table) + : scope(s), saved(s) { + if (const oscad::Scope* stmtScope = table ? table->get(node) : nullptr) scope = stmtScope; } ~ScopeGuard() { scope = saved; } - } scopeGuard(scope_, stmt); + } scopeGuard(scope_, stmt, scopeTable_); switch (stmt.kind()) { // Pure declarations, no-ops at statement-eval time (already // hoisted into scope by buildScopes()) -- matches evalStatement's @@ -1612,7 +1621,7 @@ class Compiler { } case NodeKind::ModularCall: { auto& call = static_cast(stmt); - const oscad::Scope* lookupScope = stmt.scope() ? stmt.scope() : scope_; + const oscad::Scope* lookupScope = scopeOf(stmt) ? scopeOf(stmt) : scope_; const oscad::ASTNode* resolved = lookupScope ? lookupScope->lookupModule(call.name->name) : nullptr; if (resolved && resolved->kind() == NodeKind::ModuleDeclaration) { CompiledChunk::ModuleCallSite site; @@ -1842,17 +1851,18 @@ class Compiler { CompiledChunk& chunk_; const oscad::Scope* scope_; const oscad::ASTNode* selfDecl_; + const oscad::ScopeTable* scopeTable_ = nullptr; std::vector enclosing_; int nextSlot_ = 0; int nextIterList_ = 0; }; bool compileFunctionLike(CompiledChunk& chunk, const oscad::Scope* staticScope, const oscad::ASTNode* selfDecl, - std::vector enclosing, + const oscad::ScopeTable* scopeTable, std::vector enclosing, const std::vector>& params, const oscad::Expression& bodyExpr) { chunk.selfDecl = selfDecl; - Compiler compiler(chunk, staticScope, selfDecl, std::move(enclosing)); + Compiler compiler(chunk, staticScope, selfDecl, scopeTable, std::move(enclosing)); CompileScope bodyScope; bodyScope.push(); for (const auto& p : params) { @@ -1888,16 +1898,20 @@ bool compileFunctionLike(CompiledChunk& chunk, const oscad::Scope* staticScope, } // namespace -std::optional tryCompileFunction(const oscad::FunctionDeclaration& decl) { +std::optional tryCompileFunction(const oscad::FunctionDeclaration& decl, + const oscad::ScopeTable* scopeTable) { CompiledChunk chunk; - if (!compileFunctionLike(chunk, decl.scope(), &decl, {}, decl.parameters, *decl.expr)) return std::nullopt; + if (!compileFunctionLike(chunk, scopeTable ? scopeTable->get(decl) : nullptr, &decl, scopeTable, {}, + decl.parameters, *decl.expr)) + return std::nullopt; return chunk; } -std::optional tryCompileStatementExpr(const oscad::Expression& expr, const oscad::Scope* scope) { +std::optional tryCompileStatementExpr(const oscad::Expression& expr, const oscad::Scope* scope, + const oscad::ScopeTable* scopeTable) { static const std::vector> kNoParams; CompiledChunk chunk; - if (!compileFunctionLike(chunk, scope, nullptr, {}, kNoParams, expr)) return std::nullopt; + if (!compileFunctionLike(chunk, scope, nullptr, scopeTable, {}, kNoParams, expr)) return std::nullopt; // See this function's own doc comment (bytecode_compiler.hpp) for why a // captures-having nested closure can't be supported by this bare // wrapper -- selfDecl is nullptr and enclosing is empty above, so its @@ -1910,6 +1924,7 @@ std::optional tryCompileStatementExpr(const oscad::Expression& ex } std::optional tryCompileAssignmentBlock(const std::vector& assigns, + const oscad::ScopeTable* scopeTable, const oscad::Scope* scope) { // Reassignment-warning fidelity (see this function's own doc comment, // bytecode_compiler.hpp) -- cheap, one-time scan before touching the @@ -1926,7 +1941,7 @@ std::optional tryCompileAssignmentBlock(const std::vector tryCompileAssignmentBlock(const std::vector tryCompileModuleBody(const oscad::ModuleDeclaration& decl) { +std::optional tryCompileModuleBody(const oscad::ModuleDeclaration& decl, + const oscad::ScopeTable* scopeTable) { CompiledChunk chunk; chunk.isModule = true; chunk.selfDecl = &decl; @@ -1978,7 +1994,7 @@ std::optional tryCompileModuleBody(const oscad::ModuleDeclaration // compile path, just scoped to that one statement's own sub- // expression -- see this file's own module-chunk doc comment // (bytecode.hpp) for the full reasoning. - Compiler compiler(chunk, decl.scope(), nullptr, {}); + Compiler compiler(chunk, scopeTable ? scopeTable->get(decl) : nullptr, nullptr, scopeTable, {}); try { compiler.compileStatementList(decl.children, chunk.bodyCode); } catch (const NotCompilable&) { @@ -1990,6 +2006,7 @@ std::optional tryCompileModuleBody(const oscad::ModuleDeclaration } std::optional tryCompileChildrenList(const std::vector& children, + const oscad::ScopeTable* scopeTable, const oscad::Scope* scope) { CompiledChunk chunk; // Same completion semantics as a module chunk (no return value, its @@ -2001,7 +2018,7 @@ std::optional tryCompileChildrenList(const std::vectorfind(cap.name); capturedTrail->set(cap.name, v ? *v : Value{}); } - auto closure = std::make_shared(Closure{site.node, capturedTrail}); + auto closure = std::make_shared(Closure{site.node, capturedTrail, ctx.scopeOf(*site.node)}); for (const std::string* selfName : selfNames) { capturedTrail->set(*selfName, Value{closure}); } @@ -810,7 +810,7 @@ Value driveVm(Evaluator& ev, size_t floor) { BoundArgs bound = buildBoundArgs(ev, site, args, argCount, site.decl->parameters); const CompiledChunk* calleeChunk = ev.useBytecodeVm() ? ev.lookupOrCompileChunk(*site.decl) : nullptr; if (calleeChunk) { - const oscad::Scope* fnScope = site.decl->scope() ? site.decl->scope() : ctx.scope; + const oscad::Scope* fnScope = ctx.scopeOf(*site.decl) ? ctx.scopeOf(*site.decl) : ctx.scope; pushBracketedCallFrame(ev, *calleeChunk, *site.decl, *site.decl->expr, site.calleeName, std::move(bound), ctx, fnScope, nullptr, &site.callNode->position()); // f.pc deliberately NOT advanced -- resumes when the @@ -841,7 +841,7 @@ Value driveVm(Evaluator& ev, size_t floor) { BoundArgs bound = buildBoundArgs(ev, site, args, argCount, funcNode.parameters); const CompiledChunk* calleeChunk = ev.useBytecodeVm() ? ev.lookupCompiledLiteralChunk(funcNode) : nullptr; if (calleeChunk) { - const oscad::Scope* fnScope = funcNode.scope() ? funcNode.scope() : ctx.scope; + const oscad::Scope* fnScope = ctx.scopeOf(funcNode) ? ctx.scopeOf(funcNode) : ctx.scope; pushBracketedCallFrame(ev, *calleeChunk, funcNode, *funcNode.body, "", std::move(bound), ctx, fnScope, capturedLetTrail(closure), ins.pos); } else { @@ -906,7 +906,7 @@ Value driveVm(Evaluator& ev, size_t floor) { const CompiledChunk* fallbackChunk = ev.useBytecodeVm() ? ev.lookupOrCompileChunk(*site.decl) : nullptr; if (fallbackChunk) { - const oscad::Scope* fnScope = site.decl->scope() ? site.decl->scope() : ctx.scope; + const oscad::Scope* fnScope = ctx.scopeOf(*site.decl) ? ctx.scopeOf(*site.decl) : ctx.scope; pushBracketedCallFrame(ev, *fallbackChunk, *site.decl, *site.decl->expr, site.calleeName, std::move(bound), ctx, fnScope, nullptr, &site.callNode->position()); } else { @@ -956,7 +956,7 @@ Value driveVm(Evaluator& ev, size_t floor) { const CompiledChunk* fallbackChunk = ev.useBytecodeVm() ? ev.lookupCompiledLiteralChunk(funcNode) : nullptr; if (fallbackChunk) { - const oscad::Scope* fnScope = funcNode.scope() ? funcNode.scope() : ctx.scope; + const oscad::Scope* fnScope = ctx.scopeOf(funcNode) ? ctx.scopeOf(funcNode) : ctx.scope; pushBracketedCallFrame(ev, *fallbackChunk, funcNode, *funcNode.body, "", std::move(bound), ctx, fnScope, capturedLetTrail(closure), ins.pos); } else { @@ -1040,7 +1040,7 @@ Value driveVm(Evaluator& ev, size_t floor) { // unobservable). const auto* callNode = static_cast(f.chunk->nativeStatements[static_cast(ins.a)]); - EvalContext scopedCtx = ctx.withScope(callNode->scope() ? callNode->scope() : ctx.scope); + EvalContext scopedCtx = ctx.withScope(ctx.scopeOf(*callNode) ? ctx.scopeOf(*callNode) : ctx.scope); ev.checkDebug(*callNode, scopedCtx); // Same order as evalModularCall's own (csg_resolve.cpp): // warn, then resolve. Without this a children() typo @@ -1398,7 +1398,7 @@ Value driveVm(Evaluator& ev, size_t floor) { } case Op::NativeStatement: { const oscad::ASTNode* stmt = f.chunk->nativeStatements[static_cast(ins.a)]; - EvalContext childCtx = ctx.withScope(stmt->scope() ? stmt->scope() : ctx.scope); + EvalContext childCtx = ctx.withScope(ctx.scopeOf(*stmt) ? ctx.scopeOf(*stmt) : ctx.scope); // Mirrors evalChildren's own per-statement loop exactly // -- no ModularLet exclusion needed here (unlike that // loop's own): ModularLet has its own compiled form now diff --git a/src/csg_resolve.cpp b/src/csg_resolve.cpp index d456543..6038ee1 100644 --- a/src/csg_resolve.cpp +++ b/src/csg_resolve.cpp @@ -205,6 +205,9 @@ void Evaluator::evalIntersectionForNode(const oscad::ModularIntersectionFor& nod template std::vector> Evaluator::resolveTreeImpl(const NodeList& nodes, EvalContext& ctx) { + // Latch the run's ScopeTable for the helpers the VM reaches without an + // EvalContext -- see Evaluator::scopeOfNode. + scopeTable_ = ctx.scopeTable; idToNode.clear(); idToColor.clear(); syntheticNodes_.clear(); @@ -246,6 +249,9 @@ template std::vector Evaluator::evaluateImpl(const NodeList& nodes, EvalContext& ctx, const std::unordered_map& viewportParams, bool generate) { + // Latch the run's ScopeTable for the helpers the VM reaches without an + // EvalContext -- see Evaluator::scopeOfNode. + scopeTable_ = ctx.scopeTable; // Seeds ctx.dyn directly, deliberately not touching ctx.dynExplicit -- // see this method's own doc comment in evaluator.hpp for why that // distinction matters to a caller. diff --git a/src/eval_context.cpp b/src/eval_context.cpp index 436a25b..09245a9 100644 --- a/src/eval_context.cpp +++ b/src/eval_context.cpp @@ -2,9 +2,12 @@ namespace oscadeval { -EvalContext EvalContext::makeRoot(const oscad::Scope* rootScope) { +EvalContext EvalContext::makeRoot(const oscad::Scope* rootScope, const oscad::ScopeTable* scopeTable) { + // Default: the table buildScopes() attached to this scope tree's root. + if (!scopeTable && rootScope) scopeTable = rootScope->rootTable(); EvalContext ctx; ctx.scope = rootScope; + ctx.scopeTable = scopeTable; // dyn/dynExplicit share one underlying trail -- see scope_trail.hpp's // own doc comment on DynValueView/DynExplicitView. auto dynTrail = IndexedTrailView::makeRoot(std::make_shared()); @@ -66,6 +69,10 @@ EvalContext EvalContext::childCtx(const oscad::Scope* newScope, std::optional newChildrenNodes, const EvalContext* newChildrenCallerCtx) const { EvalContext result; + // Constant for the whole run; a derived context must never lose it, or + // a node's scope reads back as null and name resolution silently falls + // back to the enclosing scope. + result.scopeTable = scopeTable; result.scope = newScope ? newScope : scope; auto newDynTrail = dyn.trail()->openChild(/*isolate=*/false); result.dyn = DynValueView(newDynTrail); @@ -83,6 +90,10 @@ EvalContext EvalContext::callCtx(const oscad::Scope* newScope, std::optional newChildrenNodes, const EvalContext* newChildrenCallerCtx) const { EvalContext result; + // Constant for the whole run; a derived context must never lose it, or + // a node's scope reads back as null and name resolution silently falls + // back to the enclosing scope. + result.scopeTable = scopeTable; result.scope = newScope ? newScope : scope; auto newDynTrail = dyn.trail()->openChild(/*isolate=*/false); // stays dynamically scoped through result.dyn = DynValueView(newDynTrail); @@ -101,6 +112,10 @@ EvalContext EvalContext::callCtxFromCapturedLet(const std::shared_ptr newChildrenNodes, const EvalContext* newChildrenCallerCtx) const { EvalContext result; + // Constant for the whole run; a derived context must never lose it, or + // a node's scope reads back as null and name resolution silently falls + // back to the enclosing scope. + result.scopeTable = scopeTable; result.scope = newScope ? newScope : scope; auto newDynTrail = dyn.trail()->openChild(/*isolate=*/false); // stays dynamically scoped through the CALL SITE result.dyn = DynValueView(newDynTrail); diff --git a/src/eval_use.cpp b/src/eval_use.cpp index 48136f7..781def3 100644 --- a/src/eval_use.cpp +++ b/src/eval_use.cpp @@ -8,6 +8,17 @@ namespace oscadeval { ResolvedUseScopes resolveUseScopes(const std::vector>& ownNodes, const std::string& currentFile, const std::function& logFn) { + // Borrow, then share the one implementation: nothing here needs + // ownership, and the cached-include path (ParsedProgram) only ever has + // borrowed nodes to offer. + std::vector borrowed; + borrowed.reserve(ownNodes.size()); + for (const auto& n : ownNodes) borrowed.push_back(n.get()); + return resolveUseScopes(borrowed, currentFile, logFn); +} + +ResolvedUseScopes resolveUseScopes(const std::vector& ownNodes, + const std::string& currentFile, const std::function& logFn) { ResolvedUseScopes result; std::vector injected; @@ -16,7 +27,7 @@ ResolvedUseScopes resolveUseScopes(const std::vector, oscad::Scope*>> reanchor; - for (const auto& nodePtr : ownNodes) { + for (const oscad::ASTNode* nodePtr : ownNodes) { if (nodePtr->kind() != oscad::NodeKind::UseStatement) continue; const auto& useNode = static_cast(*nodePtr); @@ -64,8 +75,8 @@ ResolvedUseScopes resolveUseScopes(const std::vectorkind() != oscad::NodeKind::UseStatement) result.ownNodesFiltered.push_back(nodePtr.get()); + for (const oscad::ASTNode* nodePtr : ownNodes) { + if (nodePtr->kind() != oscad::NodeKind::UseStatement) result.ownNodesFiltered.push_back(nodePtr); } result.processedNodes = injected; @@ -83,8 +94,13 @@ ResolvedUseScopes resolveUseScopes(const std::vector(n)); result.rootScope = oscad::buildScopes(mutableProcessed); - for (const auto& [libInjected, libRootScope] : reanchor) { - for (const oscad::ASTNode* n : libInjected) const_cast(n)->buildScope(*libRootScope); + { + // 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())); + for (const auto& [libInjected, libRootScope] : reanchor) { + for (const oscad::ASTNode* n : libInjected) const_cast(n)->buildScope(*libRootScope); + } } return result; diff --git a/src/expr_eval.cpp b/src/expr_eval.cpp index f715dfa..6195b68 100644 --- a/src/expr_eval.cpp +++ b/src/expr_eval.cpp @@ -456,7 +456,7 @@ Value Evaluator::evalExpr(const oscad::Expression& node, EvalContext& ctx) { // time, matching real OpenSCAD), and the lexical scope is // static AST data already reachable via the node itself. return Value{std::make_shared( - Closure{static_cast(&node), ctx.let_})}; + Closure{static_cast(&node), ctx.let_, ctx.scopeOf(node)})}; case NodeKind::PrimaryCall: return evalFunctionCall(static_cast(node), ctx); case NodeKind::LetOp: diff --git a/src/stmt_eval.cpp b/src/stmt_eval.cpp index 087397d..230c5ad 100644 --- a/src/stmt_eval.cpp +++ b/src/stmt_eval.cpp @@ -291,7 +291,7 @@ void Evaluator::evalChildren(const std::vector& children, auto runAll = [&](const std::vector& nodes) { for (const oscad::ASTNode* child : nodes) { - const oscad::Scope* childScope = child->scope() ? child->scope() : ctx.scope; + const oscad::Scope* childScope = ctx.scopeOf(*child) ? ctx.scopeOf(*child) : ctx.scope; EvalContext childCtx = ctx.withScope(childScope); // Safe despite childCtx's own scope ending at this iteration's // close: lastCtx_ is only ever read (by error()) synchronously diff --git a/src/user_calls.cpp b/src/user_calls.cpp index 918d10b..2920910 100644 --- a/src/user_calls.cpp +++ b/src/user_calls.cpp @@ -11,7 +11,7 @@ namespace oscadeval { const CompiledChunk* Evaluator::lookupOrCompileChunk(const oscad::FunctionDeclaration& decl) { auto it = chunkCache_.find(&decl); if (it == chunkCache_.end()) { - it = chunkCache_.emplace(&decl, tryCompileFunction(decl)).first; + it = chunkCache_.emplace(&decl, tryCompileFunction(decl, scopeTable_)).first; if (it->second) flattenNestedLiterals(*it->second); } if (!it->second) return nullptr; @@ -33,7 +33,7 @@ const CompiledChunk* Evaluator::lookupCompiledLiteralChunk(const oscad::Function const CompiledChunk* Evaluator::lookupOrCompileModuleChunk(const oscad::ModuleDeclaration& decl) { auto it = moduleChunkCache_.find(&decl); if (it == moduleChunkCache_.end()) { - it = moduleChunkCache_.emplace(&decl, tryCompileModuleBody(decl)).first; + it = moduleChunkCache_.emplace(&decl, tryCompileModuleBody(decl, scopeTable_)).first; if (it->second) flattenNestedLiterals(*it->second); } if (!it->second) return nullptr; @@ -52,7 +52,7 @@ Value Evaluator::evalExprMaybeCompiled(const oscad::Expression& node, EvalContex if (!useBytecodeVm() || !inResolvePass_) return evalExpr(node, ctx); auto it = stmtExprChunkCache_.find(&node); if (it == stmtExprChunkCache_.end()) { - it = stmtExprChunkCache_.emplace(&node, tryCompileStatementExpr(node, node.scope())).first; + it = stmtExprChunkCache_.emplace(&node, tryCompileStatementExpr(node, scopeOfNode(node), scopeTable_)).first; // A zero-capture closure literal (e.g. `x = function(y) y + 1;`) // still reaches chunk.nestedLiterals even though it never touches // closureSites (see tryCompileStatementExpr's own doc comment: only @@ -74,7 +74,7 @@ bool Evaluator::tryRunCompiledAssignmentBlock(const std::vector assigns; assigns.reserve(assignments.size()); for (const oscad::ASTNode* n : assignments) assigns.push_back(static_cast(n)); - it = assignBlockChunkCache_.emplace(first, tryCompileAssignmentBlock(assigns, first->scope())).first; + it = assignBlockChunkCache_.emplace(first, tryCompileAssignmentBlock(assigns, scopeTable_, scopeOfNode(*first))).first; if (it->second) flattenNestedLiterals(*it->second); } if (!it->second || !chunkEligibleNow(*it->second)) return false; @@ -102,7 +102,7 @@ const CompiledChunk* Evaluator::lookupOrCompileChildrenListChunk(const std::vect const auto key = std::make_pair(first, children.size()); auto it = childrenListChunkCache_.find(key); if (it == childrenListChunkCache_.end()) { - it = childrenListChunkCache_.emplace(key, tryCompileChildrenList(children, first->scope())).first; + it = childrenListChunkCache_.emplace(key, tryCompileChildrenList(children, scopeTable_, scopeOfNode(*first))).first; if (it->second) flattenNestedLiterals(*it->second); } if (!it->second || !chunkEligibleNow(*it->second)) return nullptr; @@ -277,7 +277,7 @@ std::optional Evaluator::isolatedCallCtxFor(const oscad::ASTNode& d // scope to walk outward from -- the caller's scope at the call site // has no relation to it at all. bool usedChildCtx = false; - const oscad::Scope* declScope = declNode.scope() ? declNode.scope() : ctx.scope; + const oscad::Scope* declScope = ctx.scopeOf(declNode) ? ctx.scopeOf(declNode) : ctx.scope; EvalContext result = callCtxFor(declNode, ctx, declScope, nullptr, nullptr, &usedChildCtx, capturedLet); if (usedChildCtx) return std::nullopt; return result; @@ -337,7 +337,7 @@ void Evaluator::bindCallArgsInto(const std::vector Evaluator::tryTailStepFor( bool hasCompiledChunk, const std::vector>& arguments, EvalContext& ctx, const oscad::Position& callPos, const std::shared_ptr>& capturedLet) { if (hasCompiledChunk) return std::nullopt; - const oscad::Scope* fnScope = declNode.scope() ? declNode.scope() : ctx.scope; + const oscad::Scope* fnScope = ctx.scopeOf(declNode) ? ctx.scopeOf(declNode) : ctx.scope; bool usedChildCtx = false; EvalContext childCtx = callCtxFor(declNode, ctx, fnScope, nullptr, nullptr, &usedChildCtx, capturedLet); if (usedChildCtx) return std::nullopt; @@ -769,7 +769,7 @@ void Evaluator::exitUserCallException(const UserCallHandle& handle) { Value Evaluator::evalUserFunction(const std::string& name, const oscad::FunctionDeclaration& decl, const std::vector>& arguments, EvalContext& ctx, const oscad::ASTNode* callNode) { - const oscad::Scope* fnScope = decl.scope() ? decl.scope() : ctx.scope; + const oscad::Scope* fnScope = ctx.scopeOf(decl) ? ctx.scopeOf(decl) : ctx.scope; const int callerFrameIdx = callStack_.empty() ? -1 : static_cast(callStack_.size()) - 1; bool usedChildCtx = false; EvalContext childCtx = callCtxFor(decl, ctx, fnScope, nullptr, nullptr, &usedChildCtx); @@ -794,7 +794,7 @@ Value Evaluator::evalUserFunction(const std::string& name, const oscad::Function Value Evaluator::evalUserFunctionFromBound(const std::string& name, const oscad::FunctionDeclaration& decl, BoundArgs bound, EvalContext& ctx, const oscad::Position* callPos) { - const oscad::Scope* fnScope = decl.scope() ? decl.scope() : ctx.scope; + const oscad::Scope* fnScope = ctx.scopeOf(decl) ? ctx.scopeOf(decl) : ctx.scope; const int callerFrameIdx = callStack_.empty() ? -1 : static_cast(callStack_.size()) - 1; bool usedChildCtx = false; EvalContext childCtx = callCtxFor(decl, ctx, fnScope, nullptr, nullptr, &usedChildCtx); @@ -814,7 +814,7 @@ Value Evaluator::evalFunctionLiteral(const Closure& closure, const std::vector>& arguments, EvalContext& ctx, const oscad::ASTNode* callNode) { const oscad::FunctionLiteral& funcNode = *closure.node; - const oscad::Scope* fnScope = funcNode.scope() ? funcNode.scope() : ctx.scope; + const oscad::Scope* fnScope = ctx.scopeOf(funcNode) ? ctx.scopeOf(funcNode) : ctx.scope; const int callerFrameIdx = callStack_.empty() ? -1 : static_cast(callStack_.size()) - 1; bool usedChildCtx = false; EvalContext childCtx = @@ -850,7 +850,7 @@ Value Evaluator::evalFunctionLiteral(const Closure& closure, Value Evaluator::evalFunctionLiteralFromBound(const Closure& closure, BoundArgs bound, EvalContext& ctx, const oscad::Position* callPos) { const oscad::FunctionLiteral& funcNode = *closure.node; - const oscad::Scope* fnScope = funcNode.scope() ? funcNode.scope() : ctx.scope; + const oscad::Scope* fnScope = ctx.scopeOf(funcNode) ? ctx.scopeOf(funcNode) : ctx.scope; const int callerFrameIdx = callStack_.empty() ? -1 : static_cast(callStack_.size()) - 1; bool usedChildCtx = false; EvalContext childCtx = diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 353af09..52d8829 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -27,6 +27,7 @@ add_executable(oscad_eval_tests test_function_builtins.cpp test_import_export.cpp test_zip_stored.cpp + test_include_cache.cpp test_use.cpp test_extrude_roof.cpp test_surface.cpp diff --git a/tests/test_include_cache.cpp b/tests/test_include_cache.cpp new file mode 100644 index 0000000..ccd5c8c --- /dev/null +++ b/tests/test_include_cache.cpp @@ -0,0 +1,132 @@ +// The shared include cache (oscad::getProgramFromFile) and the ScopeTable +// that makes sharing possible (oscad::ScopeTable). +// +// A parsed tree used to be owned by whoever asked for it, and each node +// carried its own Scope*. Both had to change together: sharing one tree +// between evaluations is what makes a re-render cheap (parsing +// BOSL2/std.scad is ~55ms and ~82% of evaluating a small script against +// it), and a shared node cannot hold a scope, because `include` puts it in +// the INCLUDER's scope -- a different one per script. + +#include "openscad_cpp_evaluator/eval_use.hpp" +#include "openscad_cpp_evaluator/evaluator.hpp" + +#include "openscad_cpp_parser/api.hpp" + +#include +#include +#include + +using namespace oscadeval; + +namespace { + +std::filesystem::path writeFile(const std::string& name, const std::string& content) { + const auto p = std::filesystem::temp_directory_path() / ("oscad_inccache_" + name); + std::ofstream out(p); + out << content; + return p; +} + +std::vector echoesOf(const std::filesystem::path& path) { + std::vector echoes; + auto log = [&echoes](const std::string& m) { echoes.push_back(m); }; + oscad::ParsedProgram program = oscad::getProgramFromFile(path.string()); + ResolvedUseScopes used = resolveUseScopes(program.nodes, path.string(), log); + Evaluator ev(log); + EvalContext ctx = EvalContext::makeRoot(used.rootScope.get()); + ev.evaluate(used.processedNodes, ctx, {}, /*generate=*/false); + return echoes; +} + +} // namespace + +TEST(IncludeCache, SameFileIncludedTwiceContributesItsStatementsOnce) { + // A file is spliced in at most once per resolution. This is why only + // the per-FILE parses are cached and never a whole resolved program: + // caching `mid` already-resolved would splice `base` twice here. + writeFile("base.scad", "BASE = 1;\n"); + const auto mid = writeFile("mid.scad", "include \nMID = 2;\n"); + const auto top = writeFile("top.scad", + "include \n" + "include \n" + "echo(BASE + MID);\n"); + (void)mid; + oscad::ParsedProgram program = oscad::getProgramFromFile(top.string()); + int baseAssignments = 0; + for (const oscad::ASTNode* n : program.nodes) { + if (n->kind() == oscad::NodeKind::Assignment && + static_cast(*n).name->name == "BASE") + ++baseAssignments; + } + EXPECT_EQ(baseAssignments, 1); + EXPECT_EQ(echoesOf(top), std::vector{"ECHO: 3"}); +} + +TEST(IncludeCache, TwoProgramsShareTheSameParsedNodes) { + // The whole point: the second script must not re-parse the library. + writeFile("lib.scad", "LIB = 7;\nfunction libf(x) = x * 2;\n"); + const auto a = writeFile("a.scad", "include \necho(libf(LIB));\n"); + const auto b = writeFile("b.scad", "include \necho(libf(LIB) + 1);\n"); + + oscad::ParsedProgram pa = oscad::getProgramFromFile(a.string()); + oscad::ParsedProgram pb = oscad::getProgramFromFile(b.string()); + // Same node ADDRESSES, not merely equal trees. + EXPECT_EQ(pa.nodes.front(), pb.nodes.front()); + + // And both still evaluate correctly, each in its own scope. + EXPECT_EQ(echoesOf(a), std::vector{"ECHO: 14"}); + EXPECT_EQ(echoesOf(b), std::vector{"ECHO: 15"}); +} + +TEST(IncludeCache, EditingAnIncludedFileReplacesItsEntryRatherThanAddingOne) { + // Keyed on (path, mtime, size), so an edit invalidates on its own. + const auto lib = writeFile("edited.scad", "V = 1;\n"); + const auto top = writeFile("edtop.scad", "include \necho(V);\n"); + EXPECT_EQ(echoesOf(top), std::vector{"ECHO: 1"}); + const size_t cacheEntriesBefore = oscad::astCacheSize(); + + std::filesystem::last_write_time(lib, std::filesystem::last_write_time(lib) + std::chrono::seconds(2)); + { std::ofstream out(lib); out << "V = 99;\n"; } + std::filesystem::last_write_time(lib, std::filesystem::last_write_time(lib) + std::chrono::seconds(4)); + EXPECT_EQ(echoesOf(top), std::vector{"ECHO: 99"}); + + // The old version must not still be held: an editor re-renders on every + // save, so keying the cache by (path, stamp) instead of replacing in + // place would keep a full AST copy of every version ever saved. + oscad::ParsedProgram again = oscad::getProgramFromFile(top.string()); + const oscad::ASTNode* first = again.nodes.front(); + ASSERT_EQ(first->kind(), oscad::NodeKind::Assignment); + EXPECT_EQ(oscad::astCacheSize(), cacheEntriesBefore) << "editing a file must replace its entry, not add one"; +} + +TEST(IncludeCache, ADerivedContextKeepsTheScopeTable) { + // The bug this exists for: EvalContext::childCtx/callCtx build a FRESH + // context field by field, and one that forgot to carry scopeTable made + // every node's scope read back as null. Name resolution then fell back + // to the enclosing scope, so a module body calling a builtin of its own + // name found ITSELF -- BOSL2's `module _cube(...) cube(...);` wrapper + // recursed until the depth limit. Nothing else in the suite noticed. + const auto top = writeFile("derived.scad", + "module _mycube(s) cube(s);\n" + "module wrap(s) _mycube(s);\n" + "wrap(2);\n"); + std::vector logs; + auto log = [&logs](const std::string& m) { logs.push_back(m); }; + oscad::ParsedProgram program = oscad::getProgramFromFile(top.string()); + ResolvedUseScopes used = resolveUseScopes(program.nodes, top.string(), log); + Evaluator ev(log); + EvalContext ctx = EvalContext::makeRoot(used.rootScope.get()); + ASSERT_NE(ctx.scopeTable, nullptr); + + // Every derived shape must carry it -- checked directly, so a new + // constructor that forgets fails here rather than as a recursion + // blow-up in somebody's library. + EXPECT_EQ(ctx.withScope(ctx.scope).scopeTable, ctx.scopeTable); + EXPECT_EQ(ctx.childCtx().scopeTable, ctx.scopeTable); + EXPECT_EQ(ctx.callCtx().scopeTable, ctx.scopeTable); + EXPECT_EQ(ctx.letChildCtx().scopeTable, ctx.scopeTable); + + ev.evaluate(used.processedNodes, ctx, {}, /*generate=*/false); + for (const std::string& m : logs) EXPECT_EQ(m.find("Recursion"), std::string::npos) << m; +}