diff --git a/include/openscad_cpp_parser/ast/expression.hpp b/include/openscad_cpp_parser/ast/expression.hpp index cf84932..456d499 100644 --- a/include/openscad_cpp_parser/ast/expression.hpp +++ b/include/openscad_cpp_parser/ast/expression.hpp @@ -80,14 +80,22 @@ class UndefinedLiteral : public Primary { class RangeLiteral : public Primary { public: RangeLiteral(Position position, std::unique_ptr start, std::unique_ptr end, - std::unique_ptr step) + std::unique_ptr step, bool implicitStep = false) : Primary(NodeKind::RangeLiteral, std::move(position)), start(std::move(start)), end(std::move(end)), - step(std::move(step)) {} + step(std::move(step)), implicitStep(implicitStep) {} std::unique_ptr start; std::unique_ptr end; std::unique_ptr step; + // True when the source wrote the two-argument form `[a:b]` and the step + // node below is the literal 1.0 this parser synthesized for it. Consumers + // that only need the value can ignore this and read `step` as always + // present; it exists because "the author did not choose a step" is not + // recoverable from the synthesized node, and the evaluator's backwards-range + // warning fires only for that case (an explicit step is taken as deliberate). + bool implicitStep = false; + std::string toString() const override; void buildScope(Scope& parentScope) override; }; diff --git a/src/ast/expression.cpp b/src/ast/expression.cpp index 180aaf6..84875af 100644 --- a/src/ast/expression.cpp +++ b/src/ast/expression.cpp @@ -22,6 +22,10 @@ std::string NumberLiteral::toString() const { } std::string RangeLiteral::toString() const { + // Print back the form that was written. Emitting the synthesized step for + // a two-argument range would round-trip `[5:0]` into `[5 : 1 : 0]`, which + // reads identically but suppresses the evaluator's backwards-range warning. + if (implicitStep) return "[" + start->toString() + " : " + end->toString() + "]"; return "[" + start->toString() + " : " + step->toString() + " : " + end->toString() + "]"; } diff --git a/src/grammar/driver.cpp b/src/grammar/driver.cpp index 36f7092..fb6a26d 100644 --- a/src/grammar/driver.cpp +++ b/src/grammar/driver.cpp @@ -46,11 +46,13 @@ NodePtr makeUndefinedLiteral(ParserDriver& driver, const OscadLocation& loc) { } NodePtr makeRangeLiteral(ParserDriver& driver, const OscadLocation& loc, NodePtr start, NodePtr end, NodePtr step) { - if (!step) { + const bool implicitStep = !step; + if (implicitStep) { step = std::make_unique(driver.toPosition(loc), 1.0); } return std::make_unique(driver.toPosition(loc), nodeCast(std::move(start)), - nodeCast(std::move(end)), nodeCast(std::move(step))); + nodeCast(std::move(end)), nodeCast(std::move(step)), + implicitStep); } NodePtr makePositionalArgument(ParserDriver& driver, const OscadLocation& loc, NodePtr expr) { diff --git a/src/serialization/json_io.cpp b/src/serialization/json_io.cpp index bef1843..c5faf7d 100644 --- a/src/serialization/json_io.cpp +++ b/src/serialization/json_io.cpp @@ -112,6 +112,7 @@ json toJsonImpl(const ASTNode& node, bool includePos) { j["start"] = valueToJson(n.start.get(), includePos); j["end"] = valueToJson(n.end.get(), includePos); j["step"] = valueToJson(n.step.get(), includePos); + if (n.implicitStep) j["implicitStep"] = true; break; } case NodeKind::ParameterDeclaration: { @@ -410,7 +411,8 @@ const std::unordered_map& registry() { {"RangeLiteral", [](const json& j, Position pos) -> std::unique_ptr { return std::make_unique(std::move(pos), childFromJson(j, "start"), - childFromJson(j, "end"), childFromJson(j, "step")); + childFromJson(j, "end"), childFromJson(j, "step"), + j.value("implicitStep", false)); }}, {"ParameterDeclaration", [](const json& j, Position pos) -> std::unique_ptr { diff --git a/tests/test_pretty_print.cpp b/tests/test_pretty_print.cpp index 7caa093..720211f 100644 --- a/tests/test_pretty_print.cpp +++ b/tests/test_pretty_print.cpp @@ -515,3 +515,16 @@ TEST(PrettyPrint, AssignmentRhsWithLeadingCommentAndTrailingLineComment) { // no other translation unit -- including this test file -- can name them. // Left as a known, permanent coverage ceiling rather than weakening the // encapsulation just to reach 100%. + +// The two-argument range keeps its shape through a print/reparse cycle. +// It would be easy to always print the synthesized step, and the result +// would still be a correct program -- but `[5:0]` and `[5:1:0]` mean +// different things to the evaluator's backwards-range warning, so +// reformatting a file must not quietly convert one into the other. +TEST(PrettyPrint, ImplicitRangeStepSurvivesRoundTrip) { + auto ast = parseAst("a = [5:0];\nb = [5:1:0];\n"); + const std::string printed = toOpenscad(ast); + EXPECT_NE(printed.find("[5 : 0]"), std::string::npos) << printed; + EXPECT_NE(printed.find("[5 : 1 : 0]"), std::string::npos) << printed; + expectStablePrint("a = [5:0];\nb = [5:1:0];\n"); +}