Test that short-circuiting actually short-circuits

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"

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]");

tests/test_control_flow.cpp

@@ -279,6 +279,62 @@ TEST(ListComprehension, ForIfElseMapsBothBranches) {
     EXPECT_EQ(std::get<std::string>(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<ListPtr>(varValue(r, "x"))->items;