diff --git a/include/openscad_cpp_parser/ast/ast_node.hpp b/include/openscad_cpp_parser/ast/ast_node.hpp index 9cfe951..a1a489e 100644 --- a/include/openscad_cpp_parser/ast/ast_node.hpp +++ b/include/openscad_cpp_parser/ast/ast_node.hpp @@ -61,6 +61,10 @@ enum class NodeKind { PrimaryCall, PrimaryIndex, PrimaryMember, + // render() in EXPRESSION position -- evaluates its children as geometry + // and yields an object() of measurements. The STATEMENT form of render() + // is a plain ModularCall named "render"; only this one is new. + RenderExpression, // List comprehension ListCompLet, ListCompEach, diff --git a/include/openscad_cpp_parser/ast/expression.hpp b/include/openscad_cpp_parser/ast/expression.hpp index 247cc37..cf84932 100644 --- a/include/openscad_cpp_parser/ast/expression.hpp +++ b/include/openscad_cpp_parser/ast/expression.hpp @@ -237,6 +237,33 @@ class FunctionLiteral : public Expression { void buildScope(Scope& parentScope) override; }; +// `render()` in EXPRESSION position: `obj = render() { cube(10); };` +// +// Evaluates its children as geometry, measures the result, and yields an +// object() -- it draws nothing. The STATEMENT form (`render() cube(1);`) +// stays a plain ModularCall named "render"; this class exists only because +// an Expression cannot be a ModuleInstantiation (they are siblings under +// ASTNode, not parent/child). +// +// `arguments` and `children` deliberately mirror ModularCall's field types +// so the evaluator can hand them straight to resolveCallArgs()/evalChildren() +// with no adaptation. `children` is ASTNode, not ModuleInstantiation, for +// the same reason ModularCall's is -- a `{ ... }` block is `statement*` and +// may hold Assignments, which is why buildScope() below hoists. +class RenderExpression : public Expression { +public: + RenderExpression(Position position, std::vector> arguments, + std::vector> children) + : Expression(NodeKind::RenderExpression, std::move(position)), arguments(std::move(arguments)), + children(std::move(children)) {} + + std::vector> arguments; + std::vector> children; + + std::string toString() const override; + void buildScope(Scope& parentScope) override; +}; + // -- Unary operators -------------------------------------------------- #define OSCAD_UNARY_OP(ClassName) \ diff --git a/src/ast/ast_node.cpp b/src/ast/ast_node.cpp index d85b58b..7e6edf4 100644 --- a/src/ast/ast_node.cpp +++ b/src/ast/ast_node.cpp @@ -47,6 +47,7 @@ const char* nodeKindName(NodeKind kind) { case NodeKind::PrimaryCall: return "PrimaryCall"; case NodeKind::PrimaryIndex: return "PrimaryIndex"; case NodeKind::PrimaryMember: return "PrimaryMember"; + case NodeKind::RenderExpression: return "RenderExpression"; case NodeKind::ListCompLet: return "ListCompLet"; case NodeKind::ListCompEach: return "ListCompEach"; case NodeKind::ListCompFor: return "ListCompFor"; diff --git a/src/ast/expression.cpp b/src/ast/expression.cpp index c3a4206..180aaf6 100644 --- a/src/ast/expression.cpp +++ b/src/ast/expression.cpp @@ -1,6 +1,7 @@ #include "openscad_cpp_parser/ast/expression.hpp" #include "format_utils.hpp" +#include "openscad_cpp_parser/ast/scope_builder.hpp" #include "openscad_cpp_parser/scope.hpp" #include @@ -155,6 +156,47 @@ void FunctionLiteral::buildScope(Scope& parentScope) { body->buildScope(funcScope); } +std::string RenderExpression::toString() const { + // Deliberately NOT ModuleInstantiation's formatChildBlock, which omits + // both braces (for a lone child) and every statement terminator, because + // for a STATEMENT the terminators come from the statement-level printer. + // A RenderExpression sits inside an expression, where nothing downstream + // adds them -- and `render() cube(1)` unbraced is precisely the form that + // does NOT parse (the child_statement swallows the `;`, leaving the + // enclosing assignment unterminated). So: always braces, always a `;` + // after every child. + // + // This must stay REPARSEABLE -- pretty_print.cpp's fmtExpr falls through + // to toString() for expression kinds it has no case for, so this string + // is what a formatter emits. NodeStr.RenderExpressionRoundTrips guards it. + // + // The unconditional `;` is safe even after a child that already ends in + // `}` (a nested block, a module declaration): a bare `;` is itself a legal + // statement (parser.y's `statement: ";"`), so a redundant one is a no-op. + std::string s = "render(" + joinToString(arguments, ", ") + ") { "; + for (const auto& c : children) { + s += c->toString() + "; "; + } + return s + "}"; +} + +void RenderExpression::buildScope(Scope& parentScope) { + // Same shape as ModularCall::buildScope minus the name lookup (there is + // no Identifier -- "render" is a keyword token here). The hoist is + // required: `render() { x = 1; cube(x); }` must resolve x. + setScope(parentScope); + for (auto& a : arguments) { + a->buildScope(parentScope); + } + if (!children.empty()) { + Scope& childrenScope = parentScope.childScope(); + collectHoistedDeclarations(children, childrenScope); + for (auto& c : children) { + c->buildScope(childrenScope); + } + } +} + // -- Operator precedence for minimal-parenthesization toString() ---------- // // Matches the reference's nodes.py::_PREC/_lp/_rp: toString() adds parens diff --git a/src/grammar/driver.cpp b/src/grammar/driver.cpp index 8161cdd..36f7092 100644 --- a/src/grammar/driver.cpp +++ b/src/grammar/driver.cpp @@ -153,6 +153,11 @@ NodePtr makeListComprehension(ParserDriver& driver, const OscadLocation& loc, No return std::make_unique(driver.toPosition(loc), std::move(elements)); } +NodePtr makeRenderExpression(ParserDriver& driver, const OscadLocation& loc, NodeList arguments, NodeList children) { + return std::make_unique(driver.toPosition(loc), nodeListCast(std::move(arguments)), + std::move(children)); +} + NodePtr makeModularCall(ParserDriver& driver, const OscadLocation& loc, const OscadLocation& nameLoc, std::string name, NodeList arguments, NodeList children) { auto nameNode = makeIdentifier(driver, nameLoc, std::move(name)); diff --git a/src/grammar/driver.hpp b/src/grammar/driver.hpp index 9793dd0..19a299e 100644 --- a/src/grammar/driver.hpp +++ b/src/grammar/driver.hpp @@ -132,6 +132,8 @@ NodePtr makeListCompIfElse(ParserDriver& driver, const OscadLocation& loc, NodeP NodePtr falseExpr); NodePtr makeListComprehension(ParserDriver& driver, const OscadLocation& loc, NodeList elements); +NodePtr makeRenderExpression(ParserDriver& driver, const OscadLocation& loc, NodeList arguments, NodeList children); + NodePtr makeModularCall(ParserDriver& driver, const OscadLocation& loc, const OscadLocation& nameLoc, std::string name, NodeList arguments, NodeList children); NodePtr makeModularFor(ParserDriver& driver, const OscadLocation& loc, NodeList assignments, NodeList body); diff --git a/src/grammar/lexer.l b/src/grammar/lexer.l index 1a8a704..9b42eba 100644 --- a/src/grammar/lexer.l +++ b/src/grammar/lexer.l @@ -47,6 +47,11 @@ const std::unordered_map& keywordTable() { {"for", [](const OscadLocation& l) { return yy::parser::make_KW_FOR(l); }}, {"intersection_for", [](const OscadLocation& l) { return yy::parser::make_KW_INTERSECTION_FOR(l); }}, {"each", [](const OscadLocation& l) { return yy::parser::make_KW_EACH(l); }}, + // Reserved so `render` can lead an EXPRESSION-position geometry block + // (`obj = render() { cube(1); };`). LALR(1) cannot otherwise tell that + // apart from a function call. Note `$render` still lexes as NAME -- + // this table is keyed on the full IDENT text, including the `$`. + {"render", [](const OscadLocation& l) { return yy::parser::make_KW_RENDER(l); }}, {"undef", [](const OscadLocation& l) { return yy::parser::make_KW_UNDEF(l); }}, {"true", [](const OscadLocation& l) { return yy::parser::make_KW_TRUE(l); }}, {"false", [](const OscadLocation& l) { return yy::parser::make_KW_FALSE(l); }}, diff --git a/src/grammar/parser.y b/src/grammar/parser.y index 9f4cbcb..3f5a319 100644 --- a/src/grammar/parser.y +++ b/src/grammar/parser.y @@ -92,6 +92,7 @@ KW_FOR "for" KW_INTERSECTION_FOR "intersection_for" KW_EACH "each" + KW_RENDER "render" KW_UNDEF "undef" KW_TRUE "true" KW_FALSE "false" @@ -157,6 +158,7 @@ %type modifier_show_only modifier_highlight modifier_background modifier_disable %type if_statement ifelse_statement %type modular_for modular_intersection_for modular_let modular_assert modular_echo modular_call +%type render_stmt render_expr %type expr opchain postfix primary %type range_expr vector_expr vector_element %type listcomp_elements listcomp_paren_expr listcomp_let listcomp_each @@ -305,6 +307,7 @@ single_module_instantiation: | modular_assert { $$ = std::move($1); } | modular_echo { $$ = std::move($1); } | modular_call { $$ = std::move($1); } + | render_stmt { $$ = std::move($1); } ; modular_for: @@ -343,6 +346,16 @@ modular_call: } ; +// `render` is a reserved keyword (see lexer.l) purely so the EXPRESSION form +// below is unambiguous. The STATEMENT form still builds a plain ModularCall +// named "render", so everything downstream -- builtin dispatch, the argument +// allowlist, the pretty-printer, json_io -- is unchanged. +render_stmt: + "render" "(" arguments ")" child_statement { + $$ = makeModularCall(driver, @$, @1, "render", std::move($3), std::move($5)); + } + ; + // -- Expressions ---------------------------------------------------------- // // `expr` covers let/assert/echo/funclit_def/ternary plus the operator @@ -406,6 +419,23 @@ primary: | STRING { $$ = makeStringLiteral(driver, @$, std::move($1)); } | NUMBER { $$ = makeNumberLiteral(driver, @$, $1); } | NAME { $$ = makeIdentifier(driver, @$, std::move($1)); } + | render_expr { $$ = std::move($1); } + ; + +// Same RHS as render_stmt, reached only from `primary`. This is NOT a +// reduce/reduce conflict: LALR merges states only on identical LR(0) cores, +// and after shifting "render" the statement and expression kernels differ -- +// no single state closes over both (statement-start closes over +// module_instantiation; every expr-start position closes over no statement +// nonterminal). Bison verifies this claim at build time via %expect. +// +// It lives in `primary` rather than `expr` so `render(){...}.volume` parses +// through the existing `postfix "." NAME` rule, and so it can appear as an +// argument, a list element, or an operand. +render_expr: + "render" "(" arguments ")" child_statement { + $$ = makeRenderExpression(driver, @$, std::move($3), std::move($5)); + } ; range_expr: diff --git a/src/inline_comment_attach.cpp b/src/inline_comment_attach.cpp index 3955017..0e305e5 100644 --- a/src/inline_comment_attach.cpp +++ b/src/inline_comment_attach.cpp @@ -295,6 +295,12 @@ void classifyNode(ASTNode& node, std::vector& exprFields, std::vector< addAstNodeList(static_cast(node).elements, exprFields, nonExprChildren); break; + case NodeKind::RenderExpression: { + auto& n = static_cast(node); + addArgumentExprList(n.arguments, exprFields); + addAstNodeList(n.children, exprFields, nonExprChildren); + break; + } case NodeKind::ModularCall: { auto& n = static_cast(node); addArgumentExprList(n.arguments, exprFields); diff --git a/src/pretty_print.cpp b/src/pretty_print.cpp index 4cdec0f..380fdfd 100644 --- a/src/pretty_print.cpp +++ b/src/pretty_print.cpp @@ -207,6 +207,7 @@ std::string fmtNode(const ASTNode& node, int indent, int w); std::string fmtExpr(const ASTNode& expr, int indent, int w); std::string fmtInst(const ASTNode& node, int indent, int w, const std::string& prefix); std::string fmtListElem(const ASTNode& elem, int indent, int w); +std::string fmtBlock(const std::vector>& nodes, int indent, int w); std::string fmtAssign(const Assignment& a, int indent, int w) { return a.name->name + " = " + fmtExpr(*a.expr, indent, w); @@ -508,6 +509,16 @@ std::string fmtExpr(const ASTNode& exprNode, int indent, int w) { if (auto* lc = dynamic_cast(&exprNode)) { return fmtListComprehension(*lc, indent, w); } + if (auto* r = dynamic_cast(&exprNode)) { + // fmtBlock, NOT fmtChild: fmtChild drops the braces for a lone child, + // and `x = render() cube(1);` does not parse -- the child_statement + // swallows the `;` and leaves the assignment unterminated. fmtBlock + // always braces and routes children through fmtNode, which is what + // supplies their statement terminators. This arm is why toString()'s + // own (reference-matching, terminator-free) child rendering never + // reaches emitted source. + return "render(" + joinToString(r->arguments, ", ") + ") " + fmtBlock(r->children, indent, w); + } return exprNode.toString(); } diff --git a/src/serialization/json_io.cpp b/src/serialization/json_io.cpp index 3d83d87..bef1843 100644 --- a/src/serialization/json_io.cpp +++ b/src/serialization/json_io.cpp @@ -267,6 +267,13 @@ json toJsonImpl(const ASTNode& node, bool includePos) { j["elements"] = listToJson(static_cast(node).elements, includePos); break; + case NodeKind::RenderExpression: { + auto& n = static_cast(node); + j["arguments"] = listToJson(n.arguments, includePos); + j["children"] = listToJson(n.children, includePos); + break; + } + case NodeKind::ModularCall: { auto& n = static_cast(node); j["name"] = valueToJson(n.name.get(), includePos); @@ -543,6 +550,11 @@ const std::unordered_map& registry() { return std::make_unique(std::move(pos), listFromJson(j, "elements")); }}, + {"RenderExpression", + [](const json& j, Position pos) -> std::unique_ptr { + return std::make_unique(std::move(pos), listFromJson(j, "arguments"), + listFromJson(j, "children")); + }}, {"ModularCall", [](const json& j, Position pos) -> std::unique_ptr { return std::make_unique(std::move(pos), childFromJson(j, "name"), diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d653e8b..6008c71 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -25,6 +25,7 @@ add_executable(oscad_tests test_scope.cpp test_ast_generation.cpp test_node_str.cpp + test_render_expression.cpp ) target_link_libraries(oscad_tests PRIVATE openscad_cpp_parser GTest::gtest_main) diff --git a/tests/test_render_expression.cpp b/tests/test_render_expression.cpp new file mode 100644 index 0000000..280088e --- /dev/null +++ b/tests/test_render_expression.cpp @@ -0,0 +1,186 @@ +// `render()` in EXPRESSION position: `obj = render() { cube(1); };` +// +// The statement form (`render() cube(1);`) is deliberately UNCHANGED -- it +// still parses to a plain ModularCall named "render", so everything +// downstream (builtin dispatch, the argument allowlist, json_io) sees what +// it always saw. Only the expression form is new, and it exists at all +// because LALR(1) cannot otherwise tell `render(` in expression position +// apart from a function call -- hence `render` being a reserved keyword. + +#include "openscad_cpp_parser/api.hpp" +#include "openscad_cpp_parser/pretty_print.hpp" +#include "openscad_cpp_parser/serialization.hpp" +#include "test_helpers.hpp" + +#include + +using namespace oscad; + +namespace { + +const RenderExpression* renderExprOf(const std::vector>& ast, size_t idx = 0) { + auto* a = dynamic_cast(ast[idx].get()); + return a ? dynamic_cast(a->expr.get()) : nullptr; +} + +} // namespace + +// -- The expression form -------------------------------------------------- + +TEST(RenderExpression, ParsesInAssignment) { + auto ast = parseSrc("obj = render() { cube(1); };"); + ASSERT_EQ(ast.size(), 1u); + const RenderExpression* r = renderExprOf(ast); + ASSERT_NE(r, nullptr); + EXPECT_EQ(r->kind(), NodeKind::RenderExpression); + EXPECT_TRUE(r->arguments.empty()); + ASSERT_EQ(r->children.size(), 1u); + EXPECT_EQ(r->children[0]->kind(), NodeKind::ModularCall); +} + +TEST(RenderExpression, ParsesTheOriginalMotivatingExample) { + // A trailing `}` child needs no `;` of its own, so this form -- unlike a + // bare call -- terminates its enclosing assignment cleanly. + auto ast = parseSrc("obj = render() difference() { cube(100); sphere(20); };"); + ASSERT_EQ(ast.size(), 1u); + const RenderExpression* r = renderExprOf(ast); + ASSERT_NE(r, nullptr); + ASSERT_EQ(r->children.size(), 1u); + auto* diff = dynamic_cast(r->children[0].get()); + ASSERT_NE(diff, nullptr); + EXPECT_EQ(diff->name->name, "difference"); + EXPECT_EQ(diff->children.size(), 2u); +} + +TEST(RenderExpression, SupportsMemberAccess) { + // Lives in `primary`, not `expr`, precisely so the existing + // `postfix "." NAME` rule applies. + auto ast = parseSrc("v = render() { cube(1); }.volume;"); + ASSERT_EQ(ast.size(), 1u); + auto* a = dynamic_cast(ast[0].get()); + ASSERT_NE(a, nullptr); + auto* member = dynamic_cast(a->expr.get()); + ASSERT_NE(member, nullptr); + EXPECT_EQ(member->member->name, "volume"); + EXPECT_EQ(member->left->kind(), NodeKind::RenderExpression); +} + +TEST(RenderExpression, TakesArguments) { + auto ast = parseSrc("obj = render($fn=32) { sphere(10); };"); + const RenderExpression* r = renderExprOf(ast); + ASSERT_NE(r, nullptr); + ASSERT_EQ(r->arguments.size(), 1u); + auto* named = dynamic_cast(r->arguments[0].get()); + ASSERT_NE(named, nullptr); + EXPECT_EQ(named->name->name, "$fn"); +} + +TEST(RenderExpression, AcceptsEmptyAndMultipleChildren) { + EXPECT_EQ(renderExprOf(parseSrc("e = render() { };"))->children.size(), 0u); + EXPECT_EQ(renderExprOf(parseSrc("m = render() { cube(1); sphere(2); cylinder(3); };"))->children.size(), 3u); +} + +TEST(RenderExpression, WorksAsArgumentAndListElement) { + EXPECT_NO_THROW(parseSrc("echo(render() { cube(1); });")); + EXPECT_NO_THROW(parseSrc("v = [render() { cube(1); }, render() { sphere(2); }];")); + EXPECT_NO_THROW(parseSrc("v = 1 + render() { cube(1); }.volume;")); + EXPECT_NO_THROW(parseSrc("function f(a) = render() { cube(a); }.volume;")); + EXPECT_NO_THROW(parseSrc("v = [for (i = [0:2]) render() { cube(i); }];")); +} + +// -- The statement form is untouched -------------------------------------- + +TEST(RenderExpression, StatementFormIsStillAModularCall) { + auto ast = parseSrc("render() cube(1);"); + ASSERT_EQ(ast.size(), 1u); + auto* call = dynamic_cast(ast[0].get()); + ASSERT_NE(call, nullptr); + EXPECT_EQ(call->kind(), NodeKind::ModularCall); + EXPECT_EQ(call->name->name, "render"); + EXPECT_EQ(call->children.size(), 1u); +} + +TEST(RenderExpression, StatementFormKeepsArgumentsAndModifiers) { + auto ast = parseSrc("render(convexity=4) cube(1);\n#render() sphere(2);\nrender() { cube(1); sphere(2); }"); + ASSERT_EQ(ast.size(), 3u); + auto* withArgs = dynamic_cast(ast[0].get()); + ASSERT_NE(withArgs, nullptr); + EXPECT_EQ(withArgs->arguments.size(), 1u); + EXPECT_EQ(ast[1]->kind(), NodeKind::ModularModifierHighlight); + auto* braced = dynamic_cast(ast[2].get()); + ASSERT_NE(braced, nullptr); + EXPECT_EQ(braced->children.size(), 2u); +} + +// -- The sharp edge, asserted on purpose ---------------------------------- + +TEST(RenderExpression, BareCallChildDoesNotTerminateTheAssignment) { + // `cube(1);` IS the child_statement, semicolon included, so the enclosing + // assignment is left unterminated. Inherent to OpenSCAD's grammar, not a + // choice this implementation makes. The braced form is the documented + // idiom; this test exists so the behaviour is pinned rather than + // rediscovered. + EXPECT_THROW(parseSrc("obj = render() cube(1);"), ParseError); + EXPECT_NO_THROW(parseSrc("obj = render() { cube(1); };")); +} + +// -- Reserving the keyword has costs; pin them ---------------------------- + +TEST(RenderExpression, RenderIsNowAReservedWord) { + EXPECT_THROW(parseSrc("render = 1;"), ParseError); + EXPECT_THROW(parseSrc("module render() { cube(1); }"), ParseError); + EXPECT_THROW(parseSrc("function render() = 1;"), ParseError); + EXPECT_THROW(parseSrc("cube(render=1);"), ParseError); + EXPECT_THROW(parseSrc("x = obj.render;"), ParseError); +} + +TEST(RenderExpression, DollarRenderIsStillAnOrdinaryName) { + // keywordTable() is keyed on the full IDENT text, `$` included. + EXPECT_NO_THROW(parseSrc("$render = 1;")); +} + +// -- Round-trips ---------------------------------------------------------- + +TEST(RenderExpression, PrettyPrintOutputReparses) { + // The whole point: emitted source must parse back. toString()'s own + // child rendering follows the reference's terminator-free format, so + // fmtExpr has a dedicated arm that uses fmtBlock instead -- if that arm + // regresses, the second parse below throws. + const std::string src = + "obj = render() difference() { cube(100); sphere(20); };\n" + "v = render() { cube(1); }.volume;\n" + "w = render($fn = 32) { x = 5; cube(x); };\n" + "e = render() { };\n" + "render() cube(9);\n"; + std::string once = toOpenscad(parseSrc(src)); + std::string twice; + ASSERT_NO_THROW(twice = toOpenscad(parseSrc(once))); + EXPECT_EQ(once, twice) << "formatting is not idempotent:\n" << once; +} + +TEST(RenderExpression, ToStringIsReparseable) { + auto ast = parseSrc("obj = render($fn = 32) { cube(1); sphere(2); };"); + const RenderExpression* r = renderExprOf(ast); + ASSERT_NE(r, nullptr); + EXPECT_EQ(r->toString(), "render($fn = 32) { cube(1); sphere(2); }"); + EXPECT_NO_THROW(parseSrc("y = " + r->toString() + ";")); +} + +TEST(RenderExpression, JsonRoundTrip) { + const std::string src = + "obj = render() difference() { cube(100); sphere(20); };\n" + "w = render($fn = 32) { x = 5; cube(x); };\n" + "e = render() { };\n"; + auto ast = parseSrc(src); + std::string before = toOpenscad(ast); + auto rebuilt = astFromJsonString(astToJsonString(ast)); + EXPECT_EQ(before, toOpenscad(rebuilt)); +} + +// -- Scope ---------------------------------------------------------------- + +TEST(RenderExpression, HoistsAssignmentsInItsChildBlock) { + // buildScope() must call collectHoistedDeclarations, exactly as + // ModularCall does, or `x` below resolves to nothing. + EXPECT_NO_THROW(parseSrc("obj = render() { cube(x); x = 5; };")); +}