Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,14 @@ grep for `ponytail:`.
parameter names live in `builtinParamNames` (`registry.cpp`): each list is the union of real
OpenSCAD's own `Parameters::parse` declaration and any extra name this port reads via `getArg`,
so the port never warns about an argument it goes on to honour — **add to it whenever a builtin
gains a parameter**, or that parameter starts warning. Builtin *functions* deliberately have no
gains a parameter**, or that parameter starts warning. **An argument given as `undef` is not given**: `getArg`
returns the default for it, as every upstream builtin does (`if (v.isDefined())`). Wrappers
forward their own optional parameters as undef -- BOSL2's `rotate_extrude` override passes
`angle=angle`, and reading that as 0 degrees flattened every plain `rotate_extrude()` (#680).
Checked argument by argument against OpenSCAD 2026.02 (47 cases, `tests/test_undef_args.cpp`).
Aliases (`linear_extrude`'s `h`, `rotate_extrude`'s `a`) go through `getArgOrAlias`, which
warns `Specified both "height" and "h"` as upstream and lets the primary name win.
`linear_extrude`'s default height is 100, upstream's (it was 1). Builtin *functions* deliberately have no
entries beyond `textmetrics`/`fontmetrics`: upstream reads their arguments positionally without
`Parameters::parse`, so `sin(bogus=30)` warns about nothing there (verified against 2022.08.22)
and warning here would be a divergence. Not ported: the reference's `argument X supplied more
Expand Down
12 changes: 12 additions & 0 deletions include/openscad_cpp_evaluator/call_args.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,20 @@ ResolvedCallArgs resolveCallArgs(Evaluator& ev, const std::vector<std::unique_pt
// wins). `pos = std::nullopt` for an argument with no positional slot at
// all (e.g. sphere's `d`, only ever named -- a later phase's need, plumbed
// through now since the signature shape matters more than early callers).
//
// An argument given as undef counts as not given, as in every upstream
// builtin (`if (v.isDefined()) ...`): rotate_extrude(angle=undef) is 360
// degrees, not 0. It matters for wrappers that forward their own optional
// parameters -- BOSL2's rotate_extrude override passes `angle=angle`, which
// flattened every plain rotate_extrude() into its profile (#680).
Value getArg(const CallArgs& args, std::optional<int> pos, const std::string& name, Value defaultValue = Value{});

// getArg for a parameter with an alternative name (linear_extrude's
// height/h, rotate_extrude's angle/a): `name` wins when both are given,
// with upstream's warning.
Value getArgOrAlias(Evaluator& ev, const oscad::Position* where, const CallArgs& args, std::optional<int> pos,
const std::string& name, const std::string& alias, Value defaultValue);

// Encodes a CallArgs as a single Value (positional -> a list indexed 0..N,
// named -> an object) so a resolve function can carry a call's raw
// arguments across into CSGParams (which only holds Value) for a generate
Expand Down
21 changes: 19 additions & 2 deletions src/builtins/call_args.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,31 @@ CallArgs resolveArgs(Evaluator& ev, const std::vector<std::unique_ptr<oscad::Arg
return result;
}

namespace {
bool isUndef(const Value& v) { return std::holds_alternative<std::monostate>(v); }
} // namespace

Value getArg(const CallArgs& args, std::optional<int> pos, const std::string& name, Value defaultValue) {
if (const Value* named = args.findNamed(name)) return *named;
if (const Value* named = args.findNamed(name); named && !isUndef(*named)) return *named;
if (pos.has_value()) {
if (const Value* positional = args.findPositional(*pos)) return *positional;
if (const Value* positional = args.findPositional(*pos); positional && !isUndef(*positional))
return *positional;
}
return defaultValue;
}

Value getArgOrAlias(Evaluator& ev, const oscad::Position* where, const CallArgs& args, std::optional<int> pos,
const std::string& name, const std::string& alias, Value defaultValue) {
const Value primary = getArg(args, pos, name, Value{});
const Value other = getArg(args, std::nullopt, alias, Value{});
if (!isUndef(primary)) {
if (!isUndef(other))
ev.warn("Specified both \"" + name + "\" and \"" + alias + "\"", where);
return primary;
}
return isUndef(other) ? defaultValue : other;
}

ResolvedCallArgs resolveCallArgs(Evaluator& ev, const std::vector<std::unique_ptr<oscad::Argument>>& arguments,
EvalContext& ctx) {
CallArgs args = resolveArgs(ev, arguments, ctx);
Expand Down
6 changes: 4 additions & 2 deletions src/builtins/extrude.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ manifold::Manifold extrudeTwisted(const manifold::Polygons& polys, double height
BuiltinWrapParams computeLinearExtrudeParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) {
auto [args, effCtx] = resolveCallArgs(ev, node.arguments, ctx);

const double height = toDoubleLenient(getArg(args, 0, "height", Value{1.0}));
// 100 when not given, as upstream (the old 1 was a guess), and `h` as
// upstream's alias -- BOSL2's linear_extrude override forwards it.
const double height = toDoubleLenient(getArgOrAlias(ev, &node.position(), args, 0, "height", "h", Value{100.0}));
const bool center = truthy(getArg(args, std::nullopt, "center", Value{false}));
const double twist = toDoubleLenient(getArg(args, std::nullopt, "twist", Value{0.0}));
// Upstream's validate_integral: any finite number counts as given, and is
Expand Down Expand Up @@ -239,7 +241,7 @@ std::vector<ColoredBody> generateLinearExtrude(Evaluator& ev, const CSGParams& p
BuiltinWrapParams computeRotateExtrudeParams(Evaluator& ev, const oscad::ModularCall& node, EvalContext& ctx) {
auto [args, effCtx] = resolveCallArgs(ev, node.arguments, ctx);

const double angle = toDoubleLenient(getArg(args, 0, "angle", Value{360.0}));
const double angle = toDoubleLenient(getArgOrAlias(ev, &node.position(), args, 0, "angle", "a", Value{360.0}));

CSGParams params;
params["angle"] = Value{angle};
Expand Down
4 changes: 2 additions & 2 deletions src/builtins/registry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ const std::vector<std::string>* builtinParamNames(const std::string& name) {
{"import",
{"file", "layer", "convexity", "origin", "scale", "width", "height", "filename", "layername", "center", "dpi",
"id", "class", "repair", "tolerance"}},
{"linear_extrude", {"height", "v", "scale", "center", "twist", "slices", "segments", "convexity"}},
{"rotate_extrude", {"angle", "start", "convexity"}},
{"linear_extrude", {"height", "h", "v", "scale", "center", "twist", "slices", "segments", "convexity"}},
{"rotate_extrude", {"angle", "a", "start", "convexity"}},
{"projection", {"cut", "convexity"}},
{"roof", {"method", "convexity"}},
{"fill", {}},
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ add_executable(oscad_eval_tests
test_use.cpp
test_coverage.cpp
test_extrude_roof.cpp
test_undef_args.cpp
test_discretizer.cpp
test_surface.cpp
test_dxf_svg_import.cpp
Expand Down
100 changes: 100 additions & 0 deletions tests/test_undef_args.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// A builtin argument given as undef means "not given" (#680), and the
// height/h and angle/a aliases, as upstream OpenSCAD -- each checked
// against OpenSCAD 2026.02 before being written down here.
//
// Wrappers forward their own optional parameters: BOSL2's rotate_extrude
// override passes angle=angle, undef when the user gave none, and taking
// that undef as 0 degrees flattened every plain rotate_extrude() into its
// profile.

#include "test_helpers.hpp"

#include <gtest/gtest.h>

#include <string>
#include <vector>

using namespace oscadeval;
using namespace oscadeval::test;

namespace {

struct Shape {
double volume;
manifold::Box box;
};

Shape shapeOf(const std::string& code) {
Evaluated e = evalSrc(code);
EXPECT_EQ(e.bodies.size(), 1u) << code;
if (e.bodies.empty() || !e.bodies[0].body.has_value()) return {0.0, {}};
return {e.bodies[0].body->Volume(), e.bodies[0].body->BoundingBox()};
}

void expectSameShape(const std::string& a, const std::string& b) {
const Shape x = shapeOf(a), y = shapeOf(b);
EXPECT_GT(x.volume, 0.0) << a;
EXPECT_NEAR(x.volume, y.volume, 1e-6) << a << " vs " << b;
EXPECT_NEAR(x.box.max.x, y.box.max.x, 1e-6) << a;
EXPECT_NEAR(x.box.max.y, y.box.max.y, 1e-6) << a;
EXPECT_NEAR(x.box.max.z, y.box.max.z, 1e-6) << a;
EXPECT_NEAR(x.box.min.x, y.box.min.x, 1e-6) << a;
}

std::vector<std::string> warningsFor(const std::string& code) {
std::vector<std::string> out;
Evaluated e = evalSrc(code, [&](const std::string& msg) {
if (msg.rfind("WARNING: ", 0) == 0) out.push_back(msg);
});
(void)e;
return out;
}

} // namespace

// Every argument that took an explicit undef as a value rather than as absent.
TEST(UndefArgs, AnUndefArgumentIsTheDefault) {
expectSameShape("rotate_extrude(angle=undef) translate([10,0]) square(5);",
"rotate_extrude() translate([10,0]) square(5);");
expectSameShape("linear_extrude(height=undef) square(5);", "linear_extrude() square(5);");
expectSameShape("cylinder(h=undef, r=2);", "cylinder(r=2);");
expectSameShape("linear_extrude(1) square(size=undef);", "linear_extrude(1) square();");
expectSameShape("linear_extrude(1) text(\"A\", size=undef);", "linear_extrude(1) text(\"A\");");
expectSameShape("linear_extrude(1) text(\"AB\", spacing=undef);", "linear_extrude(1) text(\"AB\");");
// Positional undef too.
expectSameShape("rotate_extrude(undef) translate([10,0]) square(5);",
"rotate_extrude() translate([10,0]) square(5);");
}

TEST(UndefArgs, AWrapperForwardingItsOwnOptionalAngleStillRevolvesFully) {
expectSameShape("module re(angle) rotate_extrude(angle=angle) children();\n"
"re() translate([10,0]) square(5);",
"rotate_extrude() translate([10,0]) square(5);");
}

TEST(UndefArgs, LinearExtrudeDefaultsToAHeightOfOneHundred) {
EXPECT_NEAR(shapeOf("linear_extrude() square(5);").box.max.z, 100.0, 1e-9);
const Shape centred = shapeOf("linear_extrude(center=true) square(5);");
EXPECT_NEAR(centred.box.min.z, -50.0, 1e-9);
EXPECT_NEAR(centred.box.max.z, 50.0, 1e-9);
}

TEST(UndefArgs, HIsHeightAndAIsAngle) {
EXPECT_NEAR(shapeOf("linear_extrude(h=7) square(5);").box.max.z, 7.0, 1e-9);
expectSameShape("rotate_extrude(a=90) translate([10,0]) square(5);",
"rotate_extrude(angle=90) translate([10,0]) square(5);");
EXPECT_TRUE(warningsFor("linear_extrude(h=7) square(5);").empty());
EXPECT_TRUE(warningsFor("rotate_extrude(a=90) translate([10,0]) square(5);").empty());
}

TEST(UndefArgs, BothNamesGivenWarnsAndThePrimaryWins) {
EXPECT_NEAR(shapeOf("linear_extrude(height=3, h=7) square(5);").box.max.z, 3.0, 1e-9);
const auto w = warningsFor("linear_extrude(height=3, h=7) square(5);");
ASSERT_EQ(w.size(), 1u);
EXPECT_NE(w[0].find("Specified both \"height\" and \"h\""), std::string::npos) << w[0];
expectSameShape("rotate_extrude(angle=45, a=90) translate([10,0]) square(5);",
"rotate_extrude(angle=45) translate([10,0]) square(5);");
const auto wa = warningsFor("rotate_extrude(angle=45, a=90) translate([10,0]) square(5);");
ASSERT_EQ(wa.size(), 1u);
EXPECT_NE(wa[0].find("Specified both \"angle\" and \"a\""), std::string::npos) << wa[0];
}
Loading