Stop reserving builtin module names as lexer keywords (v3.5 final item)

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 ✓
 

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<std::string_view, TokenKind> 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},

src/lang/Parser.cpp

@@ -2,9 +2,45 @@
 #include <cassert>
 #include <cmath>
 #include <stdexcept>
+#include <unordered_map>
 
 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<std::string_view, TokenKind> 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,66 @@ 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:
+                    // 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.
             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:

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, // <path/to/file.scad> — 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