From 094cef26d463e5041c6106eead859abec34ea719 Mon Sep 17 00:00:00 2001
From: bubnikv <bubnikv@gmail.com>
Date: Tue, 21 Jan 2020 17:11:41 +0100
Subject: [PATCH] Fix of the new PlaceholderParser int() conversion. Fixes
 https://github.com/prusa3d/PrusaSlicer/pull/3271 Also some old errors (typos,
 UBs) were fixed.

---
 src/libslic3r/PlaceholderParser.cpp         | 14 +++++++-------
 tests/libslic3r/test_placeholder_parser.cpp |  7 ++++---
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/libslic3r/PlaceholderParser.cpp b/src/libslic3r/PlaceholderParser.cpp
index bbf32a141..db3134ded 100644
--- a/src/libslic3r/PlaceholderParser.cpp
+++ b/src/libslic3r/PlaceholderParser.cpp
@@ -338,7 +338,7 @@ namespace client
             case TYPE_INT :
                 return expr<Iterator>(this->i(), start_pos, this->it_range.end());
             case TYPE_DOUBLE:
-                return expr<Iterator>((int)(this->d()), start_pos, this->it_range.end()); 
+                return expr<Iterator>(static_cast<int>(this->d()), start_pos, this->it_range.end()); 
             default:
                 this->throw_exception("Cannot convert to integer.");
             }
@@ -418,7 +418,7 @@ namespace client
         {
             this->throw_if_not_numeric("Cannot divide a non-numeric type.");
             rhs.throw_if_not_numeric("Cannot divide with a non-numeric type.");
-            if ((this->type == TYPE_INT) ? (rhs.i() == 0) : (rhs.d() == 0.))
+            if ((rhs.type == TYPE_INT) ? (rhs.i() == 0) : (rhs.d() == 0.))
                 rhs.throw_exception("Division by zero");
             if (this->type == TYPE_DOUBLE || rhs.type == TYPE_DOUBLE) {
                 double d = this->as_d() / rhs.as_d();
@@ -434,7 +434,7 @@ namespace client
         {
             this->throw_if_not_numeric("Cannot divide a non-numeric type.");
             rhs.throw_if_not_numeric("Cannot divide with a non-numeric type.");
-            if ((this->type == TYPE_INT) ? (rhs.i() == 0) : (rhs.d() == 0.))
+            if ((rhs.type == TYPE_INT) ? (rhs.i() == 0) : (rhs.d() == 0.))
                 rhs.throw_exception("Division by zero");
             if (this->type == TYPE_DOUBLE || rhs.type == TYPE_DOUBLE) {
                 double d = std::fmod(this->as_d(), rhs.as_d());
@@ -845,7 +845,7 @@ namespace client
                 } else {
                     // Use the human readable error message.
                     msg += ". ";
-                    msg + it->second;
+                    msg += it->second;
                 }
             }
             msg += '\n';
@@ -1134,7 +1134,7 @@ namespace client
                 static void string_(boost::iterator_range<Iterator> &it_range, expr<Iterator> &out)
                         { out = expr<Iterator>(std::string(it_range.begin() + 1, it_range.end() - 1), it_range.begin(), it_range.end()); }
                 static void expr_(expr<Iterator> &value, Iterator &end_pos, expr<Iterator> &out)
-                        { out = expr<Iterator>(std::move(value), out.it_range.begin(), end_pos); }
+                        { auto begin_pos = out.it_range.begin(); out = expr<Iterator>(std::move(value), begin_pos, end_pos); }
                 static void minus_(expr<Iterator> &value, expr<Iterator> &out)
                         { out = value.unary_minus(out.it_range.begin()); }
                 static void not_(expr<Iterator> &value, expr<Iterator> &out)
@@ -1152,8 +1152,7 @@ namespace client
                                                                     [ px::bind(&expr<Iterator>::min, _val, _2) ]
                 |   (kw["max"] > '(' > conditional_expression(_r1) [_val = _1] > ',' > conditional_expression(_r1) > ')') 
                                                                     [ px::bind(&expr<Iterator>::max, _val, _2) ]
-                                                                    //FIXME this is likley not correct
-                |   (kw["int"] > '(' > unary_expression(_r1) /* > ')' */   )  [ px::bind(&FactorActions::to_int,  _1,     _val) ]
+                |   (kw["int"] > '(' > unary_expression(_r1) > ')') [ px::bind(&FactorActions::to_int,  _1,     _val) ]
                 |   (strict_double > iter_pos)                      [ px::bind(&FactorActions::double_, _1, _2, _val) ]
                 |   (int_      > iter_pos)                          [ px::bind(&FactorActions::int_,    _1, _2, _val) ]
                 |   (kw[bool_] > iter_pos)                          [ px::bind(&FactorActions::bool_,   _1, _2, _val) ]
@@ -1181,6 +1180,7 @@ namespace client
             keywords.add
                 ("and")
                 ("if")
+                ("int")
                 //("inf")
                 ("else")
                 ("elsif")
diff --git a/tests/libslic3r/test_placeholder_parser.cpp b/tests/libslic3r/test_placeholder_parser.cpp
index 5802862b7..4d8217c16 100644
--- a/tests/libslic3r/test_placeholder_parser.cpp
+++ b/tests/libslic3r/test_placeholder_parser.cpp
@@ -28,8 +28,8 @@ SCENARIO("Placeholder parser scripting", "[PlaceholderParser]") {
     SECTION("math: 2*3/6") { REQUIRE(parser.process("{2*3/6}") == "1"); }
     SECTION("math: 2*3/12") { REQUIRE(parser.process("{2*3/12}") == "0"); }
     SECTION("math: 2.*3/12") { REQUIRE(std::stod(parser.process("{2.*3/12}")) == Approx(0.5)); }
-//    SECTION("math: 10 % 2.5") { REQUIRE(parser.process("{10%2.5}") == "0"); }
-//    SECTION("math: 11 / 2.5") { REQUIRE(parser.process("{11/2.5-1}") == "1"); }
+    SECTION("math: 10 % 2.5") { REQUIRE(std::stod(parser.process("{10%2.5}")) == Approx(0.)); }
+    SECTION("math: 11 % 2.5") { REQUIRE(std::stod(parser.process("{11%2.5}")) == Approx(1.)); }
     SECTION("math: 2*(3-12)") { REQUIRE(parser.process("{2*(3-12)}") == "-18"); }
     SECTION("math: 2*foo*(3-12)") { REQUIRE(parser.process("{2*foo*(3-12)}") == "0"); }
     SECTION("math: 2*bar*(3-12)") { REQUIRE(parser.process("{2*bar*(3-12)}") == "-36"); }
@@ -38,7 +38,8 @@ SCENARIO("Placeholder parser scripting", "[PlaceholderParser]") {
     SECTION("math: max(12, 14)") { REQUIRE(parser.process("{max(12, 14)}") == "14"); }
     SECTION("math: min(13.4, -1238.1)") { REQUIRE(std::stod(parser.process("{min(13.4, -1238.1)}")) == Approx(-1238.1)); }
     SECTION("math: max(13.4, -1238.1)") { REQUIRE(std::stod(parser.process("{max(13.4, -1238.1)}")) == Approx(13.4)); }
-//    SECTION("math: int(13.4)") { REQUIRE(parser.process("{int(13.4)}") == "13"); }
+    SECTION("math: int(13.4)") { REQUIRE(parser.process("{int(13.4)}") == "13"); }
+    SECTION("math: int(-13.4)") { REQUIRE(parser.process("{int(-13.4)}") == "-13"); }
 
     // Test the boolean expression parser.
     auto boolean_expression = [&parser](const std::string& templ) { return parser.evaluate_boolean_expression(templ, parser.config()); };