Merge pull request #542 from t-paul/issue500

Allow for statement with negative step value.

16 files changed, 306 insertions, 53 deletions

diff --git a/src/control.cc b/src/control.cc
index 10aadf0..d5f664e 100644
--- a/src/control.cc
+++ b/src/control.cc
@@ -78,12 +78,14 @@ void ControlModule::for_eval(AbstractNode &node, const ModuleInstantiation &inst
 		Context c(ctx);
 		if (it_values.type() == Value::RANGE) {
 			Value::RangeType range = it_values.toRange();
-			range.normalize();
-			if (range.nbsteps()<10000) {
-				for (double i = range.begin; i <= range.end; i += range.step) {
-					c.set_variable(it_name, Value(i));
-					for_eval(node, inst, l+1, &c, evalctx);
-				}
+                        uint32_t steps = range.nbsteps();
+                        if (steps >= 10000) {
+                                PRINTB("WARNING: Bad range parameter in for statement: too many elements (%lu).", steps);
+                        } else {
+                            for (Value::RangeType::iterator it = range.begin();it != range.end();it++) {
+                                c.set_variable(it_name, Value(*it));
+                                for_eval(node, inst, l+1, &c, evalctx);
+                            }
 			}
 		}
 		else if (it_values.type() == Value::VECTOR) {
@@ -227,13 +229,13 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns
 			else if (value.type() == Value::RANGE) {
 				AbstractNode* node = new AbstractNode(inst);
 				Value::RangeType range = value.toRange();
-				range.normalize();
-				if (range.nbsteps()>=10000) {
-					PRINTB("WARNING: Bad range parameter for children: too many elements (%d).", (int)((range.begin-range.end)/range.step));
+                                uint32_t steps = range.nbsteps();
+				if (steps >= 10000) {
+					PRINTB("WARNING: Bad range parameter for children: too many elements (%lu).", steps);
 					return NULL;
 				}
-				for (double i = range.begin; i <= range.end; i += range.step) {
-					AbstractNode* childnode = getChild(Value(i),modulectx); // with error cases
+                                for (Value::RangeType::iterator it = range.begin();it != range.end();it++) {
+					AbstractNode* childnode = getChild(Value(*it),modulectx); // with error cases
 					if (childnode==NULL) continue; // error
 					node->children.push_back(childnode);
 				}
diff --git a/src/expr.cc b/src/expr.cc
index 594fccf..08615ba 100644
--- a/src/expr.cc
+++ b/src/expr.cc
@@ -117,11 +117,18 @@ Value Expression::evaluate(const Context *context) const
 	if (this->type == "R") {
 		Value v1 = this->children[0]->evaluate(context);
 		Value v2 = this->children[1]->evaluate(context);
-		Value v3 = this->children[2]->evaluate(context);
-		if (v1.type() == Value::NUMBER && v2.type() == Value::NUMBER && v3.type() == Value::NUMBER) {
-			Value::RangeType range(v1.toDouble(), v2.toDouble(), v3.toDouble());
-			return Value(range);
-		}
+                if (this->children.size() == 2) {
+                        if (v1.type() == Value::NUMBER && v2.type() == Value::NUMBER) {
+                                Value::RangeType range(v1.toDouble(), v2.toDouble());
+                                return Value(range);
+                        }
+                } else {
+        		Value v3 = this->children[2]->evaluate(context);
+                        if (v1.type() == Value::NUMBER && v2.type() == Value::NUMBER && v3.type() == Value::NUMBER) {
+                                Value::RangeType range(v1.toDouble(), v2.toDouble(), v3.toDouble());
+                                return Value(range);
+                        }
+                }
 		return Value();
 	}
 	if (this->type == "V") {
@@ -192,7 +199,11 @@ std::string Expression::toString() const
 		stream << this->const_value;
 	}
 	else if (this->type == "R") {
-		stream << "[" << *this->children[0] << " : " << *this->children[1] << " : " << *this->children[2] << "]";
+		stream << "[" << *this->children[0] << " : " << *this->children[1];
+		if (this->children.size() > 2) {
+			stream << " : " << *this->children[2];
+		}
+		stream << "]";
 	}
 	else if (this->type == "V") {
 		stream << "[";
diff --git a/src/parser.y b/src/parser.y
index 5645104..6446e82 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -325,11 +325,9 @@ expr:
             }
         | '[' expr ':' expr ']'
             {
-                Expression *e_one = new Expression(Value(1.0));
                 $$ = new Expression();
                 $$->type = "R";
                 $$->children.push_back($2);
-                $$->children.push_back(e_one);
                 $$->children.push_back($4);
             }
         | '[' expr ':' expr ':' expr ']'
diff --git a/src/value.cc b/src/value.cc
index ac33a3b..5afb650 100644
--- a/src/value.cc
+++ b/src/value.cc
@@ -232,7 +232,7 @@ public:
   }
 
   std::string operator()(const Value::RangeType &v) const {
-    return (boost::format("[%1% : %2% : %3%]") % v.begin % v.step % v.end).str();
+    return (boost::format("[%1% : %2% : %3%]") % v.begin_val % v.step_val % v.end_val).str();
   }
 };
 
@@ -600,9 +600,9 @@ public:
 
   Value operator()(const Value::RangeType &range, const double &idx) const {
     switch(int(idx)) {
-    case 0: return Value(range.begin);
-    case 1: return Value(range.step);
-    case 2: return Value(range.end);
+    case 0: return Value(range.begin_val);
+    case 1: return Value(range.step_val);
+    case 2: return Value(range.end_val);
     }
     return Value::undefined;
   }
@@ -617,3 +617,97 @@ Value Value::operator[](const Value &v)
 {
   return boost::apply_visitor(bracket_visitor(), this->value, v.value);
 }
+
+void Value::RangeType::normalize() {
+  if ((step_val>0) && (end_val < begin_val)) {
+    std::swap(begin_val,end_val);
+    PRINT("DEPRECATED: Using ranges of the form [begin:end] with begin value greater than the end value is deprecated.");
+  }
+}
+
+uint32_t Value::RangeType::nbsteps() const {
+  if (begin_val == end_val) {
+    return 0;
+  }
+  
+  if (step_val == 0) { 
+    return std::numeric_limits<uint32_t>::max();
+  }
+
+  double steps;
+  if (step_val < 0) {
+    if (begin_val < end_val) {
+      return 0;
+    }
+    steps = (begin_val - end_val) / (-step_val);
+  } else {
+    if (begin_val > end_val) {
+      return 0;
+    }
+    steps = (end_val - begin_val) / step_val;
+  }
+  
+  return steps;
+}
+
+Value::RangeType::iterator::iterator(Value::RangeType &range, type_t type) : range(range), val(range.begin_val)
+{
+    this->type = type;
+    update_type();
+}
+
+void Value::RangeType::iterator::update_type()
+{
+    if (range.step_val == 0) {
+        type = RANGE_TYPE_END;
+    } else if (range.step_val < 0) {
+        if (val < range.end_val) {
+            type = RANGE_TYPE_END;
+        }
+    } else {
+        if (val > range.end_val) {
+            type = RANGE_TYPE_END;
+        }
+    }
+}
+
+Value::RangeType::iterator::reference Value::RangeType::iterator::operator*()
+{
+    return val;
+}
+
+Value::RangeType::iterator::pointer Value::RangeType::iterator::operator->()
+{
+    return &(operator*());
+}
+
+Value::RangeType::iterator::self_type Value::RangeType::iterator::operator++()
+{
+    if (type < 0) {
+        type = RANGE_TYPE_RUNNING;
+    }
+    val += range.step_val;
+    update_type();
+    return *this;
+}
+
+Value::RangeType::iterator::self_type Value::RangeType::iterator::operator++(int)
+{
+    self_type tmp(*this);
+    operator++();
+    return tmp;
+}
+
+bool Value::RangeType::iterator::operator==(const self_type &other) const
+{
+    if (type == RANGE_TYPE_RUNNING) {
+        return (type == other.type) && (val == other.val) && (range == other.range);
+    } else {
+        return (type == other.type) && (range == other.range);
+    }
+}
+
+bool Value::RangeType::iterator::operator!=(const self_type &other) const
+{
+    return !(*this == other);
+}
diff --git a/src/value.h b/src/value.h
index 388b721..8197806 100644
--- a/src/value.h
+++ b/src/value.h
@@ -31,33 +31,64 @@ std::ostream &operator<<(std::ostream &stream, const Filename &filename);
 class Value
 {
 public:
-  struct RangeType {
+  class RangeType {
+  private:
+    double begin_val;
+    double step_val;
+    double end_val;
+ 
+    /// inverse begin/end if begin is upper than end
+    void normalize();
+
+  public:
+    typedef enum { RANGE_TYPE_BEGIN, RANGE_TYPE_RUNNING, RANGE_TYPE_END } type_t;
+    
+    class iterator {
+    public:
+        typedef iterator self_type;
+        typedef double value_type;
+        typedef double& reference;
+        typedef double* pointer;
+        typedef std::forward_iterator_tag iterator_category;
+        typedef double difference_type;
+        iterator(RangeType &range, type_t type);
+        self_type operator++();
+        self_type operator++(int junk);
+        reference operator*();
+        pointer operator->();
+        bool operator==(const self_type& other) const;
+        bool operator!=(const self_type& other) const;
+    private:
+      RangeType &range;
+      double val;
+      type_t type;
+      
+      void update_type();
+    };
+        
+    RangeType(double begin, double end)
+      : begin_val(begin), step_val(1.0), end_val(end)
+    {
+      normalize();
+    }
+
     RangeType(double begin, double step, double end)
-      : begin(begin), step(step), end(end) {}
+      : begin_val(begin), step_val(step), end_val(end) {}
 
     bool operator==(const RangeType &other) const {
-      return this->begin == other.begin &&
-        this->step == other.step &&
-        this->end == other.end;
+      return this->begin_val == other.begin_val &&
+        this->step_val == other.step_val &&
+        this->end_val == other.end_val;
     }
 
-    /// inverse begin/end if begin is upper than end
-    void normalize() {
-		if ((step>0) && (end < begin)) {
-			std::swap(begin,end);
-		}
-	}
-	/// return number of steps, max int value if step is null
-	int nbsteps() const {
-		if (step<=0) {
-			return std::numeric_limits<int>::max();
-		}
-		return (int)((begin-end)/step);
-	}
-
-    double begin;
-    double step;
-    double end;
+    iterator begin() { return iterator(*this, RANGE_TYPE_BEGIN); }
+    iterator end() { return iterator(*this, RANGE_TYPE_END); }
+
+    /// return number of steps, max uint32_ value if step is 0
+    uint32_t nbsteps() const;
+    
+    friend class tostring_visitor;
+    friend class bracket_visitor;
   };
 
   typedef std::vector<Value> VectorType;
diff --git a/testdata/scad/features/for-tests.scad b/testdata/scad/features/for-tests.scad
index fe36789..10295b1 100644
--- a/testdata/scad/features/for-tests.scad
+++ b/testdata/scad/features/for-tests.scad
@@ -22,10 +22,10 @@ for(r=[1:2:6]) translate([r*10-30,30,0]) difference() {cylinder(r=r, center=true
 for(r=[1.5:0.2:2.5]) translate([r*10-30,30,0]) cube([1, 4*r, 2], center=true);
 
 // Negative range, negative step
-for(r=[5:-1:1]) translate([r*10-60,40,0]) cylinder(r=r);
+for(r=[5:-1:1]) translate([r*10-30,50,0]) cylinder(r=r);
 
-// Negative range, positive step
-for(r=[5:1:1]) translate([r*10-30,40,0]) cylinder(r=r);
+// Negative range, positive step (using backward compatible auto swap of begin and end)
+for(r=[5:1]) translate([r*10-30,40,0]) cylinder(r=r);
 
 // Zero step
 
diff --git a/testdata/scad/misc/children-tests.scad b/testdata/scad/misc/children-tests.scad
index a9a3cf9..1c3d9ea 100644
--- a/testdata/scad/misc/children-tests.scad
+++ b/testdata/scad/misc/children-tests.scad
@@ -53,7 +53,7 @@ module test_children_range() {
 	children([0:4]); // all
 	children([1:2]); // child2, child3
 	children([0:2:4]); // child1, child3, child5
-	children([4:-1:0]); // out, out
+	children([0:-1:4]); // out, out
 	echo("Children range: end");
 }
 test_children_range() {
diff --git a/testdata/scad/misc/range-tests.scad b/testdata/scad/misc/range-tests.scad
new file mode 100644
index 0000000..42ef2a4
--- /dev/null
+++ b/testdata/scad/misc/range-tests.scad
@@ -0,0 +1,20 @@
+echo("[a01] ----- [1:4]");      for (a = [1:4])      echo ("[a01] ", a);
+echo("[a02] ----- [4:1]");      for (a = [4:1])      echo ("[a02] ", a);
+echo("[a03] ----- [0:0]");      for (a = [0:0])      echo ("[a03] ", a);
+echo("[a04] ----- [0:3]");      for (a = [0:3])      echo ("[a04] ", a);
+echo("[a05] ----- [-3:0]");     for (a = [-3:0])     echo ("[a05] ", a);
+echo("[a06] ----- [0:-3]");     for (a = [0:-3])     echo ("[a06] ", a);
+echo("[a07] ----- [-2:2]");     for (a = [-2:2])     echo ("[a07] ", a);
+echo("[a08] ----- [2:-2]");     for (a = [2:-2])     echo ("[a08] ", a);
+
+echo("[b01] ----- [1:1:5]");    for (a = [1:1:5])    echo ("[b01] ", a);
+echo("[b02] ----- [1:2:5]");    for (a = [1:2:5])    echo ("[b02] ", a);
+echo("[b03] ----- [1:-1:5]");   for (a = [1:-1:5])   echo ("[b03] ", a);
+echo("[b04] ----- [5:1:1]");    for (a = [5:1:1])    echo ("[b04] ", a);
+echo("[b05] ----- [5:2:1]");    for (a = [5:2:1])    echo ("[b05] ", a);
+echo("[b06] ----- [5:-1:1]");   for (a = [5:-1:1])   echo ("[b06] ", a);
+echo("[b07] ----- [0:0:0]");    for (a = [0:0:0])    echo ("[b07] ", a);
+echo("[b08] ----- [1:0:1]");    for (a = [1:0:1])    echo ("[b08] ", a);
+echo("[b09] ----- [1:0:5]");    for (a = [1:0:5])    echo ("[b09] ", a);
+echo("[b10] ----- [0:1:0]");    for (a = [0:1:0])    echo ("[b10] ", a);
+echo("[b11] ----- [3:-.5:-3]"); for (a = [3:-.5:-3]) echo ("[b11] ", a);
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 0e5981f..6aff17b 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -804,7 +804,8 @@ list(APPEND ECHO_FILES ${FUNCTION_FILES}
             ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/lookup-tests.scad
             ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/expression-shortcircuit-tests.scad
             ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/parent_module-tests.scad
-            ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/children-tests.scad)
+            ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/children-tests.scad
+            ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/range-tests.scad)
 
 list(APPEND DUMPTEST_FILES ${FEATURES_FILES} ${EXAMPLE_FILES})
 list(APPEND DUMPTEST_FILES ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/escape-test.scad
diff --git a/tests/regression/cgalpngtest/for-tests-expected.png b/tests/regression/cgalpngtest/for-tests-expected.png
index bf1970a..65db59f 100644
--- a/tests/regression/cgalpngtest/for-tests-expected.png
+++ b/tests/regression/cgalpngtest/for-tests-expected.png
diff --git a/tests/regression/dumptest/for-tests-expected.csg b/tests/regression/dumptest/for-tests-expected.csg
index 7aa29d7..b61d9cd 100644
--- a/tests/regression/dumptest/for-tests-expected.csg
+++ b/tests/regression/dumptest/for-tests-expected.csg
@@ -80,7 +80,23 @@ group() {
 			cube(size = [1, 9.2, 2], center = true);
 		}
 	}
-	group();
+	group() {
+		multmatrix([[1, 0, 0, 20], [0, 1, 0, 50], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+			cylinder($fn = 0, $fa = 12, $fs = 2, h = 1, r1 = 5, r2 = 5, center = false);
+		}
+		multmatrix([[1, 0, 0, 10], [0, 1, 0, 50], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+			cylinder($fn = 0, $fa = 12, $fs = 2, h = 1, r1 = 4, r2 = 4, center = false);
+		}
+		multmatrix([[1, 0, 0, 0], [0, 1, 0, 50], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+			cylinder($fn = 0, $fa = 12, $fs = 2, h = 1, r1 = 3, r2 = 3, center = false);
+		}
+		multmatrix([[1, 0, 0, -10], [0, 1, 0, 50], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+			cylinder($fn = 0, $fa = 12, $fs = 2, h = 1, r1 = 2, r2 = 2, center = false);
+		}
+		multmatrix([[1, 0, 0, -20], [0, 1, 0, 50], [0, 0, 1, 0], [0, 0, 0, 1]]) {
+			cylinder($fn = 0, $fa = 12, $fs = 2, h = 1, r1 = 1, r2 = 1, center = false);
+		}
+	}
 	group() {
 		multmatrix([[1, 0, 0, -20], [0, 1, 0, 40], [0, 0, 1, 0], [0, 0, 0, 1]]) {
 			cylinder($fn = 0, $fa = 12, $fs = 2, h = 1, r1 = 1, r2 = 1, center = false);
diff --git a/tests/regression/echotest/children-tests-expected.echo b/tests/regression/echotest/children-tests-expected.echo
index 7b8278a..cdd2eb1 100644
--- a/tests/regression/echotest/children-tests-expected.echo
+++ b/tests/regression/echotest/children-tests-expected.echo
@@ -31,5 +31,4 @@ ECHO: "child3"
 ECHO: "child1"
 ECHO: "child3"
 ECHO: "child5"
-WARNING: Bad range parameter for children: too many elements (-4).
 ECHO: "Children range: end"
diff --git a/tests/regression/echotest/range-tests-expected.echo b/tests/regression/echotest/range-tests-expected.echo
new file mode 100644
index 0000000..ddff38e
--- /dev/null
+++ b/tests/regression/echotest/range-tests-expected.echo
@@ -0,0 +1,81 @@
+ECHO: "[a01] ----- [1:4]"
+ECHO: "[a01] ", 1
+ECHO: "[a01] ", 2
+ECHO: "[a01] ", 3
+ECHO: "[a01] ", 4
+ECHO: "[a02] ----- [4:1]"
+DEPRECATED: Using ranges of the form [begin:end] with begin value greater than the end value is deprecated.
+ECHO: "[a02] ", 1
+ECHO: "[a02] ", 2
+ECHO: "[a02] ", 3
+ECHO: "[a02] ", 4
+ECHO: "[a03] ----- [0:0]"
+ECHO: "[a03] ", 0
+ECHO: "[a04] ----- [0:3]"
+ECHO: "[a04] ", 0
+ECHO: "[a04] ", 1
+ECHO: "[a04] ", 2
+ECHO: "[a04] ", 3
+ECHO: "[a05] ----- [-3:0]"
+ECHO: "[a05] ", -3
+ECHO: "[a05] ", -2
+ECHO: "[a05] ", -1
+ECHO: "[a05] ", 0
+ECHO: "[a06] ----- [0:-3]"
+DEPRECATED: Using ranges of the form [begin:end] with begin value greater than the end value is deprecated.
+ECHO: "[a06] ", -3
+ECHO: "[a06] ", -2
+ECHO: "[a06] ", -1
+ECHO: "[a06] ", 0
+ECHO: "[a07] ----- [-2:2]"
+ECHO: "[a07] ", -2
+ECHO: "[a07] ", -1
+ECHO: "[a07] ", 0
+ECHO: "[a07] ", 1
+ECHO: "[a07] ", 2
+ECHO: "[a08] ----- [2:-2]"
+DEPRECATED: Using ranges of the form [begin:end] with begin value greater than the end value is deprecated.
+ECHO: "[a08] ", -2
+ECHO: "[a08] ", -1
+ECHO: "[a08] ", 0
+ECHO: "[a08] ", 1
+ECHO: "[a08] ", 2
+ECHO: "[b01] ----- [1:1:5]"
+ECHO: "[b01] ", 1
+ECHO: "[b01] ", 2
+ECHO: "[b01] ", 3
+ECHO: "[b01] ", 4
+ECHO: "[b01] ", 5
+ECHO: "[b02] ----- [1:2:5]"
+ECHO: "[b02] ", 1
+ECHO: "[b02] ", 3
+ECHO: "[b02] ", 5
+ECHO: "[b03] ----- [1:-1:5]"
+ECHO: "[b04] ----- [5:1:1]"
+ECHO: "[b05] ----- [5:2:1]"
+ECHO: "[b06] ----- [5:-1:1]"
+ECHO: "[b06] ", 5
+ECHO: "[b06] ", 4
+ECHO: "[b06] ", 3
+ECHO: "[b06] ", 2
+ECHO: "[b06] ", 1
+ECHO: "[b07] ----- [0:0:0]"
+ECHO: "[b08] ----- [1:0:1]"
+ECHO: "[b09] ----- [1:0:5]"
+WARNING: Bad range parameter in for statement: too many elements (4294967295).
+ECHO: "[b10] ----- [0:1:0]"
+ECHO: "[b10] ", 0
+ECHO: "[b11] ----- [3:-.5:-3]"
+ECHO: "[b11] ", 3
+ECHO: "[b11] ", 2.5
+ECHO: "[b11] ", 2
+ECHO: "[b11] ", 1.5
+ECHO: "[b11] ", 1
+ECHO: "[b11] ", 0.5
+ECHO: "[b11] ", 0
+ECHO: "[b11] ", -0.5
+ECHO: "[b11] ", -1
+ECHO: "[b11] ", -1.5
+ECHO: "[b11] ", -2
+ECHO: "[b11] ", -2.5
+ECHO: "[b11] ", -3
diff --git a/tests/regression/moduledumptest/allexpressions-expected.ast b/tests/regression/moduledumptest/allexpressions-expected.ast
index 6d9de40..69ec746 100644
--- a/tests/regression/moduledumptest/allexpressions-expected.ast
+++ b/tests/regression/moduledumptest/allexpressions-expected.ast
@@ -6,7 +6,7 @@ e = $fn;
 f1 = [1];
 f2 = [1, 2, 3];
 g = ((f2.x + f2.y) + f2.z);
-h1 = [2 : 1 : 5];
+h1 = [2 : 5];
 h2 = [1 : 2 : 10];
 i = ((h2.begin - h2.step) - h2.end);
 j = "test";
diff --git a/tests/regression/opencsgtest/for-tests-expected.png b/tests/regression/opencsgtest/for-tests-expected.png
index 968659d..5218b8b 100644
--- a/tests/regression/opencsgtest/for-tests-expected.png
+++ b/tests/regression/opencsgtest/for-tests-expected.png
diff --git a/tests/regression/throwntogethertest/for-tests-expected.png b/tests/regression/throwntogethertest/for-tests-expected.png
index 6641555..7b58005 100644
--- a/tests/regression/throwntogethertest/for-tests-expected.png
+++ b/tests/regression/throwntogethertest/for-tests-expected.png