From f7d8e9d02dd18bc6887cbb09097e376b3d338ac1 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Thu, 27 Aug 2026 07:42:19 -0700 Subject: [PATCH] An explicitly-undef optional argument means absent, not bad BOSL2 reaches a builtin whose name it also defines by wrapping it in builtins.scad, which use<>s nothing -- so the names in that file bind to the builtins rather than to BOSL2's own: function _linear_solve(A, b) = linear_solve(A, b); That wrapper has a fixed signature, so a determinant()-style caller passing no right-hand side still forwards b as undef. We treated that as a bad argument: warning on every call and returning undef, which made determinant() unusable through the wrapper. Passing undef is idiomatically the same as not passing in OpenSCAD -- it is how BOSL2 threads optional arguments through wrappers everywhere -- so undef now counts as absent. Same fix for levelset's isovalue, which had the identical problem waiting. Found by testing BOSL2's actual delegation plan rather than assuming it would work. The whole pattern now runs clean on both engines: square [1,3], det 5, least-squares [-1.4,3.1], minimum-norm [0.333,2.333,2.667], singular [], no warnings. The wrapper mechanism itself was verified against OpenSCAD 2026.02.01 first: a function defined in a use<>d file that includes nothing sees the builtin, while the caller's own definition still shadows it at the call site. We match exactly. 1064 tests pass under both engines. --- CLAUDE.md | 8 ++++++++ pyproject.toml | 2 +- src/builtins/function_builtins.cpp | 16 +++++++++++++--- src/builtins/topology.cpp | 16 +++++++++++----- tests/test_booleans.cpp | 13 +++++++++++++ tests/test_function_builtins.cpp | 14 ++++++++++++++ 6 files changed, 60 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1fd77ab..6f2dbaf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -492,6 +492,14 @@ grep for `ponytail:`. same reason. Calling the field function from the generate pass has no live `EvalContext`, so a root is built from the closure's own scope — `$`-variables sit at their defaults inside it. + **An explicitly-undef optional argument counts as ABSENT**, not as a bad one — the same rule in + `linear_solve` (`b`) and `levelset` (`isovalue`). BOSL2 reaches a builtin it also defines by + wrapping it in `builtins.scad`, which `use <>`s nothing, so the names there bind to the builtins: + `function _linear_solve(A, b) = linear_solve(A, b);`. That wrapper has a fixed signature and so + forwards `b` even when the caller passed none, so treating undef as a *bad* argument warned on + every `determinant()`-style call. Passing undef is idiomatically the same as not passing in + OpenSCAD, which is how BOSL2 threads optional arguments through wrappers throughout. + Verified against OpenSCAD 2026.02.01: the wrapper pattern resolves identically there. Sign convention: Manifold takes positive as inside; a distance field is the other way round, so the default flips it and `invert=true` flips back. Two behaviours that look alike and are opposite: an isovalue **nothing reaches** gives empty diff --git a/pyproject.toml b/pyproject.toml index e494826..5d216e6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.50.0" +version = "0.50.1" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/builtins/function_builtins.cpp b/src/builtins/function_builtins.cpp index 69389ae..e211aa3 100644 --- a/src/builtins/function_builtins.cpp +++ b/src/builtins/function_builtins.cpp @@ -1345,9 +1345,19 @@ Value evalBuiltinFunction(Evaluator& ev, const std::string& name, const CallArgs // The OpenSCAD release we track. version_num() is that same // year/month/day folded as y * 10000 + m * 100 + d, exactly like the // reference's own builtin_version_num (builtin_functions.cc). - case BuiltinFnId::LinearSolve: - return builtinLinearSolve(ev, getArg(args, 0, "A", Value{}), getArg(args, 1, "b", Value{}), - args.findPositional(1) != nullptr || args.findNamed("b") != nullptr, node); + case BuiltinFnId::LinearSolve: { + // An explicitly-undef b counts as ABSENT, not as a bad argument. + // A wrapper with a fixed signature -- BOSL2's builtins.scad + // pattern, `function _linear_solve(A, b) = linear_solve(A, b);` + // -- always forwards b, so determinant()-style callers that pass + // no right-hand side would otherwise warn on every call. + // Passing undef is idiomatically the same as not passing in + // OpenSCAD, which is how optional arguments are threaded through + // wrappers throughout BOSL2. + const Value bArg = getArg(args, 1, "b", Value{}); + const bool haveB = !std::holds_alternative(bArg); + return builtinLinearSolve(ev, getArg(args, 0, "A", Value{}), bArg, haveB, node); + } case BuiltinFnId::SupportedFeature: { const Value name = getArg(args, 0, "feature", Value{}); const std::string* s = std::get_if(&name); diff --git a/src/builtins/topology.cpp b/src/builtins/topology.cpp index 37c1042..6e1ece3 100644 --- a/src/builtins/topology.cpp +++ b/src/builtins/topology.cpp @@ -588,12 +588,18 @@ std::vector generateLevelSet(Evaluator& ev, const CSGParams& params } } - const double* isoArg = std::get_if(¶ms.at("isovalue")); - if (!isoArg) { - ev.warn("levelset(): isovalue must be a number", &node.position()); - return {}; + // Same rule as linear_solve's b: an explicitly-undef argument is ABSENT, + // so a fixed-signature wrapper forwarding every parameter still works. + const Value& isoVal = params.at("isovalue"); + double isovalue = 0.0; + if (!std::holds_alternative(isoVal)) { + const double* isoArg = std::get_if(&isoVal); + if (!isoArg) { + ev.warn("levelset(): isovalue must be a number", &node.position()); + return {}; + } + isovalue = *isoArg; } - const double isovalue = *isoArg; const bool invert = truthy(params.at("invert")); const std::array origin = *lo; diff --git a/tests/test_booleans.cpp b/tests/test_booleans.cpp index f1cec87..a1680eb 100644 --- a/tests/test_booleans.cpp +++ b/tests/test_booleans.cpp @@ -1007,3 +1007,16 @@ TEST(LevelSetFn, AFieldFunctionCanCaptureOuterVariables) { ASSERT_EQ(e.bodies.size(), 1u); EXPECT_NEAR(soleBody(e).Volume(), 4.0 / 3.0 * 3.14159265358979 * 8000, 400.0); } + +TEST(LevelSet, AnExplicitlyUndefIsovalueFallsBackToTheDefault) { + // Same rule as linear_solve's b, and for the same reason: a wrapper + // with a fixed signature forwards every parameter, undef included. + Evaluated withUndef = evalSrc(sphereFieldSrc(30, 30) + + "levelset(f, bounds=[[-30,-30,-30],[30,30,30]], isovalue=undef);"); + Evaluated withZero = evalSrc(sphereFieldSrc(30, 30) + + "levelset(f, bounds=[[-30,-30,-30],[30,30,30]], isovalue=0);"); + EXPECT_EQ(withUndef.bodies.size(), withZero.bodies.size()); + EXPECT_TRUE(levelsetWarnings(sphereFieldSrc(30, 30) + + "levelset(f, bounds=[[-30,-30,-30],[30,30,30]], isovalue=undef);") + .empty()); +} diff --git a/tests/test_function_builtins.cpp b/tests/test_function_builtins.cpp index 8ac8c93..27aae63 100644 --- a/tests/test_function_builtins.cpp +++ b/tests/test_function_builtins.cpp @@ -883,3 +883,17 @@ TEST(LinearSolve, WorksUnderBothEngines) { } } + +TEST(LinearSolve, AnExplicitlyUndefRightHandSideCountsAsAbsent) { + // BOSL2's builtins.scad pattern is a wrapper with a fixed signature: + // function _linear_solve(A, b) = linear_solve(A, b); + // so a determinant()-style caller that passes no right-hand side still + // forwards b as undef. Treating that as a BAD argument would warn on + // every such call. Passing undef is idiomatically the same as not + // passing in OpenSCAD -- BOSL2 threads optional arguments through + // wrappers that way throughout. + Evaluator ev; + EXPECT_NEAR(asNum(evalSrc("linear_solve([[2,1],[1,3]], undef).det", ev)), 5.0, 1e-12); + EXPECT_TRUE(isUndef(evalSrc("linear_solve([[2,1],[1,3]], undef).x", ev))); + EXPECT_TRUE(builtinWarnings("linear_solve([[2,1],[1,3]], undef)").empty()); +}