RangeLiteral records whether the step was written (#6)

`[a:b]` and `[a:1:b]` produced identical ASTs, because makeRangeLiteral
synthesizes the 1.0 for the two-argument form. That is fine for evaluating
the range -- the value is the same either way -- but it throws away the one
thing the evaluator needs to tell a typo from a choice: `[5:0]` is almost
always `[5:-1:0]` written wrong, while `[5:1:0]` is someone saying what they
meant.

So keep a flag. The synthesized step node stays exactly where it was, so
every consumer that just wants the value is untouched.

toString() now prints back the form that was written. Emitting the
synthesized step for a two-argument range produced a program that behaves
identically but reads as an explicit step -- so reformatting a file would
have quietly suppressed the evaluator's warning for it.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: revarbat

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<Expression> start, std::unique_ptr<Expression> end,
-                 std::unique_ptr<Expression> step)
+                 std::unique_ptr<Expression> 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<Expression> start;
     std::unique_ptr<Expression> end;
     std::unique_ptr<Expression> 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;
 };

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

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<NumberLiteral>(driver.toPosition(loc), 1.0);
     }
     return std::make_unique<RangeLiteral>(driver.toPosition(loc), nodeCast<Expression>(std::move(start)),
-                                           nodeCast<Expression>(std::move(end)), nodeCast<Expression>(std::move(step)));
+                                           nodeCast<Expression>(std::move(end)), nodeCast<Expression>(std::move(step)),
+                                           implicitStep);
 }
 
 NodePtr makePositionalArgument(ParserDriver& driver, const OscadLocation& loc, NodePtr expr) {

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<std::string, Builder>& registry() {
         {"RangeLiteral",
          [](const json& j, Position pos) -> std::unique_ptr<ASTNode> {
              return std::make_unique<RangeLiteral>(std::move(pos), childFromJson<Expression>(j, "start"),
-                                                    childFromJson<Expression>(j, "end"), childFromJson<Expression>(j, "step"));
+                                                    childFromJson<Expression>(j, "end"), childFromJson<Expression>(j, "step"),
+                                                    j.value("implicitStep", false));
          }},
         {"ParameterDeclaration",
          [](const json& j, Position pos) -> std::unique_ptr<ASTNode> {

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");
+}