From 6e9d24ccf78ede32957882e28a0865b33ac4cb7f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 01:55:46 +0000 Subject: [PATCH 1/2] Stop reserving builtin module names as lexer keywords (v3.5 final item) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cube/sphere/translate/scale/etc. no longer occupy their own TokenKind — the Lexer's keyword table now holds only genuine OpenSCAD grammar keywords (if/else/for/each/module/function/let/include/use/undef/true/ false). The Parser recognises builtin constructs by name only at statement-start call position (new kBuiltinNodeNames table), so a script is free to use e.g. `cube`, `scale`, or `rotate` as an ordinary variable, module/function parameter, or for/let-loop variable — matching real OpenSCAD, where builtins and variables live in separate namespaces. This was the one item explicitly deferred from the v3.5 completeness pass and closes out feature completeness with OpenSCAD's builtin/ keyword surface. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UAoehmbCynYJC5K28qBVxs --- docs/roadmap.md | 19 ++++--- src/lang/Lexer.cpp | 29 +++------- src/lang/Parser.cpp | 124 ++++++++++++++++++++++++++++-------------- src/lang/Token.h | 30 ++++++---- tests/test_lexer.cpp | 88 ++++++++++++++++-------------- tests/test_parser.cpp | 73 +++++++++++++++++++++++++ 6 files changed, 239 insertions(+), 124 deletions(-) diff --git a/docs/roadmap.md b/docs/roadmap.md index ed0f34b..1ba1eca 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -93,14 +93,17 @@ cover. Fixed: default-constructed `Interpreter`) falls back to OpenSCAD's own defaults (`$vpr=[55,0,25]`, `$vpt=[0,0,0]`, `$vpd=140`) instead of `undef`. No roll in this camera model, so `$vpr[1]` is always 0. - -**Explicitly deferred, not done in this pass** (tracked here so they aren't -lost, not because they don't matter): -- `cube`/`sphere`/`translate`/etc. are reserved lexer keywords rather than - ordinary identifiers, so (unlike real OpenSCAD) a script can't use one of - those names as a variable. Real-world impact is low, but it's a genuine - parser-level deviation and any fix is a grammar-level change, not a - point fix — out of scope here. +- [x] `cube`/`sphere`/`translate`/etc. are no longer reserved lexer + keywords — matching real OpenSCAD, they're ordinary identifiers that the + Parser recognises by name only at statement-start call position + (`kBuiltinNodeNames` in `Parser.cpp`). The Lexer's keyword table now + holds only genuine grammar keywords (`if`/`else`/`for`/`each`/`module`/ + `function`/`let`/`include`/`use`/`undef`/`true`/`false`); builtins and + variables live in separate namespaces, so `cube = 5;` followed later by + `cube(cube);`, a module parameter named `scale`, or a `for`/`let` + variable named `rotate` all now parse correctly. This was the last + gap deferred from the initial v3.5 pass, closing out feature completeness + with OpenSCAD's builtin/keyword surface. ## v3.6 — First-class function literals ✓ diff --git a/src/lang/Lexer.cpp b/src/lang/Lexer.cpp index 8db348c..27b58f2 100644 --- a/src/lang/Lexer.cpp +++ b/src/lang/Lexer.cpp @@ -10,36 +10,21 @@ namespace chisel::lang { // --------------------------------------------------------------------------- // Keyword table — maps source text to TokenKind // --------------------------------------------------------------------------- +// Only genuine OpenSCAD grammar keywords go here. Builtin module/function +// names (cube, sphere, translate, union, offset, ...) are deliberately NOT +// reserved — matching real OpenSCAD, they lex as plain Ident and are +// recognised by name at the Parser level (see kBuiltinNodeNames in +// Parser.cpp). That's what lets a script use `cube`, `scale`, etc. as an +// ordinary variable/parameter name; the two live in separate namespaces +// (variable/function vs. module), same as upstream OpenSCAD. static const std::unordered_map kKeywords = { {"true", TokenKind::True}, {"false", TokenKind::False}, - {"cube", TokenKind::Cube}, - {"sphere", TokenKind::Sphere}, - {"cylinder", TokenKind::Cylinder}, - {"union", TokenKind::Union}, - {"difference", TokenKind::Difference}, - {"intersection", TokenKind::Intersection}, - {"hull", TokenKind::Hull}, - {"minkowski", TokenKind::Minkowski}, - {"translate", TokenKind::Translate}, - {"rotate", TokenKind::Rotate}, - {"scale", TokenKind::Scale}, - {"mirror", TokenKind::Mirror}, - {"multmatrix", TokenKind::Multmatrix}, - {"render", TokenKind::Render}, - {"color", TokenKind::Color}, {"if", TokenKind::If}, {"else", TokenKind::Else}, {"for", TokenKind::For}, {"each", TokenKind::Each}, {"module", TokenKind::Module}, - {"square", TokenKind::Square}, - {"circle", TokenKind::Circle}, - {"polygon", TokenKind::Polygon}, - {"linear_extrude", TokenKind::LinearExtrude}, - {"rotate_extrude", TokenKind::RotateExtrude}, - {"offset", TokenKind::Offset}, - {"projection", TokenKind::Projection}, {"undef", TokenKind::Undef}, {"function", TokenKind::Function}, {"let", TokenKind::Let}, diff --git a/src/lang/Parser.cpp b/src/lang/Parser.cpp index 4bd7969..9ce51da 100644 --- a/src/lang/Parser.cpp +++ b/src/lang/Parser.cpp @@ -2,9 +2,45 @@ #include #include #include +#include namespace chisel::lang { +// Builtin module/function names (primitives, booleans, transforms, extrusion +// ops, ...) are NOT reserved lexer keywords — the Lexer always emits Ident +// for them, so a script is free to use `cube`, `scale`, etc. as an ordinary +// variable/parameter name, matching real OpenSCAD (where variables/functions +// and modules live in separate namespaces). This table is how the Parser +// still recognises `cube(...)`, `translate(...) { ... }`, etc. by name at +// statement-start position: the TokenKind values below are reused purely as +// an internal "which builtin is this" tag passed to the existing +// parsePrimitive/parseBoolean/parseTransform/parseExtrusion helpers — they +// are never produced by the Lexer itself anymore. +static const std::unordered_map kBuiltinNodeNames = { + {"cube", TokenKind::Cube}, + {"sphere", TokenKind::Sphere}, + {"cylinder", TokenKind::Cylinder}, + {"union", TokenKind::Union}, + {"difference", TokenKind::Difference}, + {"intersection", TokenKind::Intersection}, + {"hull", TokenKind::Hull}, + {"minkowski", TokenKind::Minkowski}, + {"translate", TokenKind::Translate}, + {"rotate", TokenKind::Rotate}, + {"scale", TokenKind::Scale}, + {"mirror", TokenKind::Mirror}, + {"multmatrix", TokenKind::Multmatrix}, + {"render", TokenKind::Render}, + {"color", TokenKind::Color}, + {"square", TokenKind::Square}, + {"circle", TokenKind::Circle}, + {"polygon", TokenKind::Polygon}, + {"linear_extrude", TokenKind::LinearExtrude}, + {"rotate_extrude", TokenKind::RotateExtrude}, + {"offset", TokenKind::Offset}, + {"projection", TokenKind::Projection}, +}; + // A token can be used as a named-parameter name (e.g. `scale=`) if it's an // identifier or a keyword scanned as one (both carry non-empty `.text`). // Number/String literals also carry non-empty `.text` but must NOT be @@ -276,44 +312,6 @@ AstNodePtr Parser::parseNodeInner() { TokenKind k = peek().kind; switch (k) { - case TokenKind::Cube: - case TokenKind::Sphere: - case TokenKind::Cylinder: - case TokenKind::Square: - case TokenKind::Circle: - case TokenKind::Polygon: - return parsePrimitive(k); - - case TokenKind::LinearExtrude: - case TokenKind::RotateExtrude: - return parseExtrusion(k); - - case TokenKind::Offset: - return parseOffset(); - - case TokenKind::Projection: - return parseProjection(); - - case TokenKind::Union: - case TokenKind::Difference: - case TokenKind::Intersection: - case TokenKind::Hull: - case TokenKind::Minkowski: - return parseBoolean(k); - - case TokenKind::Translate: - case TokenKind::Rotate: - case TokenKind::Scale: - case TokenKind::Mirror: - case TokenKind::Multmatrix: - return parseTransform(k); - - case TokenKind::Render: - return parseRender(); - - case TokenKind::Color: - return parseColor(); - case TokenKind::If: return parseIf(); @@ -323,15 +321,59 @@ AstNodePtr Parser::parseNodeInner() { case TokenKind::Let: return parseLetNode(); - case TokenKind::Ident: - // Could be a module call: name(args) { ... } - if (peek(1).kind == TokenKind::LParen) + case TokenKind::Ident: { + // Builtin construct called by name: cube(...), translate(...) {...}, + // etc. These names aren't reserved keywords (see kBuiltinNodeNames + // above), so a same-named variable is legal elsewhere — but at + // statement-start position followed by '(', OpenSCAD always resolves + // the name as the builtin module, never as a variable reference. + if (peek(1).kind == TokenKind::LParen) { + auto it = kBuiltinNodeNames.find(peek().text); + if (it != kBuiltinNodeNames.end()) { + switch (it->second) { + case TokenKind::Cube: + case TokenKind::Sphere: + case TokenKind::Cylinder: + case TokenKind::Square: + case TokenKind::Circle: + case TokenKind::Polygon: + return parsePrimitive(it->second); + case TokenKind::LinearExtrude: + case TokenKind::RotateExtrude: + return parseExtrusion(it->second); + case TokenKind::Offset: + return parseOffset(); + case TokenKind::Projection: + return parseProjection(); + case TokenKind::Union: + case TokenKind::Difference: + case TokenKind::Intersection: + case TokenKind::Hull: + case TokenKind::Minkowski: + return parseBoolean(it->second); + case TokenKind::Translate: + case TokenKind::Rotate: + case TokenKind::Scale: + case TokenKind::Mirror: + case TokenKind::Multmatrix: + return parseTransform(it->second); + case TokenKind::Render: + return parseRender(); + case TokenKind::Color: + return parseColor(); + default: + break; // unreachable — every kBuiltinNodeNames value is handled above + } + } + // Not a builtin name: a user-defined module call. return parseModuleCall(); + } // Local variable assignment: name = expr; — valid as a statement in // any block (module/for/if/... body), not just at file scope. if (peek(1).kind == TokenKind::Equals) return parseAssignNode(); return nullptr; + } case TokenKind::Include: case TokenKind::Use: diff --git a/src/lang/Token.h b/src/lang/Token.h index 39230bd..75b9702 100644 --- a/src/lang/Token.h +++ b/src/lang/Token.h @@ -33,22 +33,30 @@ enum class TokenKind : uint8_t { False, // false // Identifiers / special variables - Ident, // any unrecognised identifier + Ident, // any unrecognised identifier — also every builtin module/ + // function name below (Cube, Translate, ...): unlike real + // keywords, the Lexer never emits those kinds. They exist + // purely as internal tags the Parser assigns by name lookup + // (see kBuiltinNodeNames in Parser.cpp) when it recognises a + // builtin construct at statement-start position, so that a + // script remains free to use e.g. `cube` or `scale` as an + // ordinary variable/parameter name — matching real OpenSCAD, + // where builtins and variables live in separate namespaces. SpecialVar, // $fn $fs $fa - // Primitives + // Primitives (see Ident above — not a Lexer-level keyword) Cube, Sphere, Cylinder, - // Booleans + // Booleans (see Ident above — not a Lexer-level keyword) Union, Difference, Intersection, Hull, Minkowski, - // Transforms + // Transforms (see Ident above — not a Lexer-level keyword) Translate, Rotate, Scale, @@ -57,7 +65,7 @@ enum class TokenKind : uint8_t { Render, Color, - // Control flow / definitions + // Control flow / definitions — genuine reserved keywords If, // if Else, // else For, // for @@ -66,24 +74,24 @@ enum class TokenKind : uint8_t { Function, // function Let, // let - // File inclusion + // File inclusion — genuine reserved keywords Include, // include Use, // use AngledPath, // — raw text scanned right after Include/Use // Literals - Undef, // undef + Undef, // undef — genuine reserved keyword - // 2-D primitives + // 2-D primitives (see Ident above — not a Lexer-level keyword) Square, Circle, Polygon, - // Extrusion operations + // Extrusion operations (see Ident above — not a Lexer-level keyword) LinearExtrude, RotateExtrude, - // 2-D → 2-D operations + // 2-D → 2-D operations (see Ident above — not a Lexer-level keyword) Offset, - // 3-D → 2-D operations + // 3-D → 2-D operations (see Ident above — not a Lexer-level keyword) Projection, // Range separator diff --git a/tests/test_lexer.cpp b/tests/test_lexer.cpp index 41215e4..21d8c1f 100644 --- a/tests/test_lexer.cpp +++ b/tests/test_lexer.cpp @@ -30,26 +30,30 @@ static std::vector kinds(std::string_view src) { } // --------------------------------------------------------------------------- -// Primitives +// Primitives — builtin module names are NOT reserved lexer keywords (unlike +// real keywords such as `if`/`module`), so they lex as plain Ident. The +// Parser recognises them by name at statement-start position instead (see +// kBuiltinNodeNames in Parser.cpp / Parser:builtin names usable as variables +// in test_parser.cpp). // --------------------------------------------------------------------------- -TEST_CASE("Lexer:primitive keywords", "[lexer]") { - REQUIRE(kinds("cube") == std::vector{TokenKind::Cube}); - REQUIRE(kinds("sphere") == std::vector{TokenKind::Sphere}); - REQUIRE(kinds("cylinder") == std::vector{TokenKind::Cylinder}); +TEST_CASE("Lexer:primitive names lex as plain identifiers", "[lexer]") { + REQUIRE(kinds("cube") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("sphere") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("cylinder") == std::vector{TokenKind::Ident}); } // --------------------------------------------------------------------------- -// Boolean keywords +// Boolean op names — also plain identifiers, not reserved keywords // --------------------------------------------------------------------------- -TEST_CASE("Lexer:boolean keywords", "[lexer]") { - REQUIRE(kinds("union") == std::vector{TokenKind::Union}); - REQUIRE(kinds("difference") == std::vector{TokenKind::Difference}); - REQUIRE(kinds("intersection") == std::vector{TokenKind::Intersection}); +TEST_CASE("Lexer:boolean op names lex as plain identifiers", "[lexer]") { + REQUIRE(kinds("union") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("difference") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("intersection") == std::vector{TokenKind::Ident}); } -TEST_CASE("Lexer:hull and minkowski keywords", "[lexer]") { - REQUIRE(kinds("hull") == std::vector{TokenKind::Hull}); - REQUIRE(kinds("minkowski") == std::vector{TokenKind::Minkowski}); +TEST_CASE("Lexer:hull and minkowski names lex as plain identifiers", "[lexer]") { + REQUIRE(kinds("hull") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("minkowski") == std::vector{TokenKind::Ident}); } TEST_CASE("Lexer:if and else keywords", "[lexer]") { @@ -88,13 +92,13 @@ TEST_CASE("Lexer:for range tokens", "[lexer]") { } // --------------------------------------------------------------------------- -// Transform keywords +// Transform names — also plain identifiers, not reserved keywords // --------------------------------------------------------------------------- -TEST_CASE("Lexer:transform keywords", "[lexer]") { - REQUIRE(kinds("translate") == std::vector{TokenKind::Translate}); - REQUIRE(kinds("rotate") == std::vector{TokenKind::Rotate}); - REQUIRE(kinds("scale") == std::vector{TokenKind::Scale}); - REQUIRE(kinds("mirror") == std::vector{TokenKind::Mirror}); +TEST_CASE("Lexer:transform names lex as plain identifiers", "[lexer]") { + REQUIRE(kinds("translate") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("rotate") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("scale") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("mirror") == std::vector{TokenKind::Ident}); } // --------------------------------------------------------------------------- @@ -235,12 +239,12 @@ TEST_CASE("Lexer:punctuation", "[lexer]") { // Comments // --------------------------------------------------------------------------- TEST_CASE("Lexer:line comment skipped", "[lexer]") { - REQUIRE(kinds("// this is a comment\ncube").back() == TokenKind::Cube); - REQUIRE(kinds("cube // trailing comment") == std::vector{TokenKind::Cube}); + REQUIRE(kinds("// this is a comment\ncube").back() == TokenKind::Ident); + REQUIRE(kinds("cube // trailing comment") == std::vector{TokenKind::Ident}); } TEST_CASE("Lexer:block comment skipped", "[lexer]") { - REQUIRE(kinds("/* comment */ cube") == std::vector{TokenKind::Cube}); + REQUIRE(kinds("/* comment */ cube") == std::vector{TokenKind::Ident}); // "cu" and "be" are separate idents — the comment is stripped mid-token REQUIRE(kinds("cu/* mid */be") == (std::vector{TokenKind::Ident, TokenKind::Ident})); } @@ -283,7 +287,7 @@ TEST_CASE("Lexer:source location column tracking", "[lexer]") { TEST_CASE("Lexer:translate + cube call", "[lexer]") { // translate([0, 0, 0]) cube([10, 10, 10]); auto t = lex("translate([0, 0, 0])\n cube([10, 10, 10]);"); - REQUIRE(t[0].kind == TokenKind::Translate); + REQUIRE(t[0].kind == TokenKind::Ident); REQUIRE(t[1].kind == TokenKind::LParen); REQUIRE(t[2].kind == TokenKind::LBracket); REQUIRE(t[3].kind == TokenKind::Number); @@ -293,7 +297,7 @@ TEST_CASE("Lexer:translate + cube call", "[lexer]") { TEST_CASE("Lexer:difference block header", "[lexer]") { // difference() { REQUIRE(kinds("difference() {") == std::vector{ - TokenKind::Difference, + TokenKind::Ident, TokenKind::LParen, TokenKind::RParen, TokenKind::LBrace, @@ -303,7 +307,7 @@ TEST_CASE("Lexer:difference block header", "[lexer]") { TEST_CASE("Lexer:cylinder with named params", "[lexer]") { // cylinder(h = 10, r = 5, center = true) auto t = lex("cylinder(h = 10, r = 5, center = true)"); - REQUIRE(t[0].kind == TokenKind::Cylinder); + REQUIRE(t[0].kind == TokenKind::Ident); REQUIRE(t[2].kind == TokenKind::Ident); // h REQUIRE(t[2].text == "h"); REQUIRE(t[3].kind == TokenKind::Equals); @@ -315,7 +319,7 @@ TEST_CASE("Lexer:$fn per-primitive override", "[lexer]") { // sphere(r = 5, $fn = 8) // indices: 0=sphere 1=( 2=r 3== 4=5 5=, 6=$fn 7== 8=8 9=) auto t = lex("sphere(r = 5, $fn = 8)"); - REQUIRE(t[0].kind == TokenKind::Sphere); + REQUIRE(t[0].kind == TokenKind::Ident); REQUIRE(t[6].kind == TokenKind::SpecialVar); REQUIRE(t[6].text == "$fn"); REQUIRE(t[8].numberValue() == 8.0); @@ -324,7 +328,7 @@ TEST_CASE("Lexer:$fn per-primitive override", "[lexer]") { TEST_CASE("Lexer:empty union (edge case from test file)", "[lexer]") { // union() {}; REQUIRE(kinds("union() {};") == std::vector{ - TokenKind::Union, + TokenKind::Ident, TokenKind::LParen, TokenKind::RParen, TokenKind::LBrace, @@ -340,22 +344,22 @@ TEST_CASE("Lexer:Eof token is always last", "[lexer]") { } // --------------------------------------------------------------------------- -// 2-D primitives and extrusion keywords (Tier 4) +// 2-D primitives and extrusion names (Tier 4) — also plain identifiers // --------------------------------------------------------------------------- -TEST_CASE("Lexer:2D primitive keywords", "[lexer]") { - REQUIRE(kinds("square") == std::vector{TokenKind::Square}); - REQUIRE(kinds("circle") == std::vector{TokenKind::Circle}); - REQUIRE(kinds("polygon") == std::vector{TokenKind::Polygon}); +TEST_CASE("Lexer:2D primitive names lex as plain identifiers", "[lexer]") { + REQUIRE(kinds("square") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("circle") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("polygon") == std::vector{TokenKind::Ident}); } -TEST_CASE("Lexer:extrusion keywords", "[lexer]") { - REQUIRE(kinds("linear_extrude") == std::vector{TokenKind::LinearExtrude}); - REQUIRE(kinds("rotate_extrude") == std::vector{TokenKind::RotateExtrude}); +TEST_CASE("Lexer:extrusion names lex as plain identifiers", "[lexer]") { + REQUIRE(kinds("linear_extrude") == std::vector{TokenKind::Ident}); + REQUIRE(kinds("rotate_extrude") == std::vector{TokenKind::Ident}); } -TEST_CASE("Lexer:linear_extrude with underscores tokenizes as single keyword", "[lexer]") { +TEST_CASE("Lexer:linear_extrude with underscores tokenizes as a single identifier", "[lexer]") { auto t = lex("linear_extrude(height=10)"); - REQUIRE(t[0].kind == TokenKind::LinearExtrude); + REQUIRE(t[0].kind == TokenKind::Ident); REQUIRE(t[0].text == "linear_extrude"); } @@ -457,7 +461,7 @@ TEST_CASE("Lexer:hash tokenizes as its own kind", "[lexer]") { REQUIRE(kinds("#") == std::vector{TokenKind::Hash}); REQUIRE(kinds("#cube();") == std::vector{ TokenKind::Hash, - TokenKind::Cube, + TokenKind::Ident, TokenKind::LParen, TokenKind::RParen, TokenKind::Semicolon, @@ -469,21 +473,21 @@ TEST_CASE("Lexer:percent/star/bang keep their existing operator token kinds", "[ // modifiers only when found in statement-start position. REQUIRE(kinds("%cube();") == std::vector{ TokenKind::Percent, - TokenKind::Cube, + TokenKind::Ident, TokenKind::LParen, TokenKind::RParen, TokenKind::Semicolon, }); REQUIRE(kinds("*cube();") == std::vector{ TokenKind::Star, - TokenKind::Cube, + TokenKind::Ident, TokenKind::LParen, TokenKind::RParen, TokenKind::Semicolon, }); REQUIRE(kinds("!cube();") == std::vector{ TokenKind::Bang, - TokenKind::Cube, + TokenKind::Ident, TokenKind::LParen, TokenKind::RParen, TokenKind::Semicolon, @@ -494,7 +498,7 @@ TEST_CASE("Lexer:stacked modifier characters", "[lexer]") { REQUIRE(kinds("#!cube();") == std::vector{ TokenKind::Hash, TokenKind::Bang, - TokenKind::Cube, + TokenKind::Ident, TokenKind::LParen, TokenKind::RParen, TokenKind::Semicolon, diff --git a/tests/test_parser.cpp b/tests/test_parser.cpp index e769e23..05b88c5 100644 --- a/tests/test_parser.cpp +++ b/tests/test_parser.cpp @@ -1026,3 +1026,76 @@ TEST_CASE("Parser:modifier before a nested (block-scoped) assignment is a parse REQUIRE(asAssign(u.children[0]).name == "x"); REQUIRE(asAssign(u.children[0]).modifiers == ModNone); // rejected, not silently tagged } + +// --------------------------------------------------------------------------- +// v3.5 deferred item: builtin module/function names (cube, translate, scale, +// ...) are not reserved lexer keywords, so a script can use one as an +// ordinary variable/parameter/loop-variable name — matching real OpenSCAD, +// where builtins and variables live in separate namespaces. +// --------------------------------------------------------------------------- +TEST_CASE("Parser:builtin primitive name usable as a top-level variable", "[parser]") { + auto r = parse("cube = 5;\nsphere(cube);"); + REQUIRE(r.assignments.size() == 1); + REQUIRE(r.assignments[0].name == "cube"); + REQUIRE(r.roots.size() == 1); + const auto& p = asPrim(r.roots[0]); + REQUIRE(p.kind == PrimitiveNode::Kind::Sphere); + + Interpreter interp; + interp.loadAssignments(r); + REQUIRE(interp.getVar("cube").asNumber() == Approx(5.0)); + REQUIRE(interp.evalNumber(*p.params.at("_pos0")) == Approx(5.0)); +} + +TEST_CASE("Parser:builtin transform name usable as a variable, still callable as a transform", "[parser]") { + // 'scale' is both a variable (assigned above) and a module invocation + // (translate(...)) below — separate namespaces, exactly like OpenSCAD. + auto r = parse("scale = 2;\ntranslate([scale, 0, 0]) cube(1);"); + REQUIRE(r.assignments.size() == 1); + REQUIRE(r.assignments[0].name == "scale"); + REQUIRE(r.roots.size() == 1); + const auto& t = asTrans(r.roots[0]); + REQUIRE(t.kind == TransformNode::Kind::Translate); + REQUIRE(t.children.size() == 1); + + Interpreter interp; + interp.loadAssignments(r); + const auto& vlit = std::get(*t.vec); + REQUIRE(interp.evalNumber(*vlit.elements[0].value) == Approx(2.0)); +} + +TEST_CASE("Parser:module parameter named after a builtin transform", "[parser]") { + auto r = parse("module box(scale) { cylinder(h=scale, r=1); }\nbox(3);"); + REQUIRE(r.moduleDefs.size() == 1); + REQUIRE(r.moduleDefs[0].params.size() == 1); + REQUIRE(r.moduleDefs[0].params[0].name == "scale"); + REQUIRE(r.roots.size() == 1); + auto& c = asModuleCall(r.roots[0]); + REQUIRE(c.name == "box"); +} + +TEST_CASE("Parser:for-loop variable named after a builtin primitive", "[parser]") { + auto r = parse("for (cube = [0:2]) { translate([cube, 0, 0]) sphere(1); }"); + REQUIRE(r.roots.size() == 1); + const auto& f = asFor(r.roots[0]); + REQUIRE(f.var == "cube"); +} + +TEST_CASE("Parser:let binding named after a builtin boolean op", "[parser]") { + auto r = parse("let(union = 4) sphere(union);"); + REQUIRE(r.roots.size() == 1); + const auto& letNode = std::get(*r.roots[0]); + REQUIRE(letNode.bindings.size() == 1); + REQUIRE(letNode.bindings[0].first == "union"); + REQUIRE(letNode.children.size() == 1); +} + +TEST_CASE("Parser:named module-call argument named after a builtin transform", "[parser]") { + // Previously 'scale=' here would fail to parse as a named argument + // because 'scale' lexed as TokenKind::Scale, not Ident. + auto r = parse("module box(scale) { cube(scale); }\nbox(scale = 4);"); + REQUIRE(r.roots.size() == 1); + auto& c = asModuleCall(r.roots[0]); + REQUIRE(c.args.size() == 1); + REQUIRE(c.args[0].name == "scale"); +} From 044c1cdd0992c59380c91eafd4c7fcc5f93ea596 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 06:36:06 +0000 Subject: [PATCH 2/2] Fail loudly instead of silently misparsing on kBuiltinNodeNames/switch divergence Per PR review: the previous default: break; in parseNodeInner's builtin dispatch switch would silently fall through to parseModuleCall() if a name were ever added to kBuiltinNodeNames without a matching case, misparsing a builtin call as a user-defined module call with no diagnostic. Replace with an assert so any future divergence fails fast during development instead of misbehaving silently. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UAoehmbCynYJC5K28qBVxs --- src/lang/Parser.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/lang/Parser.cpp b/src/lang/Parser.cpp index 9ce51da..d6d9978 100644 --- a/src/lang/Parser.cpp +++ b/src/lang/Parser.cpp @@ -362,7 +362,14 @@ AstNodePtr Parser::parseNodeInner() { case TokenKind::Color: return parseColor(); default: - break; // unreachable — every kBuiltinNodeNames value is handled above + // Every kBuiltinNodeNames value is handled above; reaching + // here means the map and this switch have diverged (a + // name was added to one but not the other). Fail loudly + // instead of silently falling through to parseModuleCall() + // below, which would misparse a builtin call as a + // user-defined module call with no diagnostic at all. + assert(false && "kBuiltinNodeNames entry with no matching parseNodeInner case"); + break; } } // Not a builtin name: a user-defined module call.