Add first-class function literals: function(params) expr (v3.6)

docs/roadmap.md

@@ -87,12 +87,6 @@ cover. Fixed:
 
 **Explicitly deferred, not done in this pass** (tracked here so they aren't
 lost, not because they don't matter):
-- First-class function literals (`f = function(x) x*2;`, OpenSCAD 2019.05+)
-  as values assignable to variables/passable as arguments — the language
-  has named `function foo(x) = expr;` defs but no closure value type; adding
-  one touches the `Value` variant, the parser grammar, and call dispatch
-  broadly enough that it deserves its own pass rather than riding along with
-  smaller fixes.
 - `$vpr`/`$vpt`/`$vpd` viewport special variables — would need the render
   layer's camera state plumbed into the interpreter, a cross-subsystem wire
   that doesn't exist today; rarely load-bearing in real `.scad` files.
@@ -102,6 +96,19 @@ lost, not because they don't matter):
   parser-level deviation and any fix is a grammar-level change, not a
   point fix — out of scope here.
 
+## v3.6 — First-class function literals ✓
+
+- [x] Function literals (`f = function(x) x*2;`, OpenSCAD 2019.05+) as
+  values: assignable to variables, storable in lists, passable as
+  arguments to (and returnable from) other functions, and callable later
+  via ordinary `f(...)` syntax wherever `f` names a variable holding one.
+  Adds a `Value::Tag::Function` closure (AST pointer + captured defining
+  scope) alongside the existing named `function foo(x) = expr;` form.
+  Direct self-binding (`fact = function(n) n<=1 ? 1 : n*fact(n-1);`)
+  recurses by name without needing a named `function` def.
+- [x] `is_function()` now recognizes actual function values (was a
+  stubbed-`false` placeholder pending this work).
+
 ## v4 — Tooling & Visual Quality
 
 - [ ] VS Code LSP extension (syntax highlighting, error squiggles, completions)

src/csg/CsgEvaluator.cpp

@@ -154,7 +154,7 @@ CsgNodePtr CsgEvaluator::evalNode(const AstNode& node, const glm::mat4& xform,
                 // scope in place (the enclosing evalXxx call is responsible for
                 // snapshot/restore around its whole child list, so this doesn't
                 // leak past the block it's written in). Produces no geometry.
-                m_interp->setVar(n.name, m_interp->evaluate(*n.value));
+                m_interp->assignVar(n.name, *n.value);
                 return nullptr;
             }
             return nullptr;
@@ -634,6 +634,18 @@ std::string CsgEvaluator::formatValue(const Value& v) {
                formatValue(Value::fromNumber(v.rangeStep)) + ":" +
                formatValue(Value::fromNumber(v.rangeEnd)) + "]";
     }
+    if (v.isFunction()) {
+        std::string s = "function(";
+        if (v.closure && v.closure->def) {
+            const auto& params = v.closure->def->params;
+            for (std::size_t i = 0; i < params.size(); ++i) {
+                if (i > 0) s += ", ";
+                s += params[i].name;
+            }
+        }
+        s += ")";
+        return s;
+    }
     return "undef";
 }
 
@@ -1461,7 +1473,7 @@ CsgNodePtr CsgEvaluator::evalLet(const LetNode& node, const glm::mat4& xform,
                                  const ColorAttr& color) {
     auto savedEnv = m_interp->snapshotEnv();
     for (const auto& [name, valExpr] : node.bindings)
-        m_interp->setVar(name, m_interp->evaluate(*valExpr));
+        m_interp->assignVar(name, *valExpr);
 
     std::vector<CsgNodePtr> all;
     for (const auto& child : node.children) {

src/lang/Expr.h

@@ -25,10 +25,11 @@ struct LetExpr;
 struct FunctionCall;
 struct RangeLit;
 struct ListCompExpr;
+struct FunctionLit;
 
 using ExprNode = std::variant<NumberLit, BoolLit, UndefLit, StringLit, VectorLit, VarRef,
                                BinaryExpr, UnaryExpr, TernaryExpr, IndexExpr,
-                               LetExpr, FunctionCall, RangeLit, ListCompExpr>;
+                               LetExpr, FunctionCall, RangeLit, ListCompExpr, FunctionLit>;
 using ExprPtr  = std::unique_ptr<ExprNode>;
 
 template<typename T>
@@ -179,4 +180,25 @@ struct ListCompExpr {
     SourceLoc       loc;
 };
 
+// ---------------------------------------------------------------------------
+// Function literal — `function(params) expr`, OpenSCAD 2019.05+.
+//
+// Unlike FunctionDef (AST.h) — a named `function foo(x) = expr;` declaration
+// dispatched by name through FunctionCall — a FunctionLit is itself a
+// first-class expression: it evaluates to a closure Value (see Value::Tag::
+// Function) that can be assigned to a variable, stored in a list, passed as
+// an argument, and returned from another function, then invoked later via
+// ordinary call syntax (`f(...)`) wherever `f` names a variable holding one.
+// ---------------------------------------------------------------------------
+struct FunctionLitParam {
+    std::string name;
+    ExprPtr     defaultVal; // nullptr = no default (required)
+};
+
+struct FunctionLit {
+    std::vector<FunctionLitParam> params;
+    ExprPtr                       body;
+    SourceLoc                     loc;
+};
+
 } // namespace chisel::lang

src/lang/Interpreter.cpp

@@ -27,6 +27,8 @@ static bool valEqRec(const Value& a, const Value& b) {
     }
     if (a.isRange())
         return a.rangeStart == b.rangeStart && a.rangeStep == b.rangeStep && a.rangeEnd == b.rangeEnd;
+    if (a.isFunction())
+        return a.closure == b.closure; // identity: same closure instance
     return a.isUndef(); // both Undef, since tags already matched
 }
 
@@ -35,7 +37,7 @@ static bool valEqRec(const Value& a, const Value& b) {
 // ---------------------------------------------------------------------------
 void Interpreter::loadAssignments(const ParseResult& result) {
     for (const auto& stmt : result.assignments)
-        m_env[stmt.name] = evaluate(*stmt.value);
+        assignVar(stmt.name, *stmt.value);
 }
 
 void Interpreter::loadFunctions(const ParseResult& result) {
@@ -217,7 +219,7 @@ Value Interpreter::evaluate(const ExprNode& expr) {
         else if constexpr (std::is_same_v<T, LetExpr>) {
             auto savedEnv = snapshotEnv();
             for (const auto& [name, valExpr] : node.bindings)
-                setVar(name, evaluate(*valExpr));
+                assignVar(name, *valExpr);
             Value result = evaluate(*node.body);
             restoreEnv(std::move(savedEnv));
             return result;
@@ -258,6 +260,14 @@ Value Interpreter::evaluate(const ExprNode& expr) {
             return Value::fromVec(std::move(out));
         }
 
+        // ---- Function literal: function(params) expr ----
+        else if constexpr (std::is_same_v<T, FunctionLit>) {
+            auto env  = std::make_shared<ClosureEnv>();
+            env->def  = &node;
+            env->vars = m_env; // capture the defining scope by value
+            return Value::fromClosure(std::move(env));
+        }
+
         // ---- Function call ----
         else if constexpr (std::is_same_v<T, FunctionCall>) {
             // Collect positional and named argument values
@@ -271,6 +281,15 @@ Value Interpreter::evaluate(const ExprNode& expr) {
                     namedArgs.push_back({arg.name, std::move(v)});
             }
 
+            // A variable bound to a function-literal value takes priority
+            // over a same-named `function` definition or builtin — mirrors
+            // OpenSCAD: once `f = function(x) x*2;` exists, `f(3)` calls
+            // that closure even if a builtin or `function f(...)` also
+            // exists under that name.
+            auto varIt = m_env.find(node.name);
+            if (varIt != m_env.end() && varIt->second.isFunction())
+                return callClosure(varIt->second, posArgs, namedArgs);
+
             // Try user-defined function first
             auto fit = m_funcDefs.find(node.name);
             if (fit != m_funcDefs.end()) {
@@ -415,6 +434,54 @@ void Interpreter::setVar(const std::string& name, Value val) {
     m_env[name] = std::move(val);
 }
 
+void Interpreter::assignVar(const std::string& name, const ExprNode& valueExpr) {
+    Value v = evaluate(valueExpr);
+    if (v.isFunction() && v.closure && std::holds_alternative<FunctionLit>(valueExpr))
+        v.closure->selfName = name; // let `name(...)` recurse from within its own body (see callClosure)
+    m_env[name] = std::move(v);
+}
+
+// ---------------------------------------------------------------------------
+// callClosure — invoke a Value::Tag::Function closure
+// ---------------------------------------------------------------------------
+Value Interpreter::callClosure(Value fnVal,
+                                const std::vector<Value>& posArgs,
+                                const std::vector<std::pair<std::string, Value>>& namedArgs) {
+    if (!fnVal.closure || !fnVal.closure->def || m_callDepth >= kMaxCallDepth)
+        return Value::undef();
+    const FunctionLit& def = *fnVal.closure->def;
+
+    auto savedEnv = snapshotEnv();
+    m_env = fnVal.closure->vars; // lexical scope: where the literal was written, not where it's called
+    // Rebind the closure's own name (if any) fresh for this call only — never
+    // stored back into fnVal.closure->vars itself, which would recreate the
+    // shared_ptr<ClosureEnv> cycle assignVar's comment warns about.
+    if (!fnVal.closure->selfName.empty())
+        m_env[fnVal.closure->selfName] = fnVal;
+
+    std::size_t posIdx = 0;
+    for (const auto& param : def.params) {
+        bool bound = false;
+        for (const auto& [n, v] : namedArgs) {
+            if (n == param.name) { setVar(param.name, v); bound = true; break; }
+        }
+        if (!bound) {
+            if (posIdx < posArgs.size())
+                setVar(param.name, posArgs[posIdx++]);
+            else if (param.defaultVal)
+                setVar(param.name, evaluate(*param.defaultVal));
+            else
+                setVar(param.name, Value::undef());
+        }
+    }
+
+    ++m_callDepth;
+    Value result = evaluate(*def.body);
+    --m_callDepth;
+    restoreEnv(std::move(savedEnv));
+    return result;
+}
+
 // ---------------------------------------------------------------------------
 // Built-in functions
 // ---------------------------------------------------------------------------
@@ -633,10 +700,7 @@ Value Interpreter::callBuiltin(const std::string& name,
     if (name == "is_num")    return args.size() >= 1 ? Value::fromBool(args[0].isNumber()) : Value::undef();
     if (name == "is_string") return args.size() >= 1 ? Value::fromBool(args[0].isString()) : Value::undef();
     if (name == "is_list")   return args.size() >= 1 ? Value::fromBool(args[0].isVector()) : Value::undef();
-    // No first-class function-literal value exists yet (see roadmap v3.5),
-    // so nothing can ever actually be one — always false rather than absent,
-    // which is still correct given today's set of possible Value tags.
-    if (name == "is_function") return args.size() >= 1 ? Value::fromBool(false) : Value::undef();
+    if (name == "is_function") return args.size() >= 1 ? Value::fromBool(args[0].isFunction()) : Value::undef();
 
     // ---- version() / version_num() ----
     // ChiselCAD isn't OpenSCAD, so there's no real "version" to report; this

src/lang/Interpreter.h

@@ -34,6 +34,26 @@ class Interpreter {
     Value       getVar(const std::string& name) const;
     void        setVar(const std::string& name, Value val);
 
+    // Binds `name` in the current scope to the evaluated result of valueExpr
+    // — used everywhere a real assignment happens (global `x = expr;`, local
+    // block assignments, `let(x = expr, ...)` bindings), as opposed to
+    // setVar's other callers (module/function parameter binding, for-loop
+    // variables) which must NOT get the special case below. When valueExpr
+    // is directly a function literal (`f = function(n) ... f(n-1) ...;`),
+    // this also records the closure's own name (ClosureEnv::selfName) so
+    // callClosure can bind it for the literal's body to call itself by that
+    // name — mirroring OpenSCAD's function-literal recursion. Restricting
+    // this to a literal directly on the right-hand side (checked via the AST
+    // node type, not the runtime Value) keeps it safe: the closure was just
+    // constructed by this same evaluate() call, so nothing else can be
+    // aliasing it yet. A plain copy (`g = f;`) does not re-trigger this —
+    // renaming a closure that's already shared with other bindings would
+    // rebind `name` in their calls too. Note this stores only a name string,
+    // not a Value referencing the closure itself: the latter would be a
+    // shared_ptr<ClosureEnv> cycle back into its own vars map, which
+    // shared_ptr can never collect.
+    void assignVar(const std::string& name, const ExprNode& valueExpr);
+
     // Expands a range's [start:step:end] bounds into the concrete sequence
     // of values it denotes — shared by for-loops (both the `for (v =
     // [a:b:c])` literal form and `for (v = expr)` when expr is a Range
@@ -98,6 +118,20 @@ class Interpreter {
     Value callBuiltin(const std::string& name,
                       const std::vector<Value>& args) const;
 
+    // Invokes a closure Value (see Value::Tag::Function): binds fnVal's
+    // params against posArgs/namedArgs the same way a named FunctionDef call
+    // does, but starting from the closure's *captured* environment rather
+    // than the caller's — a function literal sees the scope where
+    // `function(...) ...` was written, not the scope it's called from.
+    // Takes fnVal by value, not by reference: the caller's reference is
+    // typically an element inside m_env (e.g. a FunctionCall looks the
+    // callee up via m_env.find), and this function reassigns m_env wholesale
+    // as its first step (to switch to the closure's own captured scope) —
+    // a reference into the old map would dangle the moment that happens.
+    Value callClosure(Value fnVal,
+                       const std::vector<Value>& posArgs,
+                       const std::vector<std::pair<std::string, Value>>& namedArgs);
+
     // Appends v's per-iteration values (see iterationValues()) onto out —
     // used by `each` in both a plain list literal and a list-comprehension
     // body.

src/lang/Parser.cpp

@@ -784,6 +784,14 @@ ExprPtr Parser::parsePrimary() {
     if (check(TokenKind::Let)) {
         return parseLetExpr();
     }
+    // Function literal: function(params) expr — a first-class function value.
+    // Only reachable here (inside expression parsing); a bare `function` at
+    // statement level is instead a named `function foo(...) = expr;`
+    // definition, handled by parseFunctionDef before expression parsing ever
+    // starts.
+    if (check(TokenKind::Function)) {
+        return parseFunctionLit();
+    }
     // Parenthesised expression
     if (match(TokenKind::LParen)) {
         auto expr = parseExpr();
@@ -927,6 +935,32 @@ void Parser::parseFunctionDef(ParseResult& result) {
     result.functionDefs.push_back(std::move(def));
 }
 
+// ---------------------------------------------------------------------------
+// function literal — function(params) expr, usable as a general expression
+// (assigned to a variable, passed as an argument, returned from another
+// function). Same parameter-list grammar as parseFunctionDef, minus the name
+// and the `=`.
+// ---------------------------------------------------------------------------
+ExprPtr Parser::parseFunctionLit() {
+    const Token& kw = advance(); // consume 'function'
+    FunctionLit lit;
+    lit.loc = kw.loc;
+
+    expect(TokenKind::LParen, "expected '(' after 'function'");
+    while (!check(TokenKind::RParen) && !atEnd()) {
+        FunctionLitParam param;
+        param.name = expect(TokenKind::Ident, "expected parameter name").text;
+        if (match(TokenKind::Equals))
+            param.defaultVal = parseExpr();
+        lit.params.push_back(std::move(param));
+        if (!match(TokenKind::Comma)) break;
+    }
+    expect(TokenKind::RParen, "expected ')' after parameter list");
+    lit.body = parseExpr();
+
+    return makeExpr(std::move(lit));
+}
+
 // ---------------------------------------------------------------------------
 // let statement — let(x = expr, ...) { children }
 // ---------------------------------------------------------------------------

src/lang/Parser.h

@@ -63,6 +63,7 @@ class Parser {
     ExprPtr parsePostfix();     // handles postfix [idx] after primary
     ExprPtr parsePrimary();
     ExprPtr parseLetExpr();
+    ExprPtr parseFunctionLit();             // function(params) expr — a function-literal value
     VectorElem parseVectorElem();           // one list element: expr, or `each expr`
     ExprPtr parseListComp(SourceLoc loc);   // [for (var = source) body]
     ListCompBodyPtr parseListCompBody();    // body clause: expr / each expr / if (..) body [else body]