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()); +}