diff --git a/pyproject.toml b/pyproject.toml index 98b14e9..2297683 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "0.32.0" +version = "0.32.1" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/tests/test_bytecode_compiler.cpp b/tests/test_bytecode_compiler.cpp index aef7791..696d095 100644 --- a/tests/test_bytecode_compiler.cpp +++ b/tests/test_bytecode_compiler.cpp @@ -99,6 +99,35 @@ TEST(BytecodeCompiler, TernaryAndShortCircuitLogicalOps) { EXPECT_EQ(runCapturingEcho("function safe(x) = is_undef(x) || x > 0;\necho(safe(undef));"), "ECHO: true"); } +// TernaryAndShortCircuitLogicalOps above only asserts the RESULT, which an +// eager compiler computes just as correctly -- so it cannot actually fail if +// someone drops the JumpIfFalse/JumpIfTrue the compiler emits around the +// right operand / untaken branch. test_expr_eval.cpp's own +// RightSideNotEvaluatedWhenShortCircuited and OnlyChosenBranchEvaluates +// cover that for the AST interpreter only. These two do it for the compiled +// path, watching for a marker echo() from the side that must never run. +TEST(BytecodeCompiler, ShortCircuitDoesNotEvaluateRightOperandCompiled) { + ScopedVm vm(true); + const std::string defs = "function rhs() = echo(\"RHS RAN\") true;\n"; + // Left operand already decides it -- rhs() must stay unevaluated. + EXPECT_EQ(runCapturingEcho(defs + "function f() = true || rhs();\necho(f());"), "ECHO: true"); + EXPECT_EQ(runCapturingEcho(defs + "function f() = false && rhs();\necho(f());"), "ECHO: false"); + // ...and the mirror image: it must still run when the left operand doesn't. + EXPECT_EQ(runCapturingEcho(defs + "function f() = false || rhs();\necho(f());"), + "ECHO: \"RHS RAN\"\nECHO: true"); + EXPECT_EQ(runCapturingEcho(defs + "function f() = true && rhs();\necho(f());"), + "ECHO: \"RHS RAN\"\nECHO: true"); +} + +TEST(BytecodeCompiler, TernaryOnlyEvaluatesChosenBranchCompiled) { + ScopedVm vm(true); + const std::string defs = "function yes() = echo(\"TRUE-BRANCH\") 1;\n" + "function no() = echo(\"FALSE-BRANCH\") 2;\n" + "function pick(c) = c ? yes() : no();\n"; + EXPECT_EQ(runCapturingEcho(defs + "echo(pick(true));"), "ECHO: \"TRUE-BRANCH\"\nECHO: 1"); + EXPECT_EQ(runCapturingEcho(defs + "echo(pick(false));"), "ECHO: \"FALSE-BRANCH\"\nECHO: 2"); +} + TEST(BytecodeCompiler, PlainListLiteralCompiles) { ScopedVm vm(true); EXPECT_EQ(runCapturingEcho("function mk(x) = [x, x * 2, x * 3];\necho(mk(2));"), "ECHO: [2, 4, 6]"); diff --git a/tests/test_control_flow.cpp b/tests/test_control_flow.cpp index 67a2185..9c8eaaa 100644 --- a/tests/test_control_flow.cpp +++ b/tests/test_control_flow.cpp @@ -279,6 +279,62 @@ TEST(ListComprehension, ForIfElseMapsBothBranches) { EXPECT_EQ(std::get(items[1]), "odd"); } +// The two tests above assert only the resulting LIST, which an eager +// implementation -- one that evaluated an `if` body whose condition failed, +// or both sides of an `if`/`else` -- would still get right. So they cannot +// fail if evalListElement's ListCompIf/ListCompIfElse cases (or their +// compiled counterparts in compileListElement) lose their laziness, the way +// `&&`/`||` demonstrably once did (see expr_eval.cpp's LogicalAndOp comment). +// These four watch for a marker echo() from the branch that must never run. +namespace { + +// Every echo() the script emits, newline-joined -- the surrounding tests' +// own `captured = msg` lambdas keep only the last one, which is no use when +// the point of the test is exactly how MANY echoes happened. +std::string echoesFrom(const std::string& code) { + std::string captured; + runScript(code, [&](const std::string& msg) { + if (!captured.empty()) captured += "\n"; + captured += msg; + }); + return captured; +} + +} // namespace + +TEST(ListComprehension, ForIfSkipsBodyWhenConditionFailsInterpreted) { + ScopedVm vm(false); + EXPECT_EQ(echoesFrom("function mark(i) = echo(str(\"BODY\", i)) i;\n" + "x = [for (i = [0:2]) if (i == 1) mark(i)];"), + "ECHO: \"BODY1\""); +} + +TEST(ListComprehension, ForIfSkipsBodyWhenConditionFailsCompiled) { + ScopedVm vm(true); + EXPECT_EQ(echoesFrom("function mark(i) = echo(str(\"BODY\", i)) i;\n" + "function mk() = [for (i = [0:2]) if (i == 1) mark(i)];\n" + "x = mk();"), + "ECHO: \"BODY1\""); +} + +TEST(ListComprehension, ForIfElseEvaluatesOnlyTheChosenBranchInterpreted) { + ScopedVm vm(false); + // One echo per iteration, not two: i=0 takes `yes`, i=1 takes `no`. + EXPECT_EQ(echoesFrom("function yes() = echo(\"TRUE-BRANCH\") 1;\n" + "function no() = echo(\"FALSE-BRANCH\") 2;\n" + "x = [for (i = [0:1]) if (i == 0) yes() else no()];"), + "ECHO: \"TRUE-BRANCH\"\nECHO: \"FALSE-BRANCH\""); +} + +TEST(ListComprehension, ForIfElseEvaluatesOnlyTheChosenBranchCompiled) { + ScopedVm vm(true); + EXPECT_EQ(echoesFrom("function yes() = echo(\"TRUE-BRANCH\") 1;\n" + "function no() = echo(\"FALSE-BRANCH\") 2;\n" + "function mk() = [for (i = [0:1]) if (i == 0) yes() else no()];\n" + "x = mk();"), + "ECHO: \"TRUE-BRANCH\"\nECHO: \"FALSE-BRANCH\""); +} + TEST(ListComprehension, EachFlattensNestedLists) { RunResult r = runScript("x = [each [1,2], each [3,4]];"); auto items = std::get(varValue(r, "x"))->items; diff --git a/uv.lock b/uv.lock index bffd47b..2a610b7 100644 --- a/uv.lock +++ b/uv.lock @@ -55,7 +55,7 @@ wheels = [ [[package]] name = "openscad-cpp-evaluator" -version = "0.32.0" +version = "0.32.1" source = { editable = "." } dependencies = [ { name = "numpy" },