diff --git a/include/openscad_cpp_parser/api.hpp b/include/openscad_cpp_parser/api.hpp index 646f899..c41b4ca 100644 --- a/include/openscad_cpp_parser/api.hpp +++ b/include/openscad_cpp_parser/api.hpp @@ -1,6 +1,7 @@ #pragma once #include "openscad_cpp_parser/ast.hpp" +#include "openscad_cpp_parser/scope_table.hpp" #include "openscad_cpp_parser/source_map.hpp" #include @@ -68,15 +69,10 @@ std::vector> getASTFromString(const std::string& code, // can't be found/read, or ParseError (see getASTFromString) on a syntax // error in any of the involved files. // -// ponytail: unlike the Python reference, this does NOT cache parsed ASTs -// (in-memory or on-disk) across calls. With unique_ptr ownership, "return -// the same cached tree to every caller" isn't representable (each caller -// needs exclusive ownership), and a clone-based cache would need a deep -// clone() for all 66 node kinds just to support an unmeasured perf -// optimization. Every call re-parses. Upgrade path: add a per-node -// clone() (mirroring toJson/toString's per-kind dispatch) and cache -// serialized snapshots keyed by (path, mtime) if repeated-parse cost of a -// real workload is shown to matter. +// This does NOT share or cache anything: every call re-parses the file and +// every file it includes, and the caller owns all of it. See +// getProgramFromFile() for the shared, cached form -- the one an evaluator +// that re-renders the same script should use. std::vector> getASTFromFile(const std::string& file, bool includeComments = false, bool processIncludes = true); @@ -95,9 +91,44 @@ LibraryFileResult getASTFromLibraryFile(const std::string& currFile, const std:: // platform default library dir. std::optional findLibraryFile(const std::string& currFile, const std::string& libFile); -// No-op: kept for API-shape parity with the Python reference's -// clear_ast_cache(). See the ponytail note on getASTFromFile() -- this -// port doesn't cache, so there's nothing to clear. +// A file's statements with its `include <...>` directives resolved, where +// each included file's AST is SHARED with every other file that includes +// it rather than re-parsed. +// +// That sharing is the point. Parsing `include ` costs +// ~55ms and is ~82% of evaluating a small BOSL2 script; the library does +// not change between renders, so re-parsing it every time is the single +// largest cost in a re-render or a docs build (which renders ~1000 +// examples, each including the same library). +// +// Only the per-FILE parses are cached, not a whole resolved program: a +// file is spliced in at most once per resolution (`visited`), so caching +// std.scad already-resolved would double its statements in a script that +// also includes something else depending on it. Re-running the resolution +// is only pointer pushes. +struct ParsedProgram { + // The flattened statement list, in source order, includes spliced in + // where their directives stood. Non-owning: see keepAlive. + std::vector nodes; + // Holds every AST the above points into -- this file's own parse and + // each shared include -- alive for as long as this object. Nodes are + // borrowed, never owned, which is what lets two scripts (or two + // threads) use the same parsed library at once. + std::vector>>> keepAlive; +}; + +// Parses `file` and resolves its includes against the cache, which is keyed +// by (path, mtime, size) so an edited file re-parses on its own. +// Thread-safe. Throws exactly as getASTFromFile does. +ParsedProgram getProgramFromFile(const std::string& file, bool includeComments = false); + +// Drops every cached file parse. Nothing already handed out is +// invalidated -- a ParsedProgram keeps what it borrowed alive. void clearAstCache(); +// How many file parses the cache is holding. For tests that need to prove +// the cache replaces an edited file's entry rather than accumulating one +// per save. +size_t astCacheSize(); + } // namespace oscad diff --git a/include/openscad_cpp_parser/ast.hpp b/include/openscad_cpp_parser/ast.hpp index dbc9d06..7b85d89 100644 --- a/include/openscad_cpp_parser/ast.hpp +++ b/include/openscad_cpp_parser/ast.hpp @@ -7,6 +7,7 @@ #include "openscad_cpp_parser/ast/expression.hpp" #include "openscad_cpp_parser/ast/module_instantiation.hpp" #include "openscad_cpp_parser/ast/scope_builder.hpp" +#include "openscad_cpp_parser/scope_table.hpp" #include "openscad_cpp_parser/ast/vector_element.hpp" #include "openscad_cpp_parser/position.hpp" #include "openscad_cpp_parser/scope.hpp" diff --git a/include/openscad_cpp_parser/ast/ast_node.hpp b/include/openscad_cpp_parser/ast/ast_node.hpp index a1a489e..578a823 100644 --- a/include/openscad_cpp_parser/ast/ast_node.hpp +++ b/include/openscad_cpp_parser/ast/ast_node.hpp @@ -2,6 +2,7 @@ #include "openscad_cpp_parser/position.hpp" +#include #include namespace oscad { @@ -98,9 +99,67 @@ const char* nodeKindName(NodeKind kind); // Base class for all AST nodes. Mirrors openscad_lalr_parser.nodes.ASTNode: // every node carries its source Position and (once buildScope() has run) a // non-owning pointer to the Scope visible at that point in the tree. +// Numbering context for the nodes one parse creates: every node built +// while it is installed gets that parse's treeId and a slot dense within +// it -- see ASTNode::slot(). +struct NodeNumbering { + uint32_t treeId = 0; + uint32_t next = 0; +}; + +// Installs a numbering for a whole parse, INCLUDING any pass that builds +// more nodes from its result (attachComments wraps expressions in +// CommentedExpr nodes after the parser proper has finished). Nested use +// reuses the outer numbering rather than starting a second one, so a tree +// stays a tree however many passes contribute to it. +// +// Thread-local: two threads may parse at once and must number +// independently. +class ParseNumberingScope { +public: + ParseNumberingScope(); + ~ParseNumberingScope(); + ParseNumberingScope(const ParseNumberingScope&) = delete; + ParseNumberingScope& operator=(const ParseNumberingScope&) = delete; + +private: + NodeNumbering numbering_; + NodeNumbering* previous_; + bool installed_; +}; + +class ScopeTable; + +// The table setScope() writes into. buildScopes() installs one for its +// walk; thread-local, so two evaluations can build scopes at once over the +// same shared tree -- the whole point of sharing it. +class ScopeTableScope { +public: + explicit ScopeTableScope(ScopeTable& table); + ~ScopeTableScope(); + ScopeTableScope(const ScopeTableScope&) = delete; + ScopeTableScope& operator=(const ScopeTableScope&) = delete; + +private: + ScopeTable* previous_; +}; + +NodeNumbering* currentNodeNumbering(); +// Slot for a node built with no parse active -- a test constructing nodes +// by hand, or astFromJson. Those all land in treeId 0 with process-unique +// slots, so they never collide with each other. +uint32_t nextLooseSlot(); + class ASTNode { public: - ASTNode(NodeKind kind, Position position) : kind_(kind), position_(std::move(position)) {} + ASTNode(NodeKind kind, Position position) : kind_(kind), position_(std::move(position)) { + if (NodeNumbering* n = currentNodeNumbering()) { + treeId_ = n->treeId; + slot_ = n->next++; + } else { + slot_ = nextLooseSlot(); + } + } virtual ~ASTNode() = default; ASTNode(const ASTNode&) = delete; @@ -109,16 +168,21 @@ class ASTNode { NodeKind kind() const { return kind_; } const Position& position() const { return position_; } - // Overloaded on the constness of `this` rather than a single `const` - // method returning `Scope*` unconditionally: a plain raw-pointer - // return type doesn't propagate constness to the pointee, so a caller - // holding only `const ASTNode&` (e.g. a read-only borrow shared across - // threads while one owner keeps the tree alive) could otherwise still - // reach through scope() and call a mutating method like - // defineVariable() on the Scope -- defeating the whole point of the - // borrow being const. This makes that a compile error instead. - Scope* scope() { return scope_; } - const Scope* scope() const { return scope_; } + // Identity of the parse that built this node, and this node's dense + // index within it. Together they address the node's Scope in a + // ScopeTable, which is what lets one parsed tree be shared by several + // evaluations at once: the scope a node sits in depends on the file + // that included it, so it cannot live in the node itself. Both are 0 + // for a node built outside a parse (a test constructing nodes by + // hand), which is a valid single tree of its own. + uint32_t treeId() const { return treeId_; } + uint32_t slot() const { return slot_; } + + // A node's Scope is NOT stored here -- it lives in the render's + // ScopeTable, addressed by (treeId, slot). It has to: one parsed tree + // is shared by every script that includes it, and `include` means the + // included nodes sit in the INCLUDER's scope, so the same node is in a + // different scope in each. Read it with ScopeTable::get(node). // Mirrors Python's __str__: every leaf node overrides this. virtual std::string toString() const = 0; @@ -126,18 +190,24 @@ class ASTNode { // Mirrors Python's build_scope(parent_scope): the default (leaf) case // just records parent_scope; nodes that introduce bindings or new // scopes override this. - virtual void buildScope(Scope& parentScope) { scope_ = &parentScope; } + virtual void buildScope(Scope& parentScope) { setScope(parentScope); } protected: // For buildScope() overrides: records the scope visible at this node, // mirroring Python's `self.scope = parent_scope` (which is often a // *different* scope than what gets passed to this node's children). - void setScope(Scope& s) { scope_ = &s; } + // + // Writes into the ScopeTable buildScopes() installed for the duration + // of its walk, rather than taking one as a parameter: that keeps all + // 37 buildScope() overrides on their existing signature, and the walk + // is a single scoped pass with nothing else running inside it. + void setScope(Scope& s); private: NodeKind kind_; Position position_; - Scope* scope_ = nullptr; + uint32_t treeId_ = 0; + uint32_t slot_ = 0; }; } // namespace oscad diff --git a/include/openscad_cpp_parser/scope.hpp b/include/openscad_cpp_parser/scope.hpp index b55e598..fe64aff 100644 --- a/include/openscad_cpp_parser/scope.hpp +++ b/include/openscad_cpp_parser/scope.hpp @@ -1,5 +1,7 @@ #pragma once +#include "openscad_cpp_parser/scope_table.hpp" + #include #include #include @@ -83,6 +85,19 @@ class Scope { return *children_.back(); } + // The ScopeTable holding every node's scope for this scope tree. + // buildScopes() attaches it to the root it returns, so the table lives + // exactly as long as the scopes it points at -- which is the ownership + // contract callers already keep. Null on a non-root scope; use + // rootTable() to reach it from anywhere in the tree. + void adoptTable(std::unique_ptr table); + const ScopeTable* table() const { return ownedTable_.get(); } + const ScopeTable* rootTable() const { + const Scope* s = this; + while (s->parent_) s = s->parent_; + return s->ownedTable_.get(); + } + private: static ASTNode* find(const std::unordered_map& table, const std::string& name) { auto it = table.find(name); @@ -94,6 +109,16 @@ class Scope { std::unordered_map functions_; std::unordered_map modules_; std::vector> children_; + std::unique_ptr ownedTable_; // root only }; +// The lexical Scope recorded for `node` when `anyScopeInTree`'s tree was +// built -- the replacement for the ASTNode::scope() field that used to +// live in the node. Any scope in the tree will do; it walks up to the root +// that owns the table. +inline const Scope* scopeOf(const Scope& anyScopeInTree, const ASTNode& node) { + const ScopeTable* table = anyScopeInTree.rootTable(); + return table ? table->get(node) : nullptr; +} + } // namespace oscad diff --git a/include/openscad_cpp_parser/scope_table.hpp b/include/openscad_cpp_parser/scope_table.hpp new file mode 100644 index 0000000..de3e8c3 --- /dev/null +++ b/include/openscad_cpp_parser/scope_table.hpp @@ -0,0 +1,58 @@ +#pragma once + +#include "openscad_cpp_parser/ast/ast_node.hpp" + +#include +#include + +namespace oscad { + +class Scope; + +// Where a node's Scope lives, now that it cannot live in the node. +// +// A parsed tree is shared: `include ` is parsed once and +// then handed to every script that includes it, which is what makes a +// render cheap (parsing BOSL2 is ~55ms and dwarfs everything else). But +// `include` means "share my scope", so the very same BOSL2 node sits in a +// different scope in every script that includes it. A `Scope*` field in +// ASTNode could only ever hold one of them, and whichever render wrote it +// last would corrupt the others. +// +// So the pointer moves out here, into a table one render owns. Addressing +// is (treeId, slot), both stamped on the node when it was parsed, so a +// lookup is two loads and no hashing -- it sits on the VM's call path. +class ScopeTable { +public: + // Null until set: buildScopes() fills in every node it walks, and a + // reader treats null as "no scope recorded", exactly as the old + // nullptr-initialised field did. + const Scope* get(const ASTNode& node) const { + const uint32_t tree = node.treeId(); + if (tree >= trees_.size()) return nullptr; + const std::vector& slots = trees_[tree]; + const uint32_t slot = node.slot(); + return slot < slots.size() ? slots[slot] : nullptr; + } + Scope* get(const ASTNode& node) { + return const_cast(static_cast(this)->get(node)); + } + + void set(const ASTNode& node, Scope* scope) { + const uint32_t tree = node.treeId(); + if (tree >= trees_.size()) trees_.resize(tree + 1); + std::vector& slots = trees_[tree]; + const uint32_t slot = node.slot(); + if (slot >= slots.size()) slots.resize(slot + 1, nullptr); + slots[slot] = scope; + } + +private: + // Indexed by treeId. Sparse in principle -- treeIds are handed out + // process-wide and a render only involves a few trees -- but the empty + // outer entries are 24 bytes each and never allocate, so the waste is + // a few tens of KB against a 55ms saving. + std::vector> trees_; +}; + +} // namespace oscad diff --git a/src/api.cpp b/src/api.cpp index ab59389..645760d 100644 --- a/src/api.cpp +++ b/src/api.cpp @@ -9,6 +9,10 @@ #include #include #include +#include +#include +#include +#include #include namespace oscad { @@ -86,6 +90,11 @@ std::string formatSyntaxError(const ParserDriver& driver, const std::string& cod } // namespace std::vector> parseAst(const std::string& code, const std::string& origin, SourceMap* sourceMap) { + // Stamps every node this parse builds with one treeId and a dense + // slot, so a ScopeTable can address it without the node carrying a + // Scope pointer of its own -- see ASTNode::slot(). + ParseNumberingScope numbering; + ParserDriver driver(origin); lexerBeginString(code); yy::parser parser(driver); @@ -100,6 +109,9 @@ std::vector> parseAst(const std::string& code, const st std::vector> getASTFromString(const std::string& code, bool includeComments, const std::string& origin) { + // Spans attachComments too: it builds CommentedExpr wrappers, and they + // belong to the same tree as what they wrap. + ParseNumberingScope numbering; auto ast = parseAst(code, origin); // propagates ParseError with the full diagnostic if (includeComments) { ast = attachComments(std::move(ast), code, origin); @@ -121,6 +133,7 @@ std::string readFile(const std::string& path) { std::vector> parseSingleFile(const std::string& filePath, bool includeComments) { std::string code = readFile(filePath); + ParseNumberingScope numbering; // spans attachComments -- see getASTFromString auto ast = parseAst(code, filePath); if (includeComments) { ast = attachComments(std::move(ast), code, filePath); @@ -238,8 +251,115 @@ LibraryFileResult getASTFromLibraryFile(const std::string& currFile, const std:: return LibraryFileResult{std::move(ast), *found}; } + + +namespace { + +using FileAst = std::vector>; +using FileAstPtr = std::shared_ptr; + +// One entry per (file, comments) -- keyed by PATH, with the content stamp +// stored beside the tree rather than in the key. A stale stamp REPLACES the +// entry instead of adding a second one: an editor re-renders on every save, +// and a stamp-in-the-key cache would keep a full copy of every version the +// file ever had. +struct CacheKey { + std::string path; + bool comments; + bool operator==(const CacheKey& o) const { return comments == o.comments && path == o.path; } +}; +struct CacheKeyHash { + size_t operator()(const CacheKey& k) const { + return std::hash{}(k.path) ^ (k.comments ? 0x5bf03635U : 0U); + } +}; +struct CacheEntry { + std::uintmax_t size = 0; + std::int64_t mtime = 0; + FileAstPtr ast; +}; + +std::mutex g_astCacheMutex; +std::unordered_map g_astCache; + +FileAstPtr parseFileShared(const std::string& absPath, bool includeComments) { + std::error_code ec; + const auto size = fs::file_size(absPath, ec); + const std::uintmax_t stampSize = ec ? 0 : size; + ec.clear(); + const auto written = fs::last_write_time(absPath, ec); + const std::int64_t stampMtime = ec ? 0 : static_cast(written.time_since_epoch().count()); + + const CacheKey key{absPath, includeComments}; + { + std::lock_guard lock(g_astCacheMutex); + auto it = g_astCache.find(key); + if (it != g_astCache.end() && it->second.size == stampSize && it->second.mtime == stampMtime) + return it->second.ast; + } + + // Parsed OUTSIDE the lock: parsing a library takes tens of + // milliseconds, and holding a global lock across it would serialise + // every thread. Two threads racing the same file both parse and one + // result is dropped -- wasteful once, never wrong, and far cheaper than + // the alternative. + auto parsed = std::make_shared(parseSingleFile(absPath, includeComments)); + std::lock_guard lock(g_astCacheMutex); + CacheEntry& entry = g_astCache[key]; + // Whoever writes last wins; the loser's tree stays alive in whatever + // ParsedProgram already borrowed it. + entry.size = stampSize; + entry.mtime = stampMtime; + entry.ast = parsed; + return parsed; +} + +// Splices includes into a flat statement list of BORROWED nodes, collecting +// what must stay alive. Mirrors resolveIncludes' walk exactly, including +// the per-resolution `visited` set that makes a file contribute at most +// once. +void collectProgram(const FileAst& nodes, const std::string& currentFile, bool includeComments, + std::set& visited, ParsedProgram& out) { + for (const auto& node : nodes) { + if (node->kind() == NodeKind::IncludeStatement) { + const auto& inc = static_cast(*node); + const std::string& filename = inc.filepath->val; + auto libFile = findLibraryFile(currentFile, filename); + if (!libFile) { + throw std::runtime_error("Included file '" + filename + "' not found. Searched relative to: " + + (currentFile.empty() ? "current directory" : currentFile)); + } + std::string absLib = fs::absolute(*libFile).string(); + if (!visited.insert(absLib).second) continue; + FileAstPtr included = parseFileShared(absLib, includeComments); + out.keepAlive.push_back(included); + collectProgram(*included, absLib, includeComments, visited, out); + } else { + out.nodes.push_back(node.get()); + } + } +} + +} // namespace + +ParsedProgram getProgramFromFile(const std::string& file, bool includeComments) { + const std::string abs = fs::absolute(file).string(); + ParsedProgram out; + FileAstPtr own = parseFileShared(abs, includeComments); + out.keepAlive.push_back(own); + std::set visited{abs}; + collectProgram(*own, abs, includeComments, visited, out); + return out; +} + void clearAstCache() { - // No-op -- see the ponytail note on getASTFromFile() in api.hpp. + std::lock_guard lock(g_astCacheMutex); + g_astCache.clear(); +} + +size_t astCacheSize() { + std::lock_guard lock(g_astCacheMutex); + return g_astCache.size(); } } // namespace oscad diff --git a/src/scope.cpp b/src/scope.cpp index 4ed38ed..2b669e8 100644 --- a/src/scope.cpp +++ b/src/scope.cpp @@ -1,25 +1,75 @@ #include "openscad_cpp_parser/api.hpp" +#include + +#include "openscad_cpp_parser/scope_table.hpp" + #include "openscad_cpp_parser/ast/scope_builder.hpp" namespace oscad { std::unique_ptr buildScopes(const std::vector>& ast) { + auto owned = std::make_unique(); + ScopeTableScope recording(*owned); auto root = std::make_unique(); collectHoistedDeclarations(ast, *root); for (auto& node : ast) { node->buildScope(*root); } + root->adoptTable(std::move(owned)); return root; } std::unique_ptr buildScopes(const std::vector& ast) { + auto owned = std::make_unique(); + ScopeTableScope recording(*owned); auto root = std::make_unique(); collectHoistedDeclarations(ast, *root); for (ASTNode* node : ast) { node->buildScope(*root); } + root->adoptTable(std::move(owned)); return root; } + +namespace { +// One per thread: a ParseNumberingScope installs a numbering for its +// duration, so a node's constructor can stamp itself without every node +// kind having to cooperate -- which matters because there is no generic +// child walker to number a finished tree with. +thread_local NodeNumbering* g_numbering = nullptr; +thread_local ScopeTable* g_scopeTable = nullptr; +std::atomic g_nextTreeId{1}; // 0 is reserved for loose nodes +std::atomic g_nextLooseSlot{0}; +} // namespace + +NodeNumbering* currentNodeNumbering() { return g_numbering; } +uint32_t nextLooseSlot() { return g_nextLooseSlot.fetch_add(1, std::memory_order_relaxed); } + +ParseNumberingScope::ParseNumberingScope() + : numbering_{g_nextTreeId.fetch_add(1, std::memory_order_relaxed), 0}, + previous_(g_numbering), + installed_(g_numbering == nullptr) { + // Only the outermost scope installs: a nested one keeps the outer + // numbering so a parse plus its comment-attach pass stay one tree. + if (installed_) g_numbering = &numbering_; +} + +ParseNumberingScope::~ParseNumberingScope() { + if (installed_) g_numbering = previous_; +} + +ScopeTableScope::ScopeTableScope(ScopeTable& table) : previous_(g_scopeTable) { g_scopeTable = &table; } +ScopeTableScope::~ScopeTableScope() { g_scopeTable = previous_; } + +void Scope::adoptTable(std::unique_ptr table) { ownedTable_ = std::move(table); } + +void ASTNode::setScope(Scope& s) { + // No table installed means nobody is recording scopes -- a parser-only + // caller walking a tree for its own reasons. Dropping the write keeps + // that a no-op rather than a crash. + if (g_scopeTable) g_scopeTable->set(*this, &s); +} + } // namespace oscad diff --git a/tests/test_const_borrow.cpp b/tests/test_const_borrow.cpp index ea2acff..4f601aa 100644 --- a/tests/test_const_borrow.cpp +++ b/tests/test_const_borrow.cpp @@ -12,9 +12,9 @@ namespace { // that only ever sees `const ASTNode&`/`const Scope*` must still be able to // read scope/lookup info, and the types it gets back must themselves be // const -- so there's no back door to mutation through a "read-only" borrow. -const ASTNode* readOnlyLookup(const ASTNode& borrowed, const std::string& name) { - const Scope* s = borrowed.scope(); - static_assert(std::is_same_v, "scope() through a const ASTNode& must yield const Scope*"); +const ASTNode* readOnlyLookup(const ASTNode& borrowed, const Scope& tree, const std::string& name) { + const Scope* s = scopeOf(tree, borrowed); + static_assert(std::is_same_v, "scopeOf() must yield const Scope*"); if (!s) { return nullptr; } @@ -29,8 +29,14 @@ const ASTNode* readOnlyLookup(const ASTNode& borrowed, const std::string& name) TEST(ConstBorrow, ScopeAccessorsAreConstCorrect) { // Non-const access still yields mutable pointers (existing behavior, // e.g. buildScopes()/collectHoistedDeclarations() need to mutate). - static_assert(std::is_same_v().scope()), Scope*>); - static_assert(std::is_same_v().scope()), const Scope*>); + // A node no longer holds its own Scope -- it lives in the tree's + // ScopeTable (see oscad::ScopeTable), and reading it through a const + // borrow must still hand back const. + static_assert(std::is_same_v().get(std::declval())), Scope*>); + static_assert( + std::is_same_v().get(std::declval())), const Scope*>); + static_assert(std::is_same_v(), std::declval())), + const Scope*>); static_assert(std::is_same_v().lookupVariable("x")), ASTNode*>); static_assert(std::is_same_v().lookupVariable("x")), const ASTNode*>); static_assert(std::is_same_v().lookupFunction("x")), ASTNode*>); @@ -50,7 +56,7 @@ TEST(ConstBorrow, ReadOnlyLookupWorksThroughConstReferences) { ASSERT_NE(func, nullptr); const Expression& borrowedDefault = *func->parameters[0]->defaultValue; // const borrow, as a worker thread would see it - const ASTNode* resolved = readOnlyLookup(borrowedDefault, "y"); + const ASTNode* resolved = readOnlyLookup(borrowedDefault, *root, "y"); ASSERT_NE(resolved, nullptr); EXPECT_EQ(resolved->kind(), NodeKind::Assignment); } @@ -59,11 +65,11 @@ TEST(ConstBorrow, ReadOnlyLookupWorksThroughConstReferences) { // const borrow is available would need a "must fail to compile" test // harness this project doesn't have, so it's asserted here in prose // instead -- `borrowedDefault.buildScope(*root)` and -// `borrowedDefault.scope()->defineVariable(...)` both fail to compile if +// `scopeOf(*root, borrowedDefault)->defineVariable(...)` both fail to compile if // uncommented (try it by hand if in doubt), since scope() on a const // object now yields `const Scope*`. Like all const-correctness in C++, // this is a static convention, not a runtime guarantee: an explicit -// `const_cast(*borrowedDefault.scope()).defineVariable(...)` still +// `const_cast(*scopeOf(*root, borrowedDefault)).defineVariable(...)` still // compiles and is well-defined (the underlying Scope was never actually // const, only const-accessed) -- the fix makes accidental mutation a // compile error, not a determined bypass impossible. diff --git a/tests/test_scope.cpp b/tests/test_scope.cpp index d051695..a13fcd4 100644 --- a/tests/test_scope.cpp +++ b/tests/test_scope.cpp @@ -100,7 +100,7 @@ TEST(ScopeBuilderBasics, SimpleAssignment) { TEST(ScopeBuilderBasics, AssignmentScopeAttached) { auto ast = parseSrc("x = 42;"); auto scope = buildScopes(ast); - EXPECT_EQ(ast[0]->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *ast[0]), scope.get()); } TEST(ScopeBuilderBasics, MultipleAssignments) { @@ -121,8 +121,8 @@ TEST(ScopeBuilderBasics, RawPointerOverloadCombinesTwoOwningVectors) { auto scope = buildScopes(combined); EXPECT_EQ(scope->lookupVariable("x"), astA[0].get()); EXPECT_EQ(scope->lookupVariable("y"), astB[0].get()); - EXPECT_EQ(astA[0]->scope(), scope.get()); - EXPECT_EQ(astB[0]->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *astA[0]), scope.get()); + EXPECT_EQ(scopeOf(*scope, *astB[0]), scope.get()); } // -- Function scope ----------------------------------------------------- @@ -138,7 +138,7 @@ TEST(FunctionScopeTest, ParametersInFunctionScope) { auto scope = buildScopes(ast); auto* func = dynamic_cast(ast[0].get()); ASSERT_NE(func, nullptr); - Scope* bodyScope = func->expr->scope(); + const Scope* bodyScope = scopeOf(*scope, *func->expr); ASSERT_NE(bodyScope, nullptr); EXPECT_NE(bodyScope->parent(), nullptr); EXPECT_NE(bodyScope->lookupVariable("a"), nullptr); @@ -150,7 +150,7 @@ TEST(FunctionScopeTest, SeesOuterVars) { auto scope = buildScopes(ast); auto* func = dynamic_cast(ast[1].get()); ASSERT_NE(func, nullptr); - EXPECT_NE(func->expr->scope()->lookupVariable("x"), nullptr); + EXPECT_NE(scopeOf(*scope, *func->expr)->lookupVariable("x"), nullptr); } TEST(FunctionScopeTest, ParameterWithDefault) { @@ -168,13 +168,13 @@ TEST(FunctionScopeTest, ParameterDefaultVisitedInCallerScope) { auto* func = dynamic_cast(ast[1].get()); ASSERT_NE(func, nullptr); auto& param = func->parameters[0]; - Scope* defaultScope = param->defaultValue->scope(); + const Scope* defaultScope = scopeOf(*scope, *param->defaultValue); ASSERT_NE(defaultScope, nullptr); EXPECT_NE(defaultScope->lookupVariable("x"), nullptr); // Strengthened beyond the Python original: confirm the default's scope // really is the caller scope, not the function's own body scope (which // would additionally see `a`). - EXPECT_NE(defaultScope, func->expr->scope()); + EXPECT_NE(defaultScope, scopeOf(*scope, *func->expr)); EXPECT_EQ(defaultScope->lookupVariable("a"), nullptr); } @@ -191,7 +191,7 @@ TEST(ModuleScopeTest, ParametersInModuleScope) { auto scope = buildScopes(ast); auto* mod = dynamic_cast(ast[0].get()); ASSERT_NE(mod, nullptr); - Scope* childScope = mod->children[0]->scope(); + const Scope* childScope = scopeOf(*scope, *mod->children[0]); ASSERT_NE(childScope, nullptr); EXPECT_NE(childScope->lookupVariable("size"), nullptr); } @@ -208,7 +208,7 @@ TEST(ModuleScopeTest, NestedFunctionInModule) { auto scope = buildScopes(ast); auto* mod = dynamic_cast(ast[0].get()); ASSERT_NE(mod, nullptr); - Scope* bodyScope = mod->children[0]->scope(); + const Scope* bodyScope = scopeOf(*scope, *mod->children[0]); ASSERT_NE(bodyScope, nullptr); EXPECT_NE(bodyScope->lookupFunction("helper"), nullptr); } @@ -221,7 +221,7 @@ TEST(HoistingTest, AssignmentHoistedInModule) { auto* mod = dynamic_cast(ast[0].get()); ASSERT_NE(mod, nullptr); ASTNode* callNode = mod->children[0].get(); - EXPECT_NE(callNode->scope()->lookupVariable("val"), nullptr); + EXPECT_NE(scopeOf(*scope, *callNode)->lookupVariable("val"), nullptr); } // -- Let expressions ------------------------------------------------ @@ -231,7 +231,7 @@ TEST(LetExpressionsTest, LetOpCreatesScope) { auto scope = buildScopes(ast); auto* letNode = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(letNode, nullptr); - Scope* bodyScope = letNode->body->scope(); + const Scope* bodyScope = scopeOf(*scope, *letNode->body); ASSERT_NE(bodyScope, nullptr); EXPECT_NE(bodyScope->lookupVariable("a"), nullptr); EXPECT_NE(bodyScope->lookupVariable("b"), nullptr); @@ -252,7 +252,7 @@ TEST(ModularConstructsTest, ForCreatesScope) { auto* forNode = dynamic_cast(ast[0].get()); ASSERT_NE(forNode, nullptr); ASSERT_FALSE(forNode->body.empty()); - EXPECT_NE(forNode->body[0]->scope()->lookupVariable("i"), nullptr); + EXPECT_NE(scopeOf(*scope, *forNode->body[0])->lookupVariable("i"), nullptr); EXPECT_EQ(scope->lookupVariable("i"), nullptr); } @@ -262,7 +262,7 @@ TEST(ModularConstructsTest, IfCreatesScope) { auto* ifNode = dynamic_cast(ast[0].get()); ASSERT_NE(ifNode, nullptr); ASSERT_FALSE(ifNode->trueBranch.empty()); - EXPECT_EQ(ifNode->trueBranch[0]->scope()->parent(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *ifNode->trueBranch[0])->parent(), scope.get()); } TEST(ModularConstructsTest, LetCreatesScope) { @@ -271,7 +271,7 @@ TEST(ModularConstructsTest, LetCreatesScope) { auto* letNode = dynamic_cast(ast[0].get()); ASSERT_NE(letNode, nullptr); ASSERT_FALSE(letNode->children.empty()); - EXPECT_NE(letNode->children[0]->scope()->lookupVariable("x"), nullptr); + EXPECT_NE(scopeOf(*scope, *letNode->children[0])->lookupVariable("x"), nullptr); } TEST(ModularConstructsTest, IfSingleBranch) { @@ -279,7 +279,7 @@ TEST(ModularConstructsTest, IfSingleBranch) { auto scope = buildScopes(ast); auto* ifNode = dynamic_cast(ast[0].get()); ASSERT_NE(ifNode, nullptr); - EXPECT_NE(ifNode->trueBranch[0]->scope(), nullptr); + EXPECT_NE(scopeOf(*scope, *ifNode->trueBranch[0]), nullptr); } TEST(ModularConstructsTest, IfElseSingleBranches) { @@ -287,8 +287,8 @@ TEST(ModularConstructsTest, IfElseSingleBranches) { auto scope = buildScopes(ast); auto* ieNode = dynamic_cast(ast[0].get()); ASSERT_NE(ieNode, nullptr); - Scope* trueScope = ieNode->trueBranch[0]->scope(); - Scope* falseScope = ieNode->falseBranch[0]->scope(); + const Scope* trueScope = scopeOf(*scope, *ieNode->trueBranch[0]); + const Scope* falseScope = scopeOf(*scope, *ieNode->falseBranch[0]); EXPECT_NE(trueScope, nullptr); EXPECT_NE(falseScope, nullptr); EXPECT_NE(trueScope, falseScope); @@ -300,7 +300,7 @@ TEST(ModularConstructsTest, ForListBody) { auto* forNode = dynamic_cast(ast[0].get()); ASSERT_NE(forNode, nullptr); for (auto& child : forNode->body) { - EXPECT_NE(child->scope()->lookupVariable("i"), nullptr); + EXPECT_NE(scopeOf(*scope, *child)->lookupVariable("i"), nullptr); } } @@ -309,9 +309,9 @@ TEST(ModularConstructsTest, EchoWithChildren) { auto scope = buildScopes(ast); auto* echoNode = dynamic_cast(ast[0].get()); ASSERT_NE(echoNode, nullptr); - EXPECT_EQ(echoNode->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *echoNode), scope.get()); for (auto& child : echoNode->children) { - EXPECT_NE(child->scope(), nullptr); + EXPECT_NE(scopeOf(*scope, *child), nullptr); } } @@ -320,13 +320,13 @@ TEST(ModularConstructsTest, AssertWithChildren) { auto scope = buildScopes(ast); auto* assertNode = dynamic_cast(ast[0].get()); ASSERT_NE(assertNode, nullptr); - EXPECT_EQ(assertNode->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *assertNode), scope.get()); } TEST(ModularConstructsTest, CallEmptyChildren) { auto ast = parseSrc("cube(1);"); auto scope = buildScopes(ast); - EXPECT_EQ(ast[0]->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *ast[0]), scope.get()); } TEST(ModularConstructsTest, ModifierShowOnly) { @@ -334,29 +334,29 @@ TEST(ModularConstructsTest, ModifierShowOnly) { auto scope = buildScopes(ast); auto* mod = dynamic_cast(ast[0].get()); ASSERT_NE(mod, nullptr); - EXPECT_EQ(mod->scope(), scope.get()); - EXPECT_NE(mod->child->scope(), nullptr); + EXPECT_EQ(scopeOf(*scope, *mod), scope.get()); + EXPECT_NE(scopeOf(*scope, *mod->child), nullptr); } TEST(ModularConstructsTest, ModifierHighlight) { auto ast = parseSrc("# cube(1);"); auto scope = buildScopes(ast); auto* mod = dynamic_cast(ast[0].get()); ASSERT_NE(mod, nullptr); - EXPECT_EQ(mod->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *mod), scope.get()); } TEST(ModularConstructsTest, ModifierBackground) { auto ast = parseSrc("% cube(1);"); auto scope = buildScopes(ast); auto* mod = dynamic_cast(ast[0].get()); ASSERT_NE(mod, nullptr); - EXPECT_EQ(mod->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *mod), scope.get()); } TEST(ModularConstructsTest, ModifierDisable) { auto ast = parseSrc("* cube(1);"); auto scope = buildScopes(ast); auto* mod = dynamic_cast(ast[0].get()); ASSERT_NE(mod, nullptr); - EXPECT_EQ(mod->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *mod), scope.get()); } // -- FunctionLiteral recursion -------------------------------------- @@ -366,7 +366,7 @@ TEST(FunctionLiteralRecursionTest, SeesAssignedVariable) { auto scope = buildScopes(ast); auto* funcLit = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(funcLit, nullptr); - EXPECT_NE(funcLit->body->scope()->lookupVariable("x"), nullptr); + EXPECT_NE(scopeOf(*scope, *funcLit->body)->lookupVariable("x"), nullptr); } TEST(FunctionLiteralRecursionTest, WithDefaultParameter) { @@ -375,7 +375,7 @@ TEST(FunctionLiteralRecursionTest, WithDefaultParameter) { auto* funcLit = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(funcLit, nullptr); EXPECT_NE(funcLit->parameters[0]->defaultValue, nullptr); - EXPECT_NE(funcLit->body->scope()->lookupVariable("x"), nullptr); + EXPECT_NE(scopeOf(*scope, *funcLit->body)->lookupVariable("x"), nullptr); } TEST(FunctionLiteralRecursionTest, InExpression) { @@ -385,7 +385,7 @@ TEST(FunctionLiteralRecursionTest, InExpression) { ASSERT_NE(lc, nullptr); auto* funcLit = dynamic_cast(lc->elements[1].get()); ASSERT_NE(funcLit, nullptr); - EXPECT_NE(funcLit->body->scope()->lookupVariable("a"), nullptr); + EXPECT_NE(scopeOf(*scope, *funcLit->body)->lookupVariable("a"), nullptr); } TEST(FunctionLiteralRecursionTest, InTernaryRhs) { @@ -395,7 +395,7 @@ TEST(FunctionLiteralRecursionTest, InTernaryRhs) { ASSERT_NE(ternary, nullptr); auto* funcLit = dynamic_cast(ternary->falseExpr.get()); ASSERT_NE(funcLit, nullptr); - EXPECT_NE(funcLit->body->scope()->lookupVariable("a"), nullptr); + EXPECT_NE(scopeOf(*scope, *funcLit->body)->lookupVariable("a"), nullptr); } // -- ModularCall children ------------------------------------------- @@ -407,9 +407,9 @@ TEST(ModularCallChildrenTest, WithNamedArgument) { ASSERT_NE(call, nullptr); auto* named = dynamic_cast(call->arguments[0].get()); ASSERT_NE(named, nullptr); - EXPECT_EQ(named->scope(), scope.get()); - EXPECT_EQ(named->name->scope(), scope.get()); - EXPECT_EQ(named->expr->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *named), scope.get()); + EXPECT_EQ(scopeOf(*scope, *named->name), scope.get()); + EXPECT_EQ(scopeOf(*scope, *named->expr), scope.get()); } TEST(ModularCallChildrenTest, PrimaryCallNamedArgumentVisitsName) { @@ -419,7 +419,7 @@ TEST(ModularCallChildrenTest, PrimaryCallNamedArgumentVisitsName) { ASSERT_NE(pcall, nullptr); auto* named = dynamic_cast(pcall->arguments[0].get()); ASSERT_NE(named, nullptr); - EXPECT_EQ(named->name->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *named->name), scope.get()); } TEST(ModularCallChildrenTest, CallChildrenScope) { @@ -428,8 +428,8 @@ TEST(ModularCallChildrenTest, CallChildrenScope) { auto* call = dynamic_cast(ast[0].get()); ASSERT_NE(call, nullptr); for (auto& child : call->children) { - ASSERT_NE(child->scope(), nullptr); - EXPECT_EQ(child->scope()->parent(), scope.get()); + ASSERT_NE(scopeOf(*scope, *child), nullptr); + EXPECT_EQ(scopeOf(*scope, *child)->parent(), scope.get()); } } @@ -440,10 +440,10 @@ TEST(ScopeLookupTest, AncestorScopeFunctionAndModule) { auto scope = buildScopes(ast); auto* func = dynamic_cast(ast[1].get()); ASSERT_NE(func, nullptr); - EXPECT_NE(func->expr->scope()->lookupVariable("x"), nullptr); + EXPECT_NE(scopeOf(*scope, *func->expr)->lookupVariable("x"), nullptr); auto* mod = dynamic_cast(ast[2].get()); ASSERT_NE(mod, nullptr); - EXPECT_NE(mod->children[0]->scope()->lookupVariable("x"), nullptr); + EXPECT_NE(scopeOf(*scope, *mod->children[0])->lookupVariable("x"), nullptr); } TEST(ScopeLookupTest, LookupInParent) { @@ -459,10 +459,10 @@ TEST(ScopeLookupTest, Shadowing) { auto scope = buildScopes(ast); auto* func = dynamic_cast(ast[1].get()); ASSERT_NE(func, nullptr); - Scope* bodyScope = func->expr->scope(); - ASTNode* found = bodyScope->lookupVariable("x"); + const Scope* bodyScope = scopeOf(*scope, *func->expr); + const ASTNode* found = bodyScope->lookupVariable("x"); ASSERT_NE(found, nullptr); - EXPECT_NE(dynamic_cast(found), nullptr); + EXPECT_NE(dynamic_cast(found), nullptr); EXPECT_NE(found, ast[0].get()); } @@ -494,7 +494,7 @@ TEST(ListComprehensionScopeTest, ForScope) { ASSERT_NE(lc, nullptr); auto* forElem = dynamic_cast(lc->elements[0].get()); ASSERT_NE(forElem, nullptr); - EXPECT_NE(forElem->body->scope()->lookupVariable("i"), nullptr); + EXPECT_NE(scopeOf(*scope, *forElem->body)->lookupVariable("i"), nullptr); EXPECT_EQ(scope->lookupVariable("i"), nullptr); } @@ -505,7 +505,7 @@ TEST(ListComprehensionScopeTest, CForScope) { ASSERT_NE(lc, nullptr); auto* cforElem = dynamic_cast(lc->elements[0].get()); ASSERT_NE(cforElem, nullptr); - EXPECT_NE(cforElem->body->scope()->lookupVariable("i"), nullptr); + EXPECT_NE(scopeOf(*scope, *cforElem->body)->lookupVariable("i"), nullptr); // Not checked by the Python original, but symmetric with the plain-for // case and worth locking down: c-style-for's loop var must not leak. EXPECT_EQ(scope->lookupVariable("i"), nullptr); @@ -518,7 +518,7 @@ TEST(ListComprehensionScopeTest, LetScope) { ASSERT_NE(lc, nullptr); auto* letElem = dynamic_cast(lc->elements[0].get()); ASSERT_NE(letElem, nullptr); - EXPECT_NE(letElem->body->scope()->lookupVariable("a"), nullptr); + EXPECT_NE(scopeOf(*scope, *letElem->body)->lookupVariable("a"), nullptr); } TEST(ListComprehensionScopeTest, IfScope) { @@ -530,8 +530,8 @@ TEST(ListComprehensionScopeTest, IfScope) { ASSERT_NE(forElem, nullptr); auto* ifElem = dynamic_cast(forElem->body.get()); ASSERT_NE(ifElem, nullptr); - EXPECT_NE(ifElem->scope(), nullptr); - EXPECT_NE(ifElem->trueExpr->scope(), nullptr); + EXPECT_NE(scopeOf(*scope, *ifElem), nullptr); + EXPECT_NE(scopeOf(*scope, *ifElem->trueExpr), nullptr); } TEST(ListComprehensionScopeTest, IfElseScope) { @@ -543,15 +543,15 @@ TEST(ListComprehensionScopeTest, IfElseScope) { ASSERT_NE(forElem, nullptr); auto* ifElseElem = dynamic_cast(forElem->body.get()); ASSERT_NE(ifElseElem, nullptr); - EXPECT_NE(ifElseElem->trueExpr->scope(), nullptr); - EXPECT_NE(ifElseElem->falseExpr->scope(), nullptr); + EXPECT_NE(scopeOf(*scope, *ifElseElem->trueExpr), nullptr); + EXPECT_NE(scopeOf(*scope, *ifElseElem->falseExpr), nullptr); // Unlike ModularIfElse, ListCompIfElse's branches are plain // expressions (not statement blocks that could contain hoistable // declarations), so build_scope legitimately does NOT create a new // scope per branch here -- both share parent_scope directly. Matches // the reference exactly; do not "strengthen" this into a distinctness // check (an earlier version of this test incorrectly did). - EXPECT_EQ(ifElseElem->trueExpr->scope(), ifElseElem->falseExpr->scope()); + EXPECT_EQ(scopeOf(*scope, *ifElseElem->trueExpr), scopeOf(*scope, *ifElseElem->falseExpr)); } TEST(ListComprehensionScopeTest, EachScope) { @@ -561,7 +561,7 @@ TEST(ListComprehensionScopeTest, EachScope) { ASSERT_NE(lc, nullptr); auto* eachElem = dynamic_cast(lc->elements[0].get()); ASSERT_NE(eachElem, nullptr); - EXPECT_NE(eachElem->scope(), nullptr); + EXPECT_NE(scopeOf(*scope, *eachElem), nullptr); } // -- Expression-operator build_scope sweep ------------------------------- @@ -574,145 +574,145 @@ TEST(ExpressionOpBuildScope, EchoOp) { auto scope = buildScopes(ast); auto* echo = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(echo, nullptr); - EXPECT_EQ(echo->scope(), scope.get()); - EXPECT_EQ(echo->body->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *echo), scope.get()); + EXPECT_EQ(scopeOf(*scope, *echo->body), scope.get()); } TEST(ExpressionOpBuildScope, AssertOp) { auto ast = parseSrc("x = assert(true) 1;"); auto scope = buildScopes(ast); auto* a = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(a, nullptr); - EXPECT_EQ(a->scope(), scope.get()); - EXPECT_EQ(a->body->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *a), scope.get()); + EXPECT_EQ(scopeOf(*scope, *a->body), scope.get()); } TEST(ExpressionOpBuildScope, DivisionOp) { auto ast = parseSrc("x = 10 / 2;"); auto scope = buildScopes(ast); auto* d = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(d, nullptr); - EXPECT_EQ(d->scope(), scope.get()); - EXPECT_EQ(d->left->scope(), scope.get()); - EXPECT_EQ(d->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *d), scope.get()); + EXPECT_EQ(scopeOf(*scope, *d->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *d->right), scope.get()); } TEST(ExpressionOpBuildScope, ModuloOp) { auto ast = parseSrc("x = 10 % 3;"); auto scope = buildScopes(ast); auto* m = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(m, nullptr); - EXPECT_EQ(m->left->scope(), scope.get()); - EXPECT_EQ(m->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *m->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *m->right), scope.get()); } TEST(ExpressionOpBuildScope, ExponentOp) { auto ast = parseSrc("x = 2 ^ 3;"); auto scope = buildScopes(ast); auto* e = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(e, nullptr); - EXPECT_EQ(e->left->scope(), scope.get()); - EXPECT_EQ(e->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *e->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *e->right), scope.get()); } TEST(ExpressionOpBuildScope, BitwiseAndOp) { auto ast = parseSrc("x = 5 & 3;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->left->scope(), scope.get()); - EXPECT_EQ(op->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->right), scope.get()); } TEST(ExpressionOpBuildScope, BitwiseOrOp) { auto ast = parseSrc("x = 5 | 3;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->left->scope(), scope.get()); - EXPECT_EQ(op->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->right), scope.get()); } TEST(ExpressionOpBuildScope, BitwiseNotOp) { auto ast = parseSrc("x = ~5;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->expr->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->expr), scope.get()); } TEST(ExpressionOpBuildScope, BitwiseShiftLeftOp) { auto ast = parseSrc("x = 1 << 4;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->left->scope(), scope.get()); - EXPECT_EQ(op->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->right), scope.get()); } TEST(ExpressionOpBuildScope, BitwiseShiftRightOp) { auto ast = parseSrc("x = 16 >> 2;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->left->scope(), scope.get()); - EXPECT_EQ(op->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->right), scope.get()); } TEST(ExpressionOpBuildScope, LogicalAndOp) { auto ast = parseSrc("x = true && false;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->left->scope(), scope.get()); - EXPECT_EQ(op->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->right), scope.get()); } TEST(ExpressionOpBuildScope, LogicalOrOp) { auto ast = parseSrc("x = true || false;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->left->scope(), scope.get()); - EXPECT_EQ(op->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->right), scope.get()); } TEST(ExpressionOpBuildScope, LogicalNotOp) { auto ast = parseSrc("x = !true;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->expr->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->expr), scope.get()); } TEST(ExpressionOpBuildScope, InequalityOp) { auto ast = parseSrc("x = 1 != 2;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->left->scope(), scope.get()); - EXPECT_EQ(op->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->right), scope.get()); } TEST(ExpressionOpBuildScope, GreaterThanOrEqualOp) { auto ast = parseSrc("x = 1 >= 2;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->left->scope(), scope.get()); - EXPECT_EQ(op->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->right), scope.get()); } TEST(ExpressionOpBuildScope, LessThanOrEqualOp) { auto ast = parseSrc("x = 1 <= 2;"); auto scope = buildScopes(ast); auto* op = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(op, nullptr); - EXPECT_EQ(op->left->scope(), scope.get()); - EXPECT_EQ(op->right->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *op->right), scope.get()); } TEST(ExpressionOpBuildScope, PrimaryIndex) { auto ast = parseSrc("x = v[0];"); auto scope = buildScopes(ast); auto* idx = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(idx, nullptr); - EXPECT_EQ(idx->scope(), scope.get()); - EXPECT_EQ(idx->left->scope(), scope.get()); - EXPECT_EQ(idx->index->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *idx), scope.get()); + EXPECT_EQ(scopeOf(*scope, *idx->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *idx->index), scope.get()); } TEST(ExpressionOpBuildScope, PrimaryMember) { auto ast = parseSrc("x = v.x;"); auto scope = buildScopes(ast); auto* mem = dynamic_cast(dynamic_cast(ast[0].get())->expr.get()); ASSERT_NE(mem, nullptr); - EXPECT_EQ(mem->scope(), scope.get()); - EXPECT_EQ(mem->left->scope(), scope.get()); - EXPECT_EQ(mem->member->scope(), scope.get()); + EXPECT_EQ(scopeOf(*scope, *mem), scope.get()); + EXPECT_EQ(scopeOf(*scope, *mem->left), scope.get()); + EXPECT_EQ(scopeOf(*scope, *mem->member), scope.get()); } // -- intersection_for build_scope ---------------------------------------- @@ -723,7 +723,7 @@ TEST(IntersectionForBuildScope, Scope) { auto* ifor = dynamic_cast(ast[0].get()); ASSERT_NE(ifor, nullptr); ASSERT_FALSE(ifor->body.empty()); - EXPECT_NE(ifor->body[0]->scope()->lookupVariable("i"), nullptr); + EXPECT_NE(scopeOf(*scope, *ifor->body[0])->lookupVariable("i"), nullptr); EXPECT_EQ(scope->lookupVariable("i"), nullptr); } @@ -733,7 +733,7 @@ TEST(IntersectionForBuildScope, BlockBody) { auto* ifor = dynamic_cast(ast[0].get()); ASSERT_NE(ifor, nullptr); for (auto& child : ifor->body) { - EXPECT_NE(child->scope()->lookupVariable("i"), nullptr); + EXPECT_NE(scopeOf(*scope, *child)->lookupVariable("i"), nullptr); } } @@ -746,7 +746,7 @@ TEST(HoistedModuleDeclarationTest, NestedModuleIsHoisted) { ASSERT_NE(outer, nullptr); auto* callNode = dynamic_cast(outer->children[0].get()); ASSERT_NE(callNode, nullptr); - EXPECT_NE(callNode->scope()->lookupModule("inner"), nullptr); + EXPECT_NE(scopeOf(*scope, *callNode)->lookupModule("inner"), nullptr); } TEST(HoistedModuleDeclarationTest, NestedModuleNotVisibleInOuterScope) { diff --git a/tests/test_smoke.cpp b/tests/test_smoke.cpp index b0d5d06..0fcb283 100644 --- a/tests/test_smoke.cpp +++ b/tests/test_smoke.cpp @@ -69,8 +69,8 @@ TEST(Smoke, ParameterDefaultResolvesInCallerScope) { ASSERT_EQ(func->parameters.size(), 1u); auto* defaultExpr = func->parameters[0]->defaultValue.get(); ASSERT_NE(defaultExpr, nullptr); - ASSERT_NE(defaultExpr->scope(), nullptr); - EXPECT_NE(defaultExpr->scope()->lookupVariable("y"), nullptr); + ASSERT_NE(scopeOf(*root, *defaultExpr), nullptr); + EXPECT_NE(scopeOf(*root, *defaultExpr)->lookupVariable("y"), nullptr); } TEST(Smoke, LetOpSequentialSelfReferentialBinding) { @@ -84,8 +84,8 @@ TEST(Smoke, LetOpSequentialSelfReferentialBinding) { // y's RHS (x + 1) should resolve `x` to the first let-assignment. auto* yRhs = dynamic_cast(let->assignments[1]->expr.get()); ASSERT_NE(yRhs, nullptr); - ASSERT_NE(yRhs->scope(), nullptr); - EXPECT_EQ(yRhs->scope()->lookupVariable("x"), let->assignments[0].get()); + ASSERT_NE(scopeOf(*root, *yRhs), nullptr); + EXPECT_EQ(scopeOf(*root, *yRhs)->lookupVariable("x"), let->assignments[0].get()); } TEST(Smoke, ModularIfElseBranchesAreIndependentScopes) { @@ -93,8 +93,8 @@ TEST(Smoke, ModularIfElseBranchesAreIndependentScopes) { auto root = buildScopes(ast); auto* ifElse = dynamic_cast(ast[0].get()); ASSERT_NE(ifElse, nullptr); - Scope* trueScope = ifElse->trueBranch[0]->scope(); - Scope* falseScope = ifElse->falseBranch[0]->scope(); + const Scope* trueScope = scopeOf(*root, *ifElse->trueBranch[0]); + const Scope* falseScope = scopeOf(*root, *ifElse->falseBranch[0]); ASSERT_NE(trueScope, nullptr); ASSERT_NE(falseScope, nullptr); EXPECT_NE(trueScope, falseScope);